Augusto Becciu <[email protected]> wrote: > Hi Eric, > > Thanks for the ideas. > > Ended up adding some debugging code to unicorn that lead me to the > culprit of the problem. It turned out being a bug in the HttpParser.
Thank you Augusto, Your explanation was good so I'll just use it as a commit message and credit you as the author. See the patch below and let me know if everything looks alright before I push it out. I have a few small things to go over, but I'll be releasing 0.999.0 tonight (which I hope will be the last before we finally celebrate have a 1.0.0 release). >From 7d2295fa774d1c98dfbde2b09d93d58712253d24 Mon Sep 17 00:00:00 2001 From: Augusto Becciu <[email protected]> Date: Mon, 7 Jun 2010 23:02:43 -0300 Subject: [PATCH] http: ignore Version: header if explicitly set by client When Unicorn receives a request with a "Version" header, the HttpParser transforms it into "HTTP_VERSION". After that tries to add it to the request hash which already contains a "HTTP_VERSION" key with the actual http version of the request. So it tries to append the new value separated by a comma. But since the http version is a freezed constant, the TypeError exception is raised. According to the HTTP RFC (http://www.w3.org/Protocols/rfc2616/rfc2616-sec7.html#sec7.1) a "Version" header is valid. However, it's not supported in rack, since rack has a HTTP_VERSION env variable for the http version. So I think the easiest way to deal with this problem is to just ignore the header since it is extremely unusual. We were getting it from a crappy bot. ref: http://mid.gmane.org/[email protected] Acked-by: Eric Wong <[email protected]> --- ext/unicorn_http/unicorn_http.rl | 9 +++++++++ test/unit/test_http_parser_ng.rb | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-) diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index 264db68..aa23024 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -197,6 +197,15 @@ static void write_value(VALUE req, struct http_parser *hp, assert_frozen(f); } + /* + * ignore "Version" headers since they conflict with the HTTP_VERSION + * rack env variable. + */ + if (rb_str_cmp(f, g_http_version) == 0) { + hp->cont = Qnil; + return; + } + e = rb_hash_aref(req, f); if (NIL_P(e)) { hp->cont = rb_hash_aset(req, f, v); diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb index a067ac8..cb30f32 100644 --- a/test/unit/test_http_parser_ng.rb +++ b/test/unit/test_http_parser_ng.rb @@ -440,4 +440,24 @@ class HttpParserNgTest < Test::Unit::TestCase end end + def test_ignore_version_header + http = "GET / HTTP/1.1\r\nVersion: hello\r\n\r\n" + req = {} + assert_equal req, @parser.headers(req, http) + assert_equal '', http + expect = { + "SERVER_NAME" => "localhost", + "rack.url_scheme" => "http", + "REQUEST_PATH" => "/", + "SERVER_PROTOCOL" => "HTTP/1.1", + "PATH_INFO" => "/", + "HTTP_VERSION" => "HTTP/1.1", + "REQUEST_URI" => "/", + "SERVER_PORT" => "80", + "REQUEST_METHOD" => "GET", + "QUERY_STRING" => "" + } + assert_equal expect, req + end + end -- Eric Wong _______________________________________________ Unicorn mailing list - [email protected] http://rubyforge.org/mailman/listinfo/mongrel-unicorn Do not quote signatures (like this one) or top post when replying
