From ac44225fb21e0dbc3a716cec3ea3133a0b2435a0 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 12 Apr 2009 21:52:48 -0700 Subject: http11: cleanup+safer rack.url_scheme handling Avoid using strcmp() since it could break badly if Ruby ever stopped null-terminating strings C-style. We're also freezing "http" as a global. Rack does not explicitly permit nor deny this, and Mongrel has always used frozen strings as hash values in other places. --- ext/unicorn/http11/http11.c | 15 ++++++++------- test/unit/test_request.rb | 12 +++++++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/ext/unicorn/http11/http11.c b/ext/unicorn/http11/http11.c index a262d18..8d2e2d7 100644 --- a/ext/unicorn/http11/http11.c +++ b/ext/unicorn/http11/http11.c @@ -41,6 +41,7 @@ static VALUE global_http_host; static VALUE global_http_x_forwarded_proto; static VALUE global_port_80; static VALUE global_localhost; +static VALUE global_http; /** Defines common length and error messages for input length validation. */ #define DEF_MAX_LENGTH(N,length) const size_t MAX_##N##_LENGTH = length; const char *MAX_##N##_LENGTH_ERR = "HTTP element " # N " is longer than the " # length " allowed length." @@ -251,13 +252,12 @@ static void header_done(void *data, const char *at, size_t length) VALUE temp = Qnil; char *colon = NULL; - /* set rack.url_scheme to "https" or "http" */ - if ((temp = rb_hash_aref(req, global_http_x_forwarded_proto)) != Qnil) { - if (strcmp("https", RSTRING_PTR(temp))) - temp = rb_str_new("http", 4); - /* we leave temp alone if it's "https" */ - } else { - temp = rb_str_new("http", 4); + /* set rack.url_scheme to "https" or "http", no others are allowed by Rack */ + temp = rb_hash_aref(req, global_http_x_forwarded_proto); + switch (temp == Qnil ? 0 : RSTRING_LEN(temp)) { + case 5: if (!memcmp("https", RSTRING_PTR(temp), 5)) break; + case 4: if (!memcmp("http", RSTRING_PTR(temp), 4)) break; + default: temp = global_http; } rb_hash_aset(req, global_rack_url_scheme, temp); @@ -403,6 +403,7 @@ void Init_http11() DEF_GLOBAL(http_x_forwarded_proto, "HTTP_X_FORWARDED_PROTO"); DEF_GLOBAL(port_80, "80"); DEF_GLOBAL(localhost, "localhost"); + DEF_GLOBAL(http, "http"); eHttpParserError = rb_define_class_under(mUnicorn, "HttpParserError", rb_eIOError); diff --git a/test/unit/test_request.rb b/test/unit/test_request.rb index 5109c7b..0613326 100644 --- a/test/unit/test_request.rb +++ b/test/unit/test_request.rb @@ -55,7 +55,7 @@ class RequestTest < Test::Unit::TestCase assert_nothing_raised { res = @lint.call(env) } end - def test_x_forwarded_proto + def test_x_forwarded_proto_https res = env = nil client = MockRequest.new("GET / HTTP/1.1\r\n" \ "X-Forwarded-Proto: https\r\n" \ @@ -65,6 +65,16 @@ class RequestTest < Test::Unit::TestCase assert_nothing_raised { res = @lint.call(env) } end + def test_x_forwarded_proto_http + res = env = nil + client = MockRequest.new("GET / HTTP/1.1\r\n" \ + "X-Forwarded-Proto: http\r\n" \ + "Host: foo\r\n\r\n") + assert_nothing_raised { env = @request.read(client) } + assert_equal "http", env['rack.url_scheme'] + assert_nothing_raised { res = @lint.call(env) } + end + def test_x_forwarded_proto_invalid res = env = nil client = MockRequest.new("GET / HTTP/1.1\r\n" \ -- cgit v1.2.3-24-ge0c7