gzip files before upload, fix header setting #310
Conversation
since Sprockets 3 no longer gzips assets for us (sstephenson/sprockets@d388ef7) , we can do it here (#304) prior to upload. Also fix the pre-detection of filename in upload_file so the cache_control and expires header get set properly on the gzipped files (#308).
improve Storage#compress_files to only compress files with extensions matching config.extensions_to_gzip. Add test for #compress_files
|
Just want to give my 👍 to this PR. It fixed gzip and cache control headers for me. |
|
👍 Any chance of this PR being merged any time soon? |
|
👍 👍 This works for my project too, could we have it merged please? |
|
💪🏽I'm now using this version instead of the production gem release. Hope this PR can be accepted so I don't have to continue supporting a non-official gem version. |
|
Since rails/sprockets#193 |
|
Yep, that's great if you are not on a CDN that gzips. Between that, and the fact that Cloudfront can now auto-gzip objects, and serve directly from the rails server origin, there is no reason to use this feature - or in fact to use asset_sync at all. Please read https://devcenter.heroku.com/articles/please-do-not-use-asset-sync We've removed asset_sync from production and are serving the complied assets directly through cloudfront. Cache-control headers and compression are working well, and deploy time is about 40% of what it used to be, with a lot less error-prone complexity and waste. Agreed with @schneems that the problems that asset_sync solves are no longer problems, or can be better solved in other ways. I'm going to close this PR. |
|
@sbeam on the heroku link about not using asset_sync, how do you sanely deal with dynamic assets such as user uploads, paperclip generated images, etc? If we should keep static assets on the web site (and the CDN pulls from it on-demand), while dynamic assets get pushed to S3/CDN as they are built, the ability to load these assets uniformly in the code is lost, since it will need two separate CDN root locations for these two types of assets. And it seems rails doesn't make it possible to refer to assets in two different places with its asset helpers either. |
|
@gsar asset sync isn't for user uploaded images, it is for putting your rails asset pipeline on S3. For serving user uploaded content you should do that directly through paperclip or s3 or however you're storing your files. You can also set up a separate CDN for these files. Asset helpers are for working with the asset pipeline, again not for working with user uploaded content. |
|
@schneems right, I wasn't referring to using asset_sync at all for pushing dynamic assets (and I currently already use paperclip). I was referring to the fallout from having to use two separate CDNs and the code needing to care about that (i.e. which type of asset goes on which CDN). I guess I could make the asset_host setting dynamic, based on the path where the content needs to be stored, or something. Just doesn't feel clean to me. |
|
Serving your own internal assets from the same source as user supplied assets seems unclean to me. Github does this, avatars are served from Site content is served from There are security concerns with serving user uploaded content from the same place as your apps assets. I would say the forced separation is a feature, not a bug. |
|
Access control of assets is an orthogonal concern, and it applies whether or not there is one CDN or two (or N). I don't see how forcing the separation of the root of the CDN somehow makes it possible to relax the security concerns. And you're embedding the assumption that all dynamic assets are like user uploads (i.e. "they are insecure") when they may not be--consider assets that need to be generated at run time by the system itself that aren't user-initiated or user-specific. My POV is that the added complexity of having to manage two CDNs instead of one is a cause for concern, especially for simple projects. What feels unclean to me is forcing a particular organization of assets on everyone. Anyhow, since this is bound to be an argument based on opinion, I will accept your position and just deal with it. Thanks for the prompt response. |
since Sprockets 3 no longer gzips assets for us
(sstephenson/sprockets@d388ef7)
, we can do it here (#304) prior to upload. Also fix the pre-detection of
filename in upload_file so the cache_control and expires header get set
properly on the gzipped files (#308).
/ht to @mirzali for the rake task patch that was modified for use here, and @wpiekutowski for the Cache-control header setting code.
(this also fixes 1 failing integration test and moves the content-encoding header detection to that test)