move pdf.js loading logic into browser-laptop

fix #4651 probably fixes #2715 but ideally the workaround in brave/pdf.js should be removed too may also fix other PDF loading errors
brave · Dec 19, 2017 · 68f188f · 68f188f
1 parent fa85019
commit 68f188f
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 17 deletions.
diff --git a/app/extensions.js b/app/extensions.js
@@ -3,7 +3,6 @@ const contextMenus = require('./browser/extensions/contextMenus')
 const extensionActions = require('./common/actions/extensionActions')
 const config = require('../js/constants/config')
 const appConfig = require('../js/constants/appConfig')
-const messages = require('../js/constants/messages')
 const {fileUrl} = require('../js/lib/appUrlUtil')
 const {getExtensionsPath, getBraveExtUrl, getBraveExtIndexHTML} = require('../js/lib/appUrlUtil')
 const {getSetting} = require('../js/settings')
@@ -17,7 +16,7 @@ const fs = require('fs')
 const path = require('path')
 const l10n = require('../js/l10n')
 const {bravifyText} = require('./renderer/lib/extensionsUtil')
-const {ipcMain, componentUpdater, session} = require('electron')
+const {componentUpdater, session} = require('electron')
 
 // Takes Content Security Policy flags, for example { 'default-src': '*' }
 // Returns a CSP string, for example 'default-src: *;'
@@ -394,10 +393,6 @@ module.exports.init = () => {
     }
   })
 
-  ipcMain.on(messages.LOAD_URL_REQUESTED, (e, tabId, url) => {
-    appActions.loadURLRequested(tabId, url)
-  })
-
   process.on('extension-load-error', (error) => {
     console.error(error)
   })
@@ -449,8 +444,26 @@ module.exports.init = () => {
   }
 
   let loadExtension = (extensionId, extensionPath, manifest = {}, manifestLocation = 'unpacked') => {
-    if (extensionId === config.PDFJSExtensionId) {
+    const isPDF = extensionId === config.PDFJSExtensionId
+    if (isPDF) {
       manifestLocation = 'component'
+      // Override the manifest. TODO: Update manifest in pdf.js itself after enough
+      // clients have updated to Brave 0.20.x+
+      manifest = {
+        name: 'PDF Viewer',
+        manifest_version: 2,
+        version: '1.9.457',
+        description: 'Uses HTML5 to display PDF files directly in the browser.',
+        icons: {
+          '128': 'icon128.png',
+          '48': 'icon48.png',
+          '16': 'icon16.png'
+        },
+        permissions: ['storage', '<all_urls>'],
+        content_security_policy: "script-src 'self'; object-src 'self'",
+        incognito: 'split',
+        key: config.PDFJSExtensionPublicKey
+      }
     }
     if (!extensionInfo.isLoaded(extensionId) && !extensionInfo.isLoading(extensionId)) {
       extensionInfo.setState(extensionId, extensionStates.LOADING)

diff --git a/app/pdfJS.js b/app/pdfJS.js
@@ -5,23 +5,102 @@
 
 const UrlUtil = require('../js/lib/urlutil')
 const Filtering = require('./filtering')
-const config = require('../js/constants/config')
 const appActions = require('../js/actions/appActions')
 const getSetting = require('../js/settings').getSetting
 const settings = require('../js/constants/settings')
 
-const pdfjsBaseUrl = `chrome-extension://${config.PDFJSExtensionId}/`
-const viewerBaseUrl = `${pdfjsBaseUrl}content/web/viewer.html`
+const getViewerUrl = UrlUtil.getPDFViewerUrl
+
+/**
+ * Check if the request is a PDF file.
+ * @param {Object} details First argument of the webRequest.onHeadersReceived
+ *                         event. The properties "responseHeaders" and "url"
+ *                         are read.
+ * @return {boolean} True if the resource is a PDF file.
+ */
+function isPdfFile (details) {
+  var header = details.responseHeaders['Content-Type']
+  if (header) {
+    if (header.includes('application/pdf')) {
+      return true
+    }
+    if (header.includes('application/octet-stream')) {
+      if (details.url.toLowerCase().indexOf('.pdf') > 0) {
+        return true
+      }
+      var cdHeader = details.responseHeaders['Content-Disposition']
+      if (cdHeader && /\.pdf(["']|$)/i.test(cdHeader[0])) {
+        return true
+      }
+    }
+  }
+}
+
+/**
+ * @param {Object} details First argument of the webRequest.onHeadersReceived
+ *                         event. The property "url" is read.
+ * @return {boolean} True if the PDF file should be downloaded.
+ */
+function isPdfDownloadable (details) {
+  if (details.url.indexOf('pdfjs.action=download') >= 0) {
+    return true
+  }
+  // Display the PDF viewer regardless of the Content-Disposition header if the
+  // file is displayed in the main frame, since most often users want to view
+  // a PDF, and servers are often misconfigured.
+  // If the query string contains "=download", do not unconditionally force the
+  // viewer to open the PDF, but first check whether the Content-Disposition
+  // header specifies an attachment. This allows sites like Google Drive to
+  // operate correctly (#6106).
+  if (details.type === 'main_frame' &&
+      details.url.indexOf('=download') === -1) {
+    return false
+  }
+  var cdHeader = (details.responseHeaders &&
+    details.responseHeaders['Content-Disposition'])
+  return (cdHeader && /^attachment/i.test(cdHeader[0]))
+}
+
+/**
+ * Takes a set of headers, and set "Content-Disposition: attachment".
+ * @param {Object} details First argument of the webRequest.onHeadersReceived
+ *                         event. The property "responseHeaders" is read and
+ *                         modified if needed.
+ * @return {Object|undefined} The return value for the responseHeaders property
+ */
+function getHeadersWithContentDispositionAttachment (details) {
+  var headers = details.responseHeaders
+  var cdHeader = headers['Content-Disposition'] || []
+  cdHeader.push('attachment')
+  headers['Content-Disposition'] = cdHeader
+  return headers
+}
 
 const onBeforeRequest = (details) => {
   const result = { resourceName: 'pdfjs' }
-  if (!(details.resourceType === 'mainFrame' &&
+  if (details.resourceType === 'mainFrame' &&
     UrlUtil.isFileScheme(details.url) &&
-    UrlUtil.isFileType(details.url, 'pdf'))) {
-    return result
+    UrlUtil.isFileType(details.url, 'pdf')) {
+    appActions.loadURLRequested(details.tabId, getViewerUrl(details.url))
+    result.cancel = true
+  }
+  return result
+}
+
+const onHeadersReceived = (details) => {
+  const result = { resourceName: 'pdfjs' }
+  // Don't intercept POST requests until http://crbug.com/104058 is fixed.
+  if (details.resourceType === 'mainFrame' && details.method === 'GET' && isPdfFile(details)) {
+    if (isPdfDownloadable(details)) {
+      // Force download by ensuring that Content-Disposition: attachment is set
+      result.responseHeaders = getHeadersWithContentDispositionAttachment(details)
+      return result
+    }
+
+    // Replace frame with viewer
+    appActions.loadURLRequested(details.tabId, getViewerUrl(details.url))
+    result.cancel = true
   }
-  appActions.loadURLRequested(details.tabId, `${viewerBaseUrl}?file=${encodeURIComponent(details.url)}`)
-  result.cancel = true
   return result
 }
 
@@ -31,5 +110,6 @@ const onBeforeRequest = (details) => {
 module.exports.init = () => {
   if (getSetting(settings.PDFJS_ENABLED)) {
     Filtering.registerBeforeRequestFilteringCB(onBeforeRequest)
+    Filtering.registerHeadersReceivedFilteringCB(onHeadersReceived)
   }
 }
diff --git a/js/lib/urlutil.js b/js/lib/urlutil.js
@@ -349,6 +349,12 @@ const UrlUtil = {
     return url.replace(`chrome-extension://${pdfjsExtensionId}/`, '')
   },
 
+  getPDFViewerUrl: function (url) {
+    const pdfjsBaseUrl = `chrome-extension://${pdfjsExtensionId}/`
+    const viewerBaseUrl = `${pdfjsBaseUrl}content/web/viewer.html`
+    return `${viewerBaseUrl}?file=${encodeURIComponent(url)}`
+  },
+
   /**
    * Converts a potential PDF URL to the PDFJS URL.
    * XXX: This only looks at the URL file extension, not MIME types.
@@ -357,7 +363,7 @@ const UrlUtil = {
    */
   toPDFJSLocation: function (url) {
     if (url && UrlUtil.isHttpOrHttps(url) && UrlUtil.isFileType(url, 'pdf')) {
-      return `chrome-extension://${pdfjsExtensionId}/${url}`
+      return UrlUtil.getPDFViewerUrl(url)
     }
     return url
   },

diff --git a/test/unit/lib/urlutilTest.js b/test/unit/lib/urlutilTest.js
@@ -195,7 +195,7 @@ describe('urlutil', function () {
   describe('toPDFJSLocation', function () {
     const baseUrl = 'chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/'
     it('pdf', function () {
-      assert.equal(urlUtil.toPDFJSLocation('http://abc.com/test.pdf'), baseUrl + 'http://abc.com/test.pdf')
+      assert.equal(urlUtil.toPDFJSLocation('http://abc.com/test.pdf'), baseUrl + 'content/web/viewer.html?file=http%3A%2F%2Fabc.com%2Ftest.pdf')
     })
     it('non-pdf', function () {
       assert.equal(urlUtil.toPDFJSLocation('http://abc.com/test.pdf.txt'), 'http://abc.com/test.pdf.txt')
@@ -208,6 +208,16 @@ describe('urlutil', function () {
     })
   })
 
+  describe('getPDFViewerUrl', function () {
+    const baseUrl = 'chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/content/web/viewer.html?file='
+    it('regular url', function () {
+      assert.equal(urlUtil.getPDFViewerUrl('http://example.com'), baseUrl + 'http%3A%2F%2Fexample.com')
+    })
+    it('file url', function () {
+      assert.equal(urlUtil.getPDFViewerUrl('file:///Users/yan/some files/test.pdf'), baseUrl + 'file%3A%2F%2F%2FUsers%2Fyan%2Fsome%20files%2Ftest.pdf')
+    })
+  })
+
   describe('getHostname', function () {
     it('returns undefined if the URL is invalid', function () {
       assert.equal(urlUtil.getHostname(null), undefined)