jonathanKingston / ember-cli-sri

Generation of SRI hashes for ember
MIT License
48 stars 15 forks source link

An Ember application that uses a non-/ baseURL does not get SRI hashes #18

Open corpulentcoffee opened 8 years ago

corpulentcoffee commented 8 years ago

Quick reproduction:

~/test$ ember --version
version: 1.13.15
Could not find watchman, falling back to NodeWatcher for file system events.
Visit http://www.ember-cli.com/user-guide/#watchman for more info.
node: 4.2.6
npm: 2.14.10
os: linux x64

~/test$ ember new some-project
[ . . . ]

~/test$ cd some-project/

~/test/some-project$ ember build --prod
[ . . . ]

~/test/some-project$ egrep '<(base|script|link)' dist/index.html
    <base href="/" />
    <link rel="stylesheet" href="assets/vendor-d41d8cd98f00b204e9800998ecf8427e.css" integrity="sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU= sha512-z4PhNX7vuL3xVChQ1m2AB9Yg5AULVxXcg/SpIdNs6c5H0NE8XYXysP+DGNKHfuwvY7kxvUdBeoGlODJ6+SfaPg==" >
    <link rel="stylesheet" href="assets/some-project-d41d8cd98f00b204e9800998ecf8427e.css" integrity="sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU= sha512-z4PhNX7vuL3xVChQ1m2AB9Yg5AULVxXcg/SpIdNs6c5H0NE8XYXysP+DGNKHfuwvY7kxvUdBeoGlODJ6+SfaPg==" >
    <script src="assets/vendor-129091df2c2fd9a59128ca3f89f5df94.js"></script>
    <script src="assets/some-project-42e8649fd82acd6a88e48d2636532778.js" integrity="sha256-6TvYzxDdVjBWYDKUqUoLsF4dJZdmdyAYzyzExgz9lwU= sha512-fI8ZVnh1/8TgtXioLeB2CFEB5HSIXirWgkQnKjZPtXKA5b18czx9i2I4L7sYhrjcblYajBuBFVp13FMmSPe8IQ==" ></script>

~/test/some-project$ perl -i -pe "s/baseURL: '\/'/baseURL: '\/some-subdirectory\/'/" config/environment.js

~/test/some-project$ ember build --prod
[ . . . ]

~/test/some-project$ egrep '<(base|script|link)' dist/index.html
    <base href="/some-subdirectory/" />
    <link rel="stylesheet" href="assets/vendor-d41d8cd98f00b204e9800998ecf8427e.css">
    <link rel="stylesheet" href="assets/some-project-d41d8cd98f00b204e9800998ecf8427e.css">
    <script src="assets/vendor-129091df2c2fd9a59128ca3f89f5df94.js"></script>
    <script src="assets/some-project-42e8649fd82acd6a88e48d2636532778.js"></script>

... as far as I can tell, the problem is that broccoli-sri-hash grabs /some-subdirectory/ out of the <base href> tag in index.html and then inserts into the path for reading each asset file off of the file system, but /some-subdirectory/ isn't actually on the file system.

Hacking SRIHashAssets.prototype.getBaseHREF in broccoli-sri-hash to always return null so it doesn't try to insert it into the path seems to restore the integrity hashes.

Thanks for the project!

jonathanKingston commented 8 years ago

Hey @stefanpenner and @rwjblue

https://github.com/jonathanKingston/broccoli-sri-hash/blob/master/index.js#L70

Some questions:

To fix this we would probably have to another option.

jonathanKingston commented 8 years ago

Also @corpulentcoffee I'm a little lost why your vendor file doesn't have an integrity on it too.

corpulentcoffee commented 8 years ago

@jonathanKingston I noticed that too, but didn't investigate. I saw that there is some stuff around not doing checksums on input files that contained non-ASCII characters, so that might be it?

jonathanKingston commented 8 years ago

@corpulentcoffee are you on @2.0.0 ? paranoiaMode should always be off now that Chrome is behaving. (paranoiaMode is the stupid name I gave the disablement you are talking about)

corpulentcoffee commented 8 years ago

@jonathanKingston I did the original reproduction steps of this on the "stable" ember-cli@1.13.x, which specifies ember-cli-sri@1.2.0. Manually switching off the paranoiaCheck for broccoli-sri-hash allows the vendor file to be given an integrity hash, as does just upgrading to ember-cli-sri@2.0.0.

(I looked, and the minified Ember dependency has non-ASCII characters in it that get bundled into vendor.js, so that is probably what caused the loss of the integrity hash with ember-cli-sri@1.2.0. I probably should have used ember-cli@2.3.0-beta.1 for the reproduction steps, which does specify ember-cli-sri@2.0.0.)

Anyway, all said, setting the baseURL to something other than '/' breaks all integrity hashes on either version.

Cheers!

jonathanKingston commented 8 years ago

:+1: thanks for checking that seems the correct behaviour for those versions... most vendors have unicode in them (if not all of them).

Yeah I think the base check works when it's relative to the current project such as making testing code look at the parent directory... however manipulating the path to have additional path breaks it as it's looking in the wrong location (I bet a symlink works :D ).

I wonder if the solution is to change if (!relativePath[0] === '/') { return null; } to if (!relativePath === '/') { return null; } which covers the case of having child directories needing the parent but relative directories falling through to default checking.

@corpulentcoffee can't you change the path ember places into the scripts instead of using <base>?

corpulentcoffee commented 8 years ago

I'm not sure why the existing !relativePath[0] === '/' check works at all, since !relativePath[0] would return a boolean, which is then strictly compared to a string...?

But anyway, we're setting baseURL in config/environment.js because the application is being served on a hostname with other junk on it, so the application needs to be served from a subdirectory, so not only do we need to be able to tell the Ember router what everything is relative to (otherwise, the Ember router would try to route based on that subdirectory), but we also need those static assets to be served up relative to that subdirectory.

Setting the baseURL makes both of those things work (given that the index.html that ember-cli ships with includes the <base href> tag set to the baseURL from config/environment.js and given that baseURL gets combined with the router's own rootURL), but it does break the SRI add-on because broccoli-sri-hash is then looking in the wrong place for the asset files... up until now baseURL seemed like the correct place to say , "hey, this subdirectory is where the application is going to run and have its assets"... but there may be other ways to put this together?

jonathanKingston commented 8 years ago

I'm not sure why the existing !relativePath[0] === '/' check works at all, since !relativePath[0] would return a boolean, which is then strictly compared to a string...?

Yup always evaluates false. I think it should have been: !(relativePath[0] === '/') but I'm thinking relativePath !== '/'.

I can't really advise on if there are better ways really, I have only ever used Ember in the parent however it was only added for testing code which was causing issues. I thought baseURL solved all the use-cases of <base>.