fix sendfile

[ahorek]

body.respond_to?(:to_ary) was returning false, but after this change, it returns true 72959eb

now the body is wrapped to a proxy here
https://github.com/rack/rack/blob/master/lib/rack/content_length.rb#L23

the wrapped body doesn't respond_to :to_path anymore
https://github.com/rack/rack/blob/master/lib/rack/sendfile.rb#L114

and the final response doesn't contain 'X-Sendfile' header as before
https://github.com/rack/rack/blob/master/lib/rack/sendfile.rb#L129

this change fixes the problem, but I've absolutely no idea if it's correct or not :)
I have to investigate more what's happening, is it a regression or is it intentional?

[ioquatix]

The first thing we need to do here is make a failing spec, can you have a go at implementing that?

[ahorek]

it turns out it's a middleware ordering problem

this line caused the failing behavior

config.middleware.insert_after Rack::Sendfile, Rack::ContentLength

Rack::ContentLength middleware discards the original body, so using Rack::Sendfile after that have no effect

unfortunately, there're no tests for combination of multiple middlewares in rack, but there's one in rails
https://github.com/rails/rails/blob/b2eb1d1c55a59fee1e6c4cba7030d8ceb524267c/railties/test/application/middleware/sendfile_test.rb#L36

If I read the documentation correctly Rack::Sendfile should be called at the top of the stack. So it's an application problem.

[ahorek]

this change fixes the problem

config.middleware.insert_before Rack::Sendfile, Rack::ContentLength

@ioquatix could you confirm, that the order of Rack::Sendfile and Rack::ContentLength should matter?

[ioquatix]

It matters.

But I also think the ContentLength middleware is junk and should not be needed.

Maybe we can modify ContentLength to not clobber SendFile or fix SendFile to include file size (if it doesn’t already) so as to not require buffering to compute content length.