Browse files

Eliminate Rack::File headers deprecation warning

See http://intertwingly.net/projects/AWDwR4/checkdepot/section-6.1.html
rake test produces:
   "Rack::File headers parameter replaces cache_control after Rack 1.5."

Despite what the message says, it appears that the hearders parameter change
will be effective as of Rack 1.5:

https://github.com/rack/rack/blob/rack-1.4/lib/rack/file.rb#L24
https://github.com/rack/rack/blob/master/lib/rack/file.rb#L24
  • Loading branch information...
1 parent c67005f commit 9cc82b77196d21a5c7021f6dca59ab9b2b158a45 @rubys rubys committed Jan 8, 2013
Showing with 1 addition and 1 deletion.
  1. +1 −1 actionpack/lib/action_dispatch/middleware/static.rb
View
2 actionpack/lib/action_dispatch/middleware/static.rb
@@ -6,7 +6,7 @@ class FileHandler
def initialize(root, cache_control)
@root = root.chomp('/')
@compiled_root = /^#{Regexp.escape(root)}/
- @file_server = ::Rack::File.new(@root, cache_control)
+ @file_server = ::Rack::File.new(@root, 'Cache-Control' => cache_control)
end
def match?(path)

7 comments on commit 9cc82b7

@dn

@rubys

I think this introduced a bug.

[2013-01-10 20:36:04] ERROR NoMethodError: undefined method `split' for nil:NilClass
gems/rack-1.4.3/lib/rack/handler/webrick.rb:69:in `block in service'
gems/rack-1.4.3/lib/rack/utils.rb:387:in `block in each'

https://github.com/rack/rack/blob/master/lib/rack/handler/webrick.rb#L69

This happens because of:

{"Last-Modified"=>"Thu, 10 Jan 2013 19:17:43 GMT", "Content-Type"=>"text/html", "Cache-Control"=>nil, "Content-Length"=>"82"}

Reproduce:

touch public/foo.html
curl http://localhost:3000/foo.html

By default cache_control is nil in development/production.

@carlosantoniodasilva
Ruby on Rails member

@dn it'd be great if you could produce a test case for us to fix it.

@dn

Would love to, I can only provide you with a simple example how to reproduce it.

touch public/foo.html
rails s
curl http://localhost:3000/foo.html

In https://github.com/rails/rails/blob/master/actionpack/test/dispatch/static_test.rb#L148 the cache_control is set, but even if I would nil it it will still not trigger the error, because in the test Rack::MockRequest https://github.com/rails/rails/blob/master/actionpack/test/dispatch/static_test.rb#L132 is used and because of that https://github.com/rack/rack/blob/master/lib/rack/handler/webrick.rb#L69 is never reached and the bug is not visible.

Can you reproduce the error - if so I would keep investigating how to write a simple test for it?

@dn

Maybe this explain it a bit further https://github.com/rack/rack/blob/master/lib/rack/lint.rb#L451. Rack does not expect a nil value in the header, but "Cache-Control"=>nil for a static file.

rails s
curl http://localhost:3000/robots.txt

Will also trigger the error. I'll try to write a test/fix for it.

@spastorino
Ruby on Rails member

@dn @carlosantoniodasilva was this issue addressed?

@spastorino
Ruby on Rails member

Saw that it's fixed here #8907 thanks

@carlosantoniodasilva
Ruby on Rails member

😄

Please sign in to comment.