Skip to content
This repository

ActionDispatch::Static serves precompressed assets #8669

Closed
wants to merge 2 commits into from

9 participants

Jonathan Baudanza Pete Keen Andrew White Jacob Elder Ellis Berner Eliot Sykes Justin Searls James S Richard Schneeman
Jonathan Baudanza

Sprockets currently generates gzipped javascript and css files next to their uncompressed counterparts. However, to take advantage of this you need to use an nginx extension or something similar.

This patch will allow ActionDispatch::Static to serve up these gzipped files when appropriate.

For example, this request:

curl -H "Accept-Encoding: gzip" http://localhost:3000/assets/application.js

Will look for application.js.gz on disk and respond with:

 HTTP/1.1 200 OK
 Content-Type: application/javascript
 Content-Encoding: gzip
 Content-Length: 1234
 More-Headers...

 <compressed data>

This is particularly helpful for environments like Heroku, that depend on ActionDispatch::Static to serve static assets. Currently, the only way to serve gzipped assets in this environment is to install Rack::Deflate ahead of ActionDispatch::Static. This is undesirable, because Rack::Deflate compresses everything, including compressed assets like images and swfs.

jbaudanza added some commits
Jonathan Baudanza jbaudanza Removed ActionDispatch::FileHandler abstraction.
This class was originally added to support multiple file roots.
Since this feature no longer exists, the extra abstraction is unnecessary.
bdcee3e
Jonathan Baudanza jbaudanza ActionDispatch::Static will serve pre-gziped assets a600cff
Pete Keen

Looks useful. +1

Andrew White
Owner

Agreed that the functionality is useful but I'm not sure that we should be building a slow, poorly specified web server as part of Rails - I think Rack::Static should be patched to handle this kind of thing and ActionDispatch::Static be a shim to that to provide backwards compatibility.

@josevalim @jeremy @tenderlove wdyt?

Jonathan Baudanza

Thanks for your feedback @pixeltrix. Your suggestion sounds reasonable, and I'd be happy to resubmit this as a patch to Rack::Static.

But if the Rack guys don't want this, I do think this should be handled by Rails somewhere. Given that we've already gone down the path of generating these gzipped assets via the asset compiler, it seems reasonable that rails should be capable of serving these files up to the client somehow.

Jacob Elder

+1

Ellis Berner

Yes +1

Andrew White
Owner

@maletor there's an open PR (rack/rack#479) that implements this functionality which seems like it will be eventually accepted once the details are ironed out - probably better to post there if you're interested in it.

Jonathan Baudanza

I don't think this is going to make it into Rack. Might we reconsider adding this to Rails?

Eliot Sykes

Another option for serving gzipped assets until its available in Rails core: https://gist.github.com/eliotsykes/6049536

Jonathan Baudanza

@pixeltrix Could we reconsider adding this to Rails? There seems to be a demand for it, and there doesn't seem to be any traction getting this into Rack.

Jonathan Baudanza

@guilleiguaran Could we consider reopening this?

I don't think this is making it into Rack, as there hasn't been a response there in 10 months.

This functionality feels more at home in Rails, since the asset pipeline is generating these files. Rack seems more like an interface library.

I'd love to have this rolled out for all the Rails apps on Heroku that are unnecessarily serving uncompressed javascript and css. CC @schneems

This is also much more graceful than using Rack::Deflate on your static assets, which will blindly recompress jpegs and pngs.

Eliot Sykes

Both @jbaudanza and I maintain separate gems that offer this functionality. It'd be a kindness to us both to know the Rails core team's direction on whether serving .gz assets is likely to be Rails' responsibility in the near future.

If gz serving is not going to become Rails' responsibility, it may be worth removing the gz creation from Rails entirely.

For developers new to Rails, it's a little disconnected to have the asset pipeline create .gz assets and then not use them. I suspect this will be burning a fair few folk on PaaSs like Heroku (where having a server like nginx handle asset requests is uncommon) and degrading performance for their apps' end-users.

Another option might be to modify the asset precompilation step to give clarification to developers who haven't stumbled upon this intricacy of the asset pipeline and give them a gentle nudge toward improving the performance of their app:

  1. Make gz asset creation a config option that is off by default, or
  2. Add an info message to the console output with something like "Gzip compressed assets have been created. These will not be served by your app unless you: a) configure your web server (e.g. nginx, Apache) to serve them or b) use a 3rd party gem or c) write your own middleware
Andrew White
Owner

@eliotsykes as I've said previously I'm loathe to try and build a poorly specified web server using ActionDispatch::Static - we don't recommend it's use in production environments and the only reason it's enabled on Heroku is so that single dyno test applications don't have to go through the hassle of setting up external asset hosts.

As for disabling gz asset creation by default I don't think that forcing everyone that doesn't use Heroku to have to manually specify an extra option is a good choice. I don't think the warning would be much help either as it would just get lost in the noise of cap deploy.

I still think getting it integrated into Rack is the best option - from re-reading the discussion it seems as though the sticking points are implementation details rather than any philosophical objection.

Justin Searls
searls commented

:+1:

I understand @pixeltrix's reasoning, but seeing as CDNs like Cloudfront require assets to be compressed by the origin server (which, for most Heroku applications, is Heroku) in order to serve gzipped assets on the CDN network, then I think there's a strong argument to be made for at least an opt-in configuration to serve up static assets when they're available.

I agree that nobody wants ActionDispatch::Static to pretend to be a production grade static asset server, but in this particular Heroku + custom origin-backed Cloudfront configuration (and perhaps many others, scant few developers even seriously look into this), behavior like this is actually required to serve gzipped assets to end-users in production at all.

James S

:thumbsup: I'm with @searls.

Also, let people make their own poor production decisions if need be. In some cases, ActionDispatch::Static is the appropriate production solution for some projects.

Richard Schneeman
Collaborator

If you're running a production site you should be using a CDN. period. ActionDispatch::Static actually slows down Heroku apps, as all requests will stat the disk for EVERY request, this is slow and bad. If you're serving behind a CDN then you only need to serve your assets once per every invalidation.

I would like to see an approach that does not put a slow tax on every request just because you want to serve assets. One approach might be mounting a sprockets related middleware only on paths that match /assets/and then we know it's an asset, and we know it will have a static precompiled file and we can do the right thing.

Pushing this functionality into Rack seemed like a good idea, but if we do that we're adding an even slower tax to those that want to run their app without being tied down by an extra non-ruby webserver. Since we're in Rails we can make a non-general purpose tool that works really well for our needs, serving assets. I have many thoughts about this, but i'm working on putting them into something coherent.

Thanks for chiming in here, i think this is a valid concern that we should be thinking about. I'm still :-1: on this exact implementation, but we should talk about a way to get gzipped assets (and non-gzipped assets) without adding a slow tax to every request.

Justin Searls
searls commented
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Jan 01, 2013
Jonathan Baudanza jbaudanza Removed ActionDispatch::FileHandler abstraction.
This class was originally added to support multiple file roots.
Since this feature no longer exists, the extra abstraction is unnecessary.
bdcee3e
Jonathan Baudanza jbaudanza ActionDispatch::Static will serve pre-gziped assets a600cff
This page is out of date. Refresh to see the latest.
72 actionpack/lib/action_dispatch/middleware/static.rb
... ... @@ -1,11 +1,13 @@
1 1 require 'rack/utils'
  2 +require 'rack/mime'
2 3 require 'active_support/core_ext/uri'
3 4
4 5 module ActionDispatch
5   - class FileHandler
6   - def initialize(root, cache_control)
  6 + class Static
  7 + def initialize(app, root, cache_control=nil)
  8 + @app = app
7 9 @root = root.chomp('/')
8   - @compiled_root = /^#{Regexp.escape(root)}/
  10 + @compiled_root = /^#{Regexp.escape(@root)}/
9 11 @file_server = ::Rack::File.new(@root, cache_control)
10 12 end
11 13
@@ -15,16 +17,46 @@ def match?(path)
15 17 full_path = path.empty? ? @root : File.join(@root, escape_glob_chars(unescape_path(path)))
16 18 paths = "#{full_path}#{ext}"
17 19
18   - matches = Dir[paths]
19   - match = matches.detect { |m| File.file?(m) }
20   - if match
21   - match.sub!(@compiled_root, '')
22   - ::Rack::Utils.escape(match)
23   - end
  20 + Dir[paths].detect { |m| File.file?(m) }
24 21 end
25 22
26 23 def call(env)
27   - @file_server.call(env)
  24 + case env['REQUEST_METHOD']
  25 + when 'GET', 'HEAD'
  26 + path = env['PATH_INFO'].chomp('/')
  27 + if filename = match?(path)
  28 + compressed_filename = "#{filename}.gz"
  29 + compressed_exists = File.file?(compressed_filename)
  30 +
  31 + wants_compressed = !!(env['HTTP_ACCEPT_ENCODING'] =~ /\bgzip\b/)
  32 +
  33 + if wants_compressed && compressed_exists
  34 + path = compressed_filename
  35 + else
  36 + path = filename
  37 + end
  38 +
  39 + path.sub!(@compiled_root, '')
  40 + env["PATH_INFO"] = ::Rack::Utils.escape(path)
  41 + status, headers, body = @file_server.call(env)
  42 +
  43 + if compressed_exists
  44 + headers['Vary'] = 'Accept-Encoding'
  45 +
  46 + if wants_compressed
  47 + headers['Content-Encoding'] = 'gzip'
  48 + # Rack::File will always return 'application/gzip', so we need
  49 + # to set the correct mime header here
  50 + headers['Content-Type'] = Rack::Mime.mime_type(
  51 + ::File.extname(filename), 'text/plain')
  52 + end
  53 + end
  54 +
  55 + return [status, headers, body]
  56 + end
  57 + end
  58 +
  59 + @app.call(env)
28 60 end
29 61
30 62 def ext
@@ -43,24 +75,4 @@ def escape_glob_chars(path)
43 75 path.gsub(/[*?{}\[\]]/, "\\\\\\&")
44 76 end
45 77 end
46   -
47   - class Static
48   - def initialize(app, path, cache_control=nil)
49   - @app = app
50   - @file_handler = FileHandler.new(path, cache_control)
51   - end
52   -
53   - def call(env)
54   - case env['REQUEST_METHOD']
55   - when 'GET', 'HEAD'
56   - path = env['PATH_INFO'].chomp('/')
57   - if match = @file_handler.match?(path)
58   - env["PATH_INFO"] = match
59   - return @file_handler.call(env)
60   - end
61   - end
62   -
63   - @app.call(env)
64   - end
65   - end
66 78 end
20 actionpack/test/dispatch/static_test.rb
@@ -104,6 +104,22 @@ def test_serves_static_file_with_at_symbol_in_filename
104 104 end
105 105 end
106 106
  107 + def test_serves_uncompressed_file
  108 + response = get('/compressed.html')
  109 + assert_html "Hello, static file", response
  110 + assert_equal "Accept-Encoding", response.headers['Vary']
  111 + assert_nil response.headers['Content-Encoding']
  112 + end
  113 +
  114 + def test_serves_compressed_file
  115 + response = get('/compressed.html', 'HTTP_ACCEPT_ENCODING' => 'gzip')
  116 + gz = Zlib::GzipReader.new(StringIO.new(response.body))
  117 + assert_equal "Hello, static file", gz.read
  118 + assert_equal "Accept-Encoding", response.headers['Vary']
  119 + assert_equal "gzip", response.headers['Content-Encoding']
  120 + assert_equal "text/html", response.headers['Content-Type']
  121 + end
  122 +
107 123 # Windows doesn't allow \ / : * ? " < > | in filenames
108 124 unless RbConfig::CONFIG['host_os'] =~ /mswin|mingw/
109 125 def test_serves_static_file_with_colon
@@ -128,8 +144,8 @@ def assert_html(body, response)
128 144 assert_equal "text/html", response.headers["Content-Type"]
129 145 end
130 146
131   - def get(path)
132   - Rack::MockRequest.new(@app).request("GET", path)
  147 + def get(path, opts={})
  148 + Rack::MockRequest.new(@app).request("GET", path, opts)
133 149 end
134 150
135 151 def with_static_file(file)
1  actionpack/test/fixtures/public/compressed.html
... ... @@ -0,0 +1 @@
  1 +Hello, static file
BIN  actionpack/test/fixtures/public/compressed.html.gz
Binary file not shown

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.