Owen Ou <[email protected]> wrote: > We recently upgraded to Unicorn 5.0 but getting the following error: > > [2015-11-16T14:54:16.943652 #19838] ERROR -- : app error: undefined > method `include?' for nil:NilClass (NoMethodError) > > E, [2015-11-16T14:54:16.943712 #19838] ERROR -- : > /home/api/vendor/bundle/ruby/2.2.0/gems/unicorn-5.0.0/lib/unicorn/http_response.rb:40:in > `block in http_response_write'
<snip> > The error came from this commit: > https://github.com/defunkt/unicorn/commit/fb2f10e1d7a72e6787720003342a21f11b879614. > And specifically the line of `if value =~ /\n/` is changed to `if > value.include?("\n".freeze)`. Apparently `value` can be nil which > caused our issue. It should be an easy fix. Yes, easy, I reverted that hunk in the original change since it's the easiest to verify as correct. It's unfortunate, change to have to make, though. Thanks, will release 5.0.1 in less than a day... ---------------------8<----------------------- Subject: [PATCH] http_response: allow nil values in response headers This blatantly violates Rack SPEC, but we've had this bug since March 2009[1]. Thus, we cannot expect all existing applications and middlewares to fix this bug and will probably have to support it forever. Unfortunately, supporting this bug contributes to application server lock-in, but at least we'll document it as such. [1] commit 1835c9e2e12e6674b52dd80e4598cad9c4ea1e84 ("HttpResponse: speed up non-multivalue headers") Reported-by: Owen Ou <[email protected]> Ref: <CAO47=rJa=zrcln_xm4v2chpr6c0uswafc_omyfeh+basxho...@mail.gmail.com> --- Side note: I don't intend to port this change to less-popular servers I maintain. This bug is yet another example of why monoculture or even any sort of majority adoption hurts an ecosystem. lib/unicorn/http_response.rb | 2 +- test/unit/test_response.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb index c1aa738..7b446c2 100644 --- a/lib/unicorn/http_response.rb +++ b/lib/unicorn/http_response.rb @@ -37,7 +37,7 @@ def http_response_write(socket, status, headers, body, # key in Rack < 1.5 hijack = value else - if value.include?("\n".freeze) + if value =~ /\n/ # avoiding blank, key-only cookies with /\n+/ value.split(/\n+/).each { |v| buf << "#{key}: #{v}\r\n" } else diff --git a/test/unit/test_response.rb b/test/unit/test_response.rb index 0b14d59..fbe433f 100644 --- a/test/unit/test_response.rb +++ b/test/unit/test_response.rb @@ -33,6 +33,15 @@ def test_response_headers assert out.length > 0, "output didn't have data" end + # ref: <CAO47=rJa=zrcln_xm4v2chpr6c0uswafc_omyfeh+basxho...@mail.gmail.com> + def test_response_header_broken_nil + out = StringIO.new + http_response_write(out, 200, {"Nil" => nil}, %w(hysterical raisin)) + assert ! out.closed? + + assert_match %r{^Nil: \r\n}sm, out.string, 'nil accepted' + end + def test_response_string_status out = StringIO.new http_response_write(out,'200', {}, []) -- EW -- unsubscribe: [email protected] archive: http://bogomips.org/unicorn-public/
