Read error: #<TypeError: can't modify frozen string> raised from HttpParser

Augusto Becciu augusto at jadedpixel.com
Mon Jun 7 22:02:43 EDT 2010


On Thu, Jun 3, 2010 at 8:40 PM, Eric Wong <normalperson at yhbt.net> wrote:
> Augusto Becciu <augusto at jadedpixel.com> wrote:
>> On Wed, Jun 2, 2010 at 6:38 PM, Eric Wong <normalperson at yhbt.net> wrote:
>> > Augusto Becciu <augusto at jadedpixel.com> wrote:
>> >> On Wed, Jun 2, 2010 at 5:25 PM, Eric Wong <normalperson at yhbt.net> wrote:
>> >> > Augusto Becciu <augusto at jadedpixel.com> wrote:
>> >> >> Hey guys,
>> >> >>
>> >> >> Started running unicorn in a production server like two weeks ago.
>> >> >> It's been running smoothly, but looking at the logs found 44
>> >> >> exceptions like this:
>> >> >>
>> >> >> E, [2010-06-02T16:17:15.117071 #22680] ERROR -- : Read error:
>> >> >> #<TypeError: can't modify frozen string>
>> >> >> E, [2010-06-02T16:17:15.117270 #22680] ERROR -- :
>> >> >> /usr/lib/ruby/gems/1.8/gems/unicorn-0.99.0/lib/unicorn/http_request.rb:59:in
>> >> >> `headers'
>> >> >
>> >> > <snip>
>> >> >
>> >> >> Ruby version: ruby 1.8.7 (2009-12-24 patchlevel 248) [i686-linux],
>> >> >> MBARI 0x8770, Ruby Enterprise Edition 2010.01
>
> <snip>
>
>> >> There's no way our application could be freezing that constant, at
>> >> least not intentionally.
>
> <snip>
>
>> Thanks Eric! Unfortunately after completely disabling RPM, we keep
>> getting that error. :(
>>
>> What else could it be?
>
> Any details about your application you're allowed to share can help us,
> especially a list of library/gem dependencies and versions.
>
> I would grep through your gems and vendor directories to ensure there's
> nothing freezing objects it shouldn't freeze.  It could also be a buggy
> C extension corrupting memory somewhere, too...
>
> This isn't happening for all your worker processes, is it?
>
> If your application logs show which PID handled each request, you can
> join the PIDs from the application logs with PIDs in the error logs
> generated by Unicorn.  That lets you narrow down the possible requests
> that could trigger the errors and help you reproduce the problem sooner.
>
> This is one of my favorite features of one-request-per-process model
> is that you can easily narrow down which request is triggering a
> particular bug without worrying about race conditions from other
> requests.
>
> This is certainly the first time I've heard of this problem, so I think
> it's specific to something in your application/environment.
>
> --
> Eric Wong
>

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.

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.

Below is a proposed patch.

Thanks a lot for your help!
Augusto


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


More information about the mongrel-unicorn mailing list