Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Path escaping regex supports RFC 3986 definition.

 * Updated the class of characters-to-not-escape passed to URI::Escape to
   support the proper definition as stipulated by RFC 3986 for URL Paths,
   namely::

    safe           = "$" | "-" | "_" | "." | "+"
    extra          = "!" | "*" | "'" | "(" | ")" | ","

    unreserved     = alpha | digit | safe | extra
    escape         = "%" hex hex

    uchar          = unreserved | escape

    hpath          = hsegment *[ "/" hsegment ]
    hsegment       = *[ uchar | ";" | ":" | "@" | "&" | "=" ]

 * Added unit test which verifies behavior by attempting to use path
   parameters.
  • Loading branch information...
commit 5f9c372480619f84ccbe80293d38703e8a0fda74 1 parent 9b7b2fe
@ssmccoy ssmccoy authored
Showing with 19 additions and 3 deletions.
  1. +3 −2 lib/Plack/Request.pm
  2. +16 −1 t/Plack-Request/uri.t
View
5 lib/Plack/Request.pm
@@ -185,8 +185,9 @@ sub uri {
# This means when a request like /foo%2fbar comes in, we recognize
# it as /foo/bar which is not ideal, but that's how the PSGI PATH_INFO
# spec goes and we can't do anything about it. See PSGI::FAQ for details.
- # http://github.com/plack/Plack/issues#issue/118
- my $path_escape_class = '^A-Za-z0-9\-\._~/';
+
+ # See RFC 3986 before modifying.
+ my $path_escape_class = q{^/;:@&=A-Za-z0-9$_.+!*'(),-};
@thaljef
thaljef added a note

uri_escape eval's the $path_escape_class into a regular expression. So we need to prevent interpolation of things that look like variables. Watch what happens now:

$_ = 'foo-bar';
my $path_escape_class = q{^/;:@&=A-Za-z0-9$_.+!*'(),-};
URI::Escape::uri_escape('yadda/yadda', $path_escape_class);

# Invalid [] range "o-b" in regex; marked by <-- HERE in m/([^/;:@&=A-Za-z0-9foo-b <-- HERE ar.+!*'(),-])/ at (eval 11

Somehow, $_ gets the contents of the Authentication header when I use the auth middleware -- but that is beside the point.

You could consider this a bug in URI::Escape, rather than Plack::Request. As a workaround, just could just put a backslash in front of the dollar sign:

my $path_escape_class = q{^/;:@&=A-Za-z0-9\$_.+!*'(),-};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
my $path = URI::Escape::uri_escape($self->env->{PATH_INFO} || '', $path_escape_class);
$path .= '?' . $self->env->{QUERY_STRING}
View
17 t/Plack-Request/uri.t
@@ -43,6 +43,13 @@ my @tests = (
},
uri => 'http://example.com/exec/',
parameters => {} },
+
+ { add_env => {
+ HTTP_HOST => 'example.com',
+ SCRIPT_NAME => '/exec/'
+ },
+ uri => 'http://example.com/exec/',
+ parameters => {} },
{ add_env => {
SERVER_NAME => 'example.com'
},
@@ -85,7 +92,15 @@ my @tests = (
PATH_INFO => "/baz quux",
},
uri => 'http://example.com/foo%20bar/baz%20quux',
- parameters => {} }
+ parameters => {} },
+ { add_env => {
+ HTTP_HOST => 'example.com',
+ SCRIPT_NAME => "/path",
+ PATH_INFO => "/parameters;path=one,two",
+ QUERY_STRING => "query=foobar",
+ },
+ uri => 'http://example.com/path/parameters;path=one,two?query=foobar',
+ parameters => { query => "foobar" } },
);
plan tests => 2 * @tests;

1 comment on commit 5f9c372

@bobtfish
Collaborator

+1 this change, however I think we should fix this issue in URI::Escape at the same time, and depend on a new version of URI (we already depend on a fairly recent version).

Please sign in to comment.
Something went wrong with that request. Please try again.