That looks perfect. Thanks Eric!
On Mon, Jun 7, 2010 at 11:45 PM, Eric Wong <[email protected]> wrote: > 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
