Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ie11 file origin does not match viewer's origin #9498

Closed
Barzou opened this issue Feb 20, 2018 · 8 comments
Closed

ie11 file origin does not match viewer's origin #9498

Barzou opened this issue Feb 20, 2018 · 8 comments
Labels

Comments

@Barzou
Copy link

Barzou commented Feb 20, 2018

Hi,

PDF.JS : latest release (v1.9.426)
Tested on IE11 (11.192.16299.0)
Use case : I get pdf js sources and I do a gulp minified.

When I open my viewer, it works on all Browser except Internet explorer.
I got this error : "file origin does not match viewer's origin"

Inside pdf.viewer.js script (yeah, i read minified js =P), the part
if(new URL(e,window.location.href).origin!==t)throw new Error("file origin does not match viewer's")
Throw an error
because
new URL(window.location.href).origin is undefined

I saw on an issue where you explained that you use a polyfill for javaScript URL object (#9358) but it doesn't seems to work on internet explorer 11.

To help you, I tried this one https://github.com/lifaon74/url-polyfill/blob/master/url-polyfill.js which seems to work great on IE11.

I'll do a temporary work around to avoid this error on my own project (literally skip throw error).

@Rob--W
Copy link
Member

Rob--W commented Feb 21, 2018

The URL polyfill in src/shared/compatibility.js overrides the URL constructor if origin is not supported.
The default demo viewer works fine for me when I open the viewer in IE 11.1000.17074.0 (and also an older version, IE11.0.9600.17728).

This is therefore not an issue with the default viewer.

I also ran gulp minified from the master branch, and simulated a broken URL API with the following test case:

<script>delete URL.prototype.origin</script>
<script src=pdf.js></script>
<script>document.write(new URL(location.href).origin)</script>

and the test passed (localhost was being printed, instead of undefined).

Did you load pdf.js before pdf.viewer.js?

@Barzou
Copy link
Author

Barzou commented Feb 21, 2018

You're right.

I tested my use case with pdf.js included and it works perfectly.
I wasn't sure about what scripts should be included, because some script are loaded inside library and it worked with other browser.

But.. the online demo https://mozilla.github.io/pdf.js/web/viewer.html include pdf.js and I didn't notice.

Mea culpa.

@Barzou
Copy link
Author

Barzou commented Feb 22, 2018

Still doesn't work on IE11, I have a second problem. Yesterday I tried :
new URL(window.location.href).origin
And this line works fine.

But when a parameter is added inside pdf.viewer.js, it doesn't seems to work.
I added some logs to pdf.viewer.js :

if (new URL(e, window.location.href).origin !== t){
  console.log('new URL(e, window.location.href).origin', new URL(e, window.location.href).origin);
  // result : new URL(e, window.location.href).origin null
  console.log('e', e);
  // result : e blob:E953C0C2-1552-4A49-9A50-54F7753827B2
  console.log('t', t);
  // result : t https://myurl
  console.log("file origin does not match viewer's")
}

new URL(e, window.location.href).origin return null.

@Rob--W Rob--W added the viewer label Feb 22, 2018
@Rob--W
Copy link
Member

Rob--W commented Feb 22, 2018

That "problem" is not IE-specific.
The code/logic in its current form is introduced by #6916, and was designed to block cross-origin requests.

We could allow blob:-URLs, since they can only be loaded if the blob:-URL is created by the same origin as where the viewer is hosted, e.g. like this:

diff --git a/web/app.js b/web/app.js
index e1891650..fe442c85 100644
--- a/web/app.js
+++ b/web/app.js
@@ -1501,11 +1501,12 @@ if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) {
         // Hosted or local viewer, allow for any file locations
         return;
       }
-      let fileOrigin = new URL(file, window.location.href).origin;
+      let inputFileUrl = new URL(file, window.location.href);
       // Removing of the following line will not guarantee that the viewer will
       // start accepting URLs from foreign origin -- CORS headers on the remote
       // server must be properly configured.
-      if (fileOrigin !== viewerOrigin) {
+      if (inputFileUrl.origin !== viewerOrigin &&
+          inputFileUrl.protocol !== 'blob:') {
         throw new Error('file origin does not match viewer\'s');
       }
     } catch (ex) {

@yurydelendik Is blob: in file= something that we want to support?

Side note: There are other URLs that can be loaded without CORS, such as data: and filesystem:.
To avoid cross-origin content spoofing, data: should still not be allowed.
Since the filesystem:-URL is only generated for the long-deprecated and Chrome-only Filesystem API, I don't think that we need to check for that either.

@yurydelendik
Copy link
Contributor

Is blob: in file= something that we want to support?

Ideally, yes. Not sure about IE11 though -- its issue with URL.createObjectURL not generating origin. (BTW, can blob's be cross origin?) We moved HTTP transport to the API side, and if there is a need, we can special case such schemas as data:, blob: or file:.

@Rob--W
Copy link
Member

Rob--W commented Feb 23, 2018

So actually, this problem is IE-specific.

blob:-URLs are specified to include the origin of the creator in the URL: https://www.w3.org/TR/2017/WD-FileAPI-20171026/#unicodeSerializationOfBlobURL

Firefox, Chrome, Safari, Opera, Edge do match the standards.

Internet Explorer does not, it creates URLs that look like blob:UUID instead of blob:origin/UUID.

BTW, can blob's be cross origin?

No, blob:-URLs can only be loaded by same-origin execution contexts (documents, workers).

@Barzou
Copy link
Author

Barzou commented Feb 23, 2018

Thank you for your expertise and the fix ; you are reactive, I really appreciate it :)
Can I close this issue ?

@timvandermeij
Copy link
Contributor

Closing since this is fixed by the PR above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants