304 response should not include Content-Type header

[eagletmt]

Rack::Lint raises an error saying "Content-Type header found in 304
response, not allowed".

[arthurnn]

I dont think the comment is necessary here, as the if already states that statement .

[eagletmt]

OK, I removed the trivial comment and force-pushed.

[arthurnn]

As the response status code is 304, I wonder if we could short-cut this middleware altogether.
cc @jeremy

[rafaelfranca]

Yeah, I think we should skip most of the work of this middleware if 304

[pixeltrix]

@rafaelfranca yes, since Rack::File just returns an empty triplet. @eagletmt can you rewrite call so that all of the response headers are only added if it's not 304, thanks.

Of course it'd be much better if this lived in Rack::File itself where it should be 😢

[wyaeld]

When did this behaviour in either rack or rails change?

[eagletmt]

@arthurnn @rafaelfranca @pixeltrix I'm sorry but I couldn't figure out what to do.
Content-Encoding header must be set because FileHandler returns the content of /path/to/foo.js.gz in response to /path/to/foo.js .
What kind of procedure can I skip or short-cut here?

[arthurnn]

As far as I know, if the status code is 304, you wont need to set any header nor a body in the response. So you could return it early.

[eagletmt]

Ah, I get it. I modified to skip Content-Encoding and Vary header.
Thank you for your advice.

[arthurnn]

thanks @eagletmt

[adrianpike]

@wyaeld I actually just ran into this bug! #16466 is where it started, looks like it went out in 4.2.

Looks like I get to live against master for a little bit. :)

[wyaeld]

@adrianpike thanks for the confirmation. We took the lesser evil temporarily of just including middleware/static.rb from master in the app, which we'll rip out once its fixed. That works fine for now.

[arthurnn]

Added a changelog entry for this b6b0884 .
And I backport it to 4-2-stable : 061b487