AlexCuse / SquishIt

Lets you *easily* bundle some css and javascript! Check out the Google group if you have questions!
http://groups.google.com/group/squishit
MIT License
4 stars 1 forks source link

Content/Mime Type, or BOM, of "squished" files #26

Open dazbradbury opened 3 years ago

dazbradbury commented 3 years ago

We've experienced an intermittent bug that is, sadly, non-trivial to reproduce and only seems to impact "squished" CSS.

Essentially Chrome is caching one of our bundled css assets, produced by Squishit. The cached file suddenly stops being UTF-8, and is encoded as Windows-1252 instead.

As far as I can tell, SquishIt isn't modifying BOM or doing anything surprising. However, Squishit.S3 also doesn't have a way to ensure the content type is set to UTF-8 when uploading to S3, and there seems to be little mention of content / mime types generally as far as I can tell.

This bug feels very much to be a Chrome issue - however, I can't find a way to force the content / mime type of squishit bundled files to try and forcibly fix the issue.

Has anyone run into this issue before? Is there a way to force the content /mime type, or BOM, of bundled files?

AlexCuse commented 3 years ago

I have not seen this but also haven't been using .net much if at all lately. I wonder if there is some kind of file size threshold that changes the behavior?

You can use the .WithHeaders method on S3Renderer to set s3 headers for the object, I believe these could be used to tell the browser the file is UTF-8?

Let me know how you get on this is an interesting one.

dazbradbury commented 3 years ago

I have not seen this but also haven't been using .net much if at all lately. I wonder if there is some kind of file size threshold that changes the behavior?

Thanks for the speedy response - our files are reasonably consistent in size, and this behaviour very much intermittent. The reason it's being highlighted is due to the fontawesome CSS file - which if encoded as Windows-1252 means all the icons no longer appear. I suspect most CSS files may well just about work regardless of encoding, which might be why this bug has gone unnoticed!

This has made me think I could/should just pull out the affected CSS file and serve it separately / in a non-bundled way though - so thanks for the thought. :-)

You can use the .WithHeaders method on S3Renderer to set s3 headers for the object, I believe these could be used to tell the browser the file is UTF-8?

I'd love to be able to do that, but I think the correct header would be:

Content-Type: text/css; charset=utf-8

However, if I set that at the s3Renderer level, it would apply to javascript files also, which would be incorrect. I'm not aware of a way to define the charset / mime type separately from the content type. I believe if I set Content-Encoding to utf-8 this would stop s3/cloudfront being able to serve it compressed (although I've not tested this). I'm also not aware of setting a header based on the file type with SquishIt.

AlexCuse commented 3 years ago

You wouldn't want to upend your whole pipeline of course but there isn't a ton of cost involved in instantiating multiple renderers (eg one per file type or even per bundle you define). I'm not sure about the cloudfront / s3 interaction but I would hope cloudfront propagates headers from the origin in a reasonable way, especially from their own origin.

What version of the AWS SDK are you using? I may have some bandwidth to look closer in the next couple weeks.

dazbradbury commented 3 years ago

What version of the AWS SDK are you using?

3.5.7.6

You wouldn't want to upend your whole pipeline of course but there isn't a ton of cost involved in instantiating multiple renderers (eg one per file type or even per bundle you define). I'm not sure about the cloudfront / s3 interaction but I would hope cloudfront propagates headers from the origin in a reasonable way, especially from their own origin.

Yep - I guess that's the best solution. We had been using a single definition, with a default renderer, to keep things neat. But I guess we could equally use a CSS and JS specific bundler, makes sense.

I've actually already ripped out the fontawesome CSS so it's not bundled internally (and treated like the external library it should be), but it's good to know if we ran into this with our internal CSS there would be a solution.

To give you a little more information, it seems the minifier was replacing the ascii escape codes with actual characters, which exacerbated the issue. So for example:

fa-heart:before{content:"\f004"}

Was being minified into:

fa-heart:before{content:""}

I didn't check if the YUI / another minifier would behave differently before I ripped out the CSS though.

AlexCuse commented 3 years ago

Oh man. The minifiers have always been hit or miss. If it is just the one file, serving separately is probably a good way to go. You could probably even load it from someone else's server if you wanted to.

I feel like there was an issue or PR around behavior like this years ago. It may have been just to ensure that when we rewrite paths in CSS files we dont do the same thing you are seeing from the minifier but not sure. I'll poke around in history and see if I can find it just in case.

Glad you got something working.

AlexCuse commented 3 years ago

I found this for java, its a bit dated but so is SquishIt.S3 at this point:

https://stackoverflow.com/a/30856191

I would guess there is a similar facility in .net. Maybe the ability to inject a class or function to calculate metadata from rendering context would be a good way to go at it.

dazbradbury commented 3 years ago

Maybe the ability to inject a class or function to calculate metadata from rendering context would be a good way to go at it.

That would be super powerful I think.

its a bit dated but so is SquishIt.S3 at this point

Yeh I guess you have to make a call on where to spend time. From our perspective we'll need to switch to Net5 soon, so we'll need to make a call on how to migrate our Squishit integration more generally. Are you looking for support on that?

AlexCuse commented 3 years ago

I tried to split out the platform details into its own package (SquishIt.AspNet) with an eye towards that kind of stuff but I am pretty out of the loop on the state of asp.net. Haven't done any serious .net coding in about a year and a half, and for a few years before that it all APIs and JS front ends so I didn't get a great picture of the "view side" changes and how it would fit in (if at all)

I would like to bring it up to date somehow and it seems like .net 5 may be the release to do it on. But it doesn't come to mind that often because the stuff I am working on now is so different. As you go through the process would welcome any PRs towards it. Or more realistically any observations about how you think it should / could work.

dazbradbury commented 3 years ago

OK thanks - when we start on it I'll make sure to update you, and if possible/needed, put together a proof of concept PR.

AlexCuse commented 3 years ago

That would be awesome. I still know this code very well but I am kind of out of the loop on the .net side so anything that helps shine a light on that would be really helpful.