ext/http11: remove strchr() and scary comment regarding it

Replace it with memchr(3) instead, which works on a buffer
with a predetermined length, so we don't have to worry about
strange versions of Ruby which don't null-terminate strings.

memchr() is used several times in the MRI source code itself
(without compatibility definitions), so it should be portable to
all platforms MRI runs on.

Additionally, we now tolerate null bytes in the Host header and
can still parse ports in them correctly if anybody sends
them :)

If it matters, it is also theoretically faster as it doesn't
need to check for a '\0' terminator.


git-svn-id: svn+ssh://rubyforge.org/var/svn/mongrel/trunk@993 19e92222-5c0b-0410-8929-a290d50e31e9

2 files changed, 38 insertions, 2 deletions

diff --git a/ext/http11/http11.c b/ext/http11/http11.c
index 5af1e8c..e0feece 100644
--- a/ext/http11/http11.c
+++ b/ext/http11/http11.c
@@ -292,8 +292,7 @@ void header_done(void *data, const char *at, size_t length)
 
   rb_hash_aset(req, global_gateway_interface, global_gateway_interface_value);
   if((temp = rb_hash_aref(req, global_http_host)) != Qnil) {
-    /* ruby better close strings off with a '\0' dammit */
-    colon = strchr(RSTRING_PTR(temp), ':');
+    colon = memchr(RSTRING_PTR(temp), ':', RSTRING_LEN(temp));
     if(colon != NULL) {
       rb_hash_aset(req, global_server_name, rb_str_substr(temp, 0, colon - RSTRING_PTR(temp)));
       rb_hash_aset(req, global_server_port, 
diff --git a/test/test_http11.rb b/test/test_http11.rb
index deaeaeb..96b6a1b 100644
--- a/test/test_http11.rb
+++ b/test/test_http11.rb
@@ -130,6 +130,43 @@ class HttpParserTest < Test::Unit::TestCase
     end
   end
 
+  def test_host_port_parsing
+    parser = HttpParser.new
+    req = {}
+    should_be_good = "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n"
+    nread = parser.execute(req, should_be_good, 0)
+    assert_equal should_be_good.length, nread
+    assert parser.finished?
+    assert !parser.error?
+    assert_equal "example.com", req["HTTP_HOST"]
+    assert_equal "example.com", req["SERVER_NAME"]
+    assert_equal "80", req["SERVER_PORT"]
+
+    parser = HttpParser.new
+    req = {}
+    should_be_good = "GET / HTTP/1.1\r\nHost: example.com:123\r\n\r\n"
+    nread = parser.execute(req, should_be_good, 0)
+    assert_equal should_be_good.length, nread
+    assert parser.finished?
+    assert !parser.error?
+    assert_equal "example.com:123", req["HTTP_HOST"]
+    assert_equal "example.com", req["SERVER_NAME"]
+    assert_equal "123", req["SERVER_PORT"]
+
+    # null character in domain name is never actually valid, but if it
+    # becomes valid in Web 3.0, we'll be ready for it.
+    parser = HttpParser.new
+    req = {}
+    should_be_good = "GET / HTTP/1.1\r\nHost: example.com\0:123\r\n\r\n"
+    nread = parser.execute(req, should_be_good, 0)
+    assert_equal should_be_good.length, nread
+    assert parser.finished?
+    assert !parser.error?
+    assert_equal "example.com\0:123", req["HTTP_HOST"]
+    assert_equal "example.com\0", req["SERVER_NAME"]
+    assert_equal "123", req["SERVER_PORT"]
+  end
+
   def test_fragment_in_uri
     parser = HttpParser.new
     req = {}