Aspnetcore: Consider JavaScript files as binary in .gitattributes

[wenz]

Is your feature request related to a problem? Please describe.

Very often in this repository, the jQuery JavaScript library is referenced in HTML files with SRI (Subresource Integrity), meaning that a hash of the file is provided as an attribute. Therefore it is crucial that JavaScript files are not modified when pushed to the repository. Git settings like core.autocrlf (defaults to true under Git tools for Windows) can therefore cause issues that are not immediately noted. The more SRI we use, the more often this problem might occur.

Describe the solution you'd like

If *.js files would be considered as binary in ´.gitattributes´, they should not be altered when being pushed to the repository.

Additional context

Suggested fix: add *.js binary in .gitattributes

[TimBo93]

I would like to note that autogenerated *.map files should also be considered as binary files.

[javiercn]

@wenz thanks for contacting us.

If I'm not wrong, that will cause the JS files to not display correctly in things like git diff and so on. So it is unlikely that we do this by default, but you can add it to the project yourself.

@SteveSandersonMS do you have any thoughts?

[SteveSandersonMS]

@wenz Can you give a specific set of repro steps showing what set of actions lead to a problem? I'm not sure how Git's CRLF conversion is affecting your workflow within this repo.

It sounds like you're implying that you compute a JS file's hash locally for some reason, and insert that hash into a PR into this repo, then the hash turns out to be wrong because CRLF conversion means that other people get a different representation of the JS file. If this is the case, what are the circumstances where we have file hashes hardcoded inside this repo?

[TimBo93]

@SteveSandersonMS it belongs to SRI please have a look at #20356

[SteveSandersonMS]

I see, thanks for clarifying. So we do hard-code hashes in our sources :) In that case I agree it's problematic to allow CRLF conversion for those files.

However like @javiercn points out, we wouldn't want to set .gitattributes to treat every single .js file in the repo as binary, since that would break our normal workflows for seeing diffs for .js files that we do actually edit. Instead, I suggest there should be more targeted .gitattributes rules pointing just to the exact problematic files (or perhaps narrow this rule down to files called jquery.js anywhere in the repo).

[wenz]

@javiercn good point with git diff, didn't think about that. Maybe *.js eol=lf might work?

@SteveSandersonMS here is what happened: while working on https://github.com/dotnet/aspnetcore/pull/20356, I had a relatively new dev machine with the Windows Git tools installed with default settings, which include core.autocrlf=true. As part of the PR, I updated the jQuery versions that were part of the repository (within several of the sample applications).
Thanks to my Git settings (and me ignoring warnings), the line endings were converted while pushing. The files in the repository would still work per se (JavaScript does not care whether it's CRLF or LF), but the JavaScript files are referenced in the HTML files with their hash values provided (Subresource Integrity - SRI). Since the files were automatically changed during the push, the hash values did not match any more. The only reason this was noticed immediately was because @anurse was validating the files to avoid me uploading something nasty ;-)

It's a hard to spot issue, and of course technically my fault, but I may not be the last person to fall into this trap. If we use the SRI feature more often, this is more likely to occur.

Maybe indeed *.js eol=lf might be a good compromise?

[SteveSandersonMS]

I'm fine either way with *.js eol=lf (assuming that works; I haven't done that before) or having a rule that matches the jQuery files specifically, if those are the only ones likely to trigger this.

[wenz]

ah, our replies overlapped ;-) Yes, just "hardcoding" jquery.js and jquery.min.js into .gitattributes might be even better since it only affects those libraries where we are actually using SRI currently. Probably jquery*.js eol=lf is good enough (subject to verifying that it works)?

[TimBo93]

@SteveSandersonMS i would recommend that the (jquery) map files are treated as binary since they are autogenerated and do not add any value to the diff. Like here https://github.com/dotnet/aspnetcore/pull/20356/commits/880bd19270b240a166921b86b90cf0800e7061ff. These files can also be addressed by rule or hardcoded

[wenz]

Verified that the .gitattributes modifications indeed work (test repo: https://github.com/wenz/crlf-test - my client had core.autocrlf set to true); created a pull request for your consideration.

[pranavkm]

Fixed in https://github.com/dotnet/aspnetcore/commit/d2f3d80c19d93e5d1cd0a12b3194478f7b81bca8