Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

pdfjs needs local file access #2714

Closed
diracdeltas opened this issue Jul 25, 2016 · 14 comments · Fixed by #8645
Closed

pdfjs needs local file access #2714

diracdeltas opened this issue Jul 25, 2016 · 14 comments · Fixed by #8645

Comments

@diracdeltas
Copy link
Member

diracdeltas commented Jul 25, 2016

Test plan

#8645 (comment)


the pdfjs extension doesn't have the 'Access File URLs' permission, so it can't browsr file:/// pdfs. cc @bridiver

@willy-b
Copy link
Contributor

willy-b commented Dec 22, 2016

hey @diracdeltas, I think you might already have fixed this with #6330.
If not, would you mind pointing me to any resources on how PDFJS is integrated in Brave now? It used to be in browser-laptop/app/extensions/pdfjs but has now moved. Also, is Brave using your fork?

Thanks in advance, I'm looking at this for PR #6385 where I try to open a generated PDF in a new tab

@diracdeltas
Copy link
Member Author

@willy-b it's not fixed yet sadly.

pdfjs is now loaded from a remote server. it gets downloaded in ~/Library/Application Support/brave-development/Extensions/jdbefljfgobbmcidnmpjamcbhnbphjnb/ so you can test changes by editing files in there directly

it's using my fork at https://github.com/diracdeltas/pdf.js (built from master using 'gulp chromium')

@willy-b
Copy link
Contributor

willy-b commented Dec 24, 2016

sweet, thanks for the info

@willy-b
Copy link
Contributor

willy-b commented Jan 4, 2017

I took a look at this. (Thought it'd be useful to share what I found -- if it isn't, feel free to edit or delete this post for me.)

Good news: this seems to be a XHR cross-origin issue. Bad news: changing extension permissions didn't seem to fix it (for me) (see next comment).

I compared Google Chrome, where PDF.js can open local PDFs, with Brave.

You can run something like the following in both browsers to see difference (from debugger during page load to ensure same context for good measure):

var xhr = new XMLHttpRequest()
xhr.open("GET", "file:///home/willy/Downloads/brave-downloads/test-pdf.pdf", true)
xhr.send()

The above works on Google Chrome (from within the PDF.js extension in process of rendering a local PDF)
and does NOT work in Brave (error: Cross origin requests are only supported for protocol schemes: [...])
I followed in debugger and everything else is identical, right up to and including the XHR.

notes / details

  • URL bar suggests an origin difference between browsers (but there isn't really)
    In Google Chrome the URL bar uses the chrome-extension: proto, but Brave shows the file:// extension.
    Chrome: chrome-extension://oemmndcbldboiebfnladdacbdfmadadm/file:///home/willy/Downloads/brave-downloads/test-pdf.pdf
    vs. Brave: file:///home/willy/Downloads/brave-downloads/test-pdf.pdf)
    Nonetheless, comparing raw window.location from debugger shows relevant code sees the same chrome-extension origin for current location in both cases.

  • Finding actual XHR in code
    To see the actual local XHR used to fetch PDF contents you can break at definition of NetworkManager_request within content/build/pdf.worker.js:

    request: function NetworkManager_request(args) {
     var xhr = this.getXhr();
     [...]
     xhr.send(null);

@willy-b
Copy link
Contributor

willy-b commented Jan 4, 2017

There seem to be two general possibilities to me:
(1) There a difference in permissions granted to extension -> check manifest.json
(2) There are differences in Muon/Chromium in how the cross-origin XHRs are handled (also possible)

(1) Differences in permissions / configuration of extension?
I checked manifest.json, tried granting every permission and compared everything.
I couldn't find some discrepancy there that would explain this.

Details below:
I set same permissions as my install of Chrome had for PDF.js, just in case: "permissions": [ "fileBrowserHandler", "webRequest", "webRequestBlocking", "\u003Call_urls>", "tabs", "webNavigation", "storage" ]

Also set same web_accessible_resources: "web_accessible_resources": [ "content/web/viewer.html", "http:/*", "https:/*", "ftp:/*", "file:/*", "chrome-extension:/*", "filesystem:/*", "drive:*" ]

content_security_policy was already identical.

content_script.matches was already more permissive than chrome's: "matches": [ "http://*/*", "https://*/*", "ftp://*/*", "file://*/*"],

This suggests to me that it is (2).

(2) differences in Muon / Chromium?
I started from the observed error to look at this.
The displayed error msg "Cross origin requests are only supported for protocol schemes: [...]"
comes from webkit (searched within browser-laptop-bootstrap)
in DocumentThreadableLoader::makeCrossOriginAccessRequest(const ResourceRequest& request):

./src/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp:285:
client->didFailAccessControlCheck(ResourceError(errorDomainBlinkInternal, 0, request.url().getString(), "Cross origin requests are only supported for protocol schemes: " + SchemeRegistry::listOfCORSEnabledURLSchemes() + "."));

The list of approved protocols is given by SchemeRegistry::listOfCORSEnabledURLSchemes,
the check (a few lines earlier) having been:

if (!SchemeRegistry::shouldTreatURLSchemeAsCORSEnabled(request.url().protocol())) {

(compare latest: https://chromium.googlesource.com/chromium/src/third_party/+/master/WebKit/Source/core/loader/DocumentThreadableLoader.cpp#320)

The SchemeRegistry is defined in:
src/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp

The CORSEnabledSchemes defined are 'http', 'https', and 'data'.
(compare latest: https://chromium.googlesource.com/chromium/src/third_party/+/master/WebKit/Source/platform/weborigin/SchemeRegistry.cpp#38)

But this is unchanged in Brave vs. upstream Chromium.

The whole CORS check only occurs if the request is "external" or not same origin
(see https://chromium.googlesource.com/chromium/src/third_party/+/master/WebKit/Source/core/loader/DocumentThreadableLoader.cpp#289)
determined by isExternalRequest and m_sameOriginRequest so that's where I'm looking now.

Still trying to figure this out. Just thought I'd share in case it is helpful.

@cndouglas
Copy link

Still a problem with Brave 0.13.0. Can we set a milestone for this?

@bbondy bbondy added this to the 1.1.0 milestone Jan 31, 2017
@bbondy
Copy link
Member

bbondy commented Jan 31, 2017

Set to 1.1.0 for now but would move up sooner if there's a fix.

@willy-b
Copy link
Contributor

willy-b commented Mar 12, 2017

Running Brave with --disable-web-security lets PDF.js in Brave open local PDFs (this is NOT recommended, of course).
Surprisingly (to me), the more specific flags --disable-extensions-file-access-check and --allow-file-access-from-files do not have any effect, though.

(I mention this because I would have thought the latter two flags would be enough if this were related to the "Access File URLs" permission.)

@diracdeltas
Copy link
Member Author

IIRC PDFJS loads each PDF via XHR, which requires the CORS header, so it makes sense that disabling web security would work around it. If you could render the contribution statement PDFs in the chrome-extension://{pdfjs_id} origin, that would probably solve the issue.

@bsclifton
Copy link
Member

see @diracdeltas comment

i would suggest working on this after brave/pdf.js#2 is merged, since that PR does a lot of cleanup and removal of code that isn't needed in brave

@diracdeltas diracdeltas self-assigned this Apr 20, 2017
@bbondy
Copy link
Member

bbondy commented Apr 21, 2017

is the new extension updated @bsclifton ?

@diracdeltas
Copy link
Member Author

diracdeltas commented Apr 21, 2017

@bbondy yes, it's merged into brave/pdf.js:master but i haven't built a crx from it yet.

@diracdeltas
Copy link
Member Author

I have a workaround for this but it will require an extension update.

diracdeltas added a commit to brave/pdf.js that referenced this issue May 2, 2017
diracdeltas added a commit that referenced this issue May 3, 2017
fix #2714
requires brave/pdf.js@6b3081e

test plan:
1. git clone https://github.com/brave/pdf.js and run 'gulp chromium'
2. copy 'build/chromium' to '~/Library/Application\ Support/brave-development/Extensions/jdbefljfgobbmcidnmpjamcbhnbphjnb/1.8.251' or whatever your corrresponding userData directory is
3. start Brave with PDFJS enabled and browse to a local PDF file. it should display correctly.
@diracdeltas
Copy link
Member Author

nvm, i found an easier way!

diracdeltas added a commit that referenced this issue May 3, 2017
fix #2714

test plan:
1. start Brave with PDFJS enabled and browse to a local PDF file. it should display correctly. the urlbar should show the 'file:' location.
2. turn off PDFJS and go to the file. it should prompt to download.
@NejcZdovc NejcZdovc modified the milestones: 0.15.2, 0.15.3 May 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.