From: Eric Wong <normalperson@yhbt.net>
To: rack-devel@googlegroups.com
Cc: Marc-Andre Cournoyer <macournoyer@gmail.com>
Subject: Re: Does Rack SPEC states that PATH_INFO must be cut after ";" (semicolon)?
Date: Sun, 6 Dec 2009 18:32:46 -0800 [thread overview]
Message-ID: <20091207023246.GA22304@dcvr.yhbt.net> (raw)
In-Reply-To: <200912060015.07634.ibc@aliax.net>
Iñaki Baz Castillo <ibc@aliax.net> 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 <normalperson@yhbt.net>
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
next prev parent reply other threads:[~2009-12-07 2:32 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-11-24 18:22 Does Rack SPEC states that PATH_INFO must be cut after ";" (semicolon)? Iñaki Baz Castillo
2009-11-30 16:27 ` Iñaki Baz Castillo
2009-11-30 23:31 ` Hongli Lai
2009-12-01 0:42 ` Iñaki Baz Castillo
2009-12-05 22:54 ` Scytrin dai Kinthra
2009-12-05 23:15 ` Iñaki Baz Castillo
2009-12-07 2:32 ` Eric Wong [this message]
2009-12-07 9:53 ` Iñaki Baz Castillo
[not found] ` <8108c6090912061939h7f2ae70rac675219ed08f2e8@mail.gmail.com>
2009-12-07 11:08 ` masayoshi takahashi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://groups.google.com/group/rack-devel
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20091207023246.GA22304@dcvr.yhbt.net \
--to=rack-devel@googlegroups.com \
--cc=macournoyer@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).