From mboxrd@z Thu Jan 1 00:00:00 1970 Delivered-To: chneukirchen@gmail.com Received: by 10.142.191.1 with SMTP id o1cs70317wff; Sun, 6 Dec 2009 18:32:52 -0800 (PST) Received: from mr.google.com ([10.141.32.10]) by 10.141.32.10 with SMTP id k10mr2591539rvj.27.1260153170787 (num_hops = 1); Sun, 06 Dec 2009 18:32:50 -0800 (PST) Received: by 10.141.32.10 with SMTP id k10mr391493rvj.27.1260153169259; Sun, 06 Dec 2009 18:32:49 -0800 (PST) X-BeenThere: rack-devel@googlegroups.com Received: by 10.141.14.15 with SMTP id r15ls185602rvi.1.p; Sun, 06 Dec 2009 18:32:47 -0800 (PST) Received: by 10.140.171.15 with SMTP id t15mr1041078rve.10.1260153167486; Sun, 06 Dec 2009 18:32:47 -0800 (PST) Received: by 10.140.171.15 with SMTP id t15mr1041077rve.10.1260153167470; Sun, 06 Dec 2009 18:32:47 -0800 (PST) Return-Path: Received: from dcvr.yhbt.net (dcvr.yhbt.net [64.71.152.64]) by gmr-mx.google.com with ESMTP id 9si280705pxi.10.2009.12.06.18.32.47; Sun, 06 Dec 2009 18:32:47 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of normalperson@yhbt.net designates 64.71.152.64 as permitted sender) client-ip=64.71.152.64; Received: from localhost (unknown [127.0.2.5]) by dcvr.yhbt.net (Postfix) with ESMTP id EFC4C1F687; Mon, 7 Dec 2009 02:32:46 +0000 (UTC) Date: Sun, 6 Dec 2009 18:32:46 -0800 From: Eric Wong To: rack-devel@googlegroups.com Cc: Marc-Andre Cournoyer Subject: Re: Does Rack SPEC states that PATH_INFO must be cut after ";" (semicolon)? Message-ID: <20091207023246.GA22304@dcvr.yhbt.net> References: <200911241922.13218.ibc@aliax.net> <5a9d52bd0912051454v2159584cuc670d0fb80aab847@mail.gmail.com> <200912060015.07634.ibc@aliax.net> MIME-Version: 1.0 In-Reply-To: <200912060015.07634.ibc@aliax.net> User-Agent: Mutt/1.5.18 (2008-05-17) X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com: best guess record for domain of normalperson@yhbt.net designates 64.71.152.64 as permitted sender) smtp.mail=normalperson@yhbt.net Reply-To: rack-devel@googlegroups.com Precedence: list Mailing-list: list rack-devel@googlegroups.com; contact rack-devel+owners@googlegroups.com List-ID: List-Post: , List-Help: , List-Archive: X-Thread-Url: http://groups.google.com/group/rack-devel/t/f48d0f995357a3de X-Message-Url: http://groups.google.com/group/rack-devel/msg/6ced64b632714068 List-Unsubscribe: , List-Subscribe: , Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit Iñaki Baz Castillo wrote: > El Sábado, 5 de Diciembre de 2009, Scytrin dai Kinthra escribió: > > After going through the various handlers and routing middlewares, I am > > unaware of any apps that modify the PATH_INFO in the way described. > > The split of PATH_INFO at ';' seems to occur before the request hits > > rack. > > I'm really sorry as I forgot to update this thread. Thin's developer already > confirmed me that this is a bug in Mongrel parser: > > http://github.com/macournoyer/thin/issues#issue/7 > > The fact is that using Webrick the issue doesn't happen. It occurs with > Mongrel itself and Thin as both uses Mongrel HTTP parser. Hi, This should be a one line fix for Thin, I just pushed out a similar fix for unicorn.git (with tests). >From e8dd3e13b9a9f548a3138debd09e87fbb69e3998 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 7 Dec 2009 02:20:18 +0000 Subject: [PATCH] http: PATH_INFO/REQUEST_PATH includes semi-colons This is allowed according to RFC 2396, section 3.3 and matches the behavior of URI.parse, as well. --- ext/unicorn_http/unicorn_http_common.rl | 2 +- lib/unicorn/app/old_rails/static.rb | 6 +--- test/unit/test_http_parser_ng.rb | 46 +++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/ext/unicorn_http/unicorn_http_common.rl b/ext/unicorn_http/unicorn_http_common.rl index 5d46087..041dfec 100644 --- a/ext/unicorn_http/unicorn_http_common.rl +++ b/ext/unicorn_http/unicorn_http_common.rl @@ -33,7 +33,7 @@ query = ( uchar | reserved )* %query_string ; param = ( pchar | "/" )* ; params = ( param ( ";" param )* ) ; - rel_path = ( path? %request_path (";" params)? ) ("?" %start_query query)?; + rel_path = (path? (";" params)? %request_path) ("?" %start_query query)?; absolute_path = ( "/"+ rel_path ); path_uri = absolute_path > mark %request_uri; Absolute_URI = (scheme "://" host_with_port path_uri); diff --git a/lib/unicorn/app/old_rails/static.rb b/lib/unicorn/app/old_rails/static.rb index 82f8aa5..13a435e 100644 --- a/lib/unicorn/app/old_rails/static.rb +++ b/lib/unicorn/app/old_rails/static.rb @@ -46,11 +46,7 @@ class Unicorn::App::OldRails::Static < Struct.new(:app, :root, :file_server) end # then try the cached version: - - # grab the semi-colon REST operator used by old versions of Rails - # this is the reason we didn't just copy the new Rails::Rack::Static - env[REQUEST_URI] =~ /^#{Regexp.escape(path_info)}(;[^\?]+)/ - path_info << "#$1#{ActionController::Base.page_cache_extension}" + path_info << ActionController::Base.page_cache_extension if File.file?("#{root}/#{::Rack::Utils.unescape(path_info)}") env[PATH_INFO] = path_info diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb index e84c765..bb61e7f 100644 --- a/test/unit/test_http_parser_ng.rb +++ b/test/unit/test_http_parser_ng.rb @@ -371,4 +371,50 @@ class HttpParserNgTest < Test::Unit::TestCase assert ! parser.headers? end + def test_path_info_semicolon + qs = "QUERY_STRING" + pi = "PATH_INFO" + req = {} + str = "GET %s HTTP/1.1\r\nHost: example.com\r\n\r\n" + { + "/1;a=b?c=d&e=f" => { qs => "c=d&e=f", pi => "/1;a=b" }, + "/1?c=d&e=f" => { qs => "c=d&e=f", pi => "/1" }, + "/1;a=b" => { qs => "", pi => "/1;a=b" }, + "/1;a=b?" => { qs => "", pi => "/1;a=b" }, + "/1?a=b;c=d&e=f" => { qs => "a=b;c=d&e=f", pi => "/1" }, + "*" => { qs => "", pi => "" }, + }.each do |uri,expect| + assert_equal req, @parser.headers(req.clear, str % [ uri ]) + @parser.reset + assert_equal uri, req["REQUEST_URI"], "REQUEST_URI mismatch" + assert_equal expect[qs], req[qs], "#{qs} mismatch" + assert_equal expect[pi], req[pi], "#{pi} mismatch" + next if uri == "*" + uri = URI.parse("http://example.com#{uri}") + assert_equal uri.query.to_s, req[qs], "#{qs} mismatch URI.parse disagrees" + assert_equal uri.path, req[pi], "#{pi} mismatch URI.parse disagrees" + end + end + + def test_path_info_semicolon_absolute + qs = "QUERY_STRING" + pi = "PATH_INFO" + req = {} + str = "GET http://example.com%s HTTP/1.1\r\nHost: www.example.com\r\n\r\n" + { + "/1;a=b?c=d&e=f" => { qs => "c=d&e=f", pi => "/1;a=b" }, + "/1?c=d&e=f" => { qs => "c=d&e=f", pi => "/1" }, + "/1;a=b" => { qs => "", pi => "/1;a=b" }, + "/1;a=b?" => { qs => "", pi => "/1;a=b" }, + "/1?a=b;c=d&e=f" => { qs => "a=b;c=d&e=f", pi => "/1" }, + }.each do |uri,expect| + assert_equal req, @parser.headers(req.clear, str % [ uri ]) + @parser.reset + assert_equal uri, req["REQUEST_URI"], "REQUEST_URI mismatch" + assert_equal "example.com", req["HTTP_HOST"], "Host: mismatch" + assert_equal expect[qs], req[qs], "#{qs} mismatch" + assert_equal expect[pi], req[pi], "#{pi} mismatch" + end + end + end -- Eric Wong