From dcbed48c2a41144b7cc3e46d4d23f9bd21fc60ff Mon Sep 17 00:00:00 2001 From: Markus Jansen Date: Thu, 28 Aug 2014 20:16:02 +0200 Subject: [PATCH 1/2] Implemented allow_encoded_slashes aka handling of 2%F as part of the PATH_INFO --- AUTHORS | 1 + lib/Dancer/Config.pm | 34 +++++++++++++++++++------------ lib/Dancer/Request.pm | 33 +++++++++++++++++++++++++----- lib/Dancer/Route.pm | 9 ++++++++ t/02_request/19_encoded_slashes.t | 33 ++++++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 18 deletions(-) create mode 100644 t/02_request/19_encoded_slashes.t diff --git a/AUTHORS b/AUTHORS index 74e65c5a7..166d22ced 100644 --- a/AUTHORS +++ b/AUTHORS @@ -87,6 +87,7 @@ A huge thank you to all of them! Marc Chantreux Mark Allen Mark Stosberg + Markus Jansen Matthew Horsfall (alh) Maurice Max Maischein diff --git a/lib/Dancer/Config.pm b/lib/Dancer/Config.pm index 0cdb88979..7e503375c 100644 --- a/lib/Dancer/Config.pm +++ b/lib/Dancer/Config.pm @@ -239,19 +239,20 @@ sub load_settings_from_yaml { } sub load_default_settings { - $SETTINGS->{server} ||= $ENV{DANCER_SERVER} || '0.0.0.0'; - $SETTINGS->{port} ||= $ENV{DANCER_PORT} || '3000'; - $SETTINGS->{content_type} ||= $ENV{DANCER_CONTENT_TYPE} || 'text/html'; - $SETTINGS->{charset} ||= $ENV{DANCER_CHARSET} || ''; - $SETTINGS->{startup_info} ||= !$ENV{DANCER_NO_STARTUP_INFO}; - $SETTINGS->{daemon} ||= $ENV{DANCER_DAEMON} || 0; - $SETTINGS->{apphandler} ||= $ENV{DANCER_APPHANDLER} || 'Standalone'; - $SETTINGS->{warnings} ||= $ENV{DANCER_WARNINGS} || 0; - $SETTINGS->{auto_reload} ||= $ENV{DANCER_AUTO_RELOAD} || 0; - $SETTINGS->{traces} ||= $ENV{DANCER_TRACES} || 0; - $SETTINGS->{server_tokens} ||= !$ENV{DANCER_NO_SERVER_TOKENS}; - $SETTINGS->{logger} ||= $ENV{DANCER_LOGGER} || 'file'; - $SETTINGS->{environment} ||= + $SETTINGS->{server} ||= $ENV{DANCER_SERVER} || '0.0.0.0'; + $SETTINGS->{port} ||= $ENV{DANCER_PORT} || '3000'; + $SETTINGS->{content_type} ||= $ENV{DANCER_CONTENT_TYPE} || 'text/html'; + $SETTINGS->{charset} ||= $ENV{DANCER_CHARSET} || ''; + $SETTINGS->{startup_info} ||= !$ENV{DANCER_NO_STARTUP_INFO}; + $SETTINGS->{daemon} ||= $ENV{DANCER_DAEMON} || 0; + $SETTINGS->{apphandler} ||= $ENV{DANCER_APPHANDLER} || 'Standalone'; + $SETTINGS->{warnings} ||= $ENV{DANCER_WARNINGS} || 0; + $SETTINGS->{auto_reload} ||= $ENV{DANCER_AUTO_RELOAD} || 0; + $SETTINGS->{traces} ||= $ENV{DANCER_TRACES} || 0; + $SETTINGS->{server_tokens} ||= !$ENV{DANCER_NO_SERVER_TOKENS}; + $SETTINGS->{logger} ||= $ENV{DANCER_LOGGER} || 'file'; + $SETTINGS->{allow_encoded_slashes} ||= $ENV{DANCER_ALLOW_ENCODED_SLASHES} || 'Off'; + $SETTINGS->{environment} ||= $ENV{DANCER_ENVIRONMENT} || $ENV{PLACK_ENV} || 'development'; @@ -670,6 +671,13 @@ Maximum size of route cache (e.g. 1024, 2M). Defaults to 10M (10MB) - see L +=head2 Route handling + +=head3 allow_encoded_slashes (string) + +Possible values are 'On', 'Off', and 'NoDecode'. +If not set to 'Off', forward slashes encoded as %2F contained in the route will not be considered for the pattern matching. +Encoded slashes as parameter values will be decoded if the value is 'On'. Default is 'Off'. =head2 DANCER_CONFDIR and DANCER_ENVDIR diff --git a/lib/Dancer/Request.pm b/lib/Dancer/Request.pm index c69fe42b6..824e2a6ba 100644 --- a/lib/Dancer/Request.pm +++ b/lib/Dancer/Request.pm @@ -444,13 +444,30 @@ sub _build_path { my ($self) = @_; my $path = ""; + # prevent decoding encoded slashes if appropriate + # which is a MAY condition according to RFC 3875 + my $allow_encoded_slashes = setting('allow_encoded_slashes') || 'Off'; + + my $unencode_slashes = $allow_encoded_slashes eq 'Off' ? 1 : 0; + $path .= $self->script_name if defined $self->script_name; - $path .= $self->env->{PATH_INFO} if defined $self->env->{PATH_INFO}; + + # CGI.pm rigorously unescapes PATH_INFO, so we cannot use that + # ... unless $request_uri is not defined + my $request_uri = $self->request_uri; + $unencode_slashes = 1 if (! defined $request_uri); + + if ($unencode_slashes) { + $path .= $self->env->{PATH_INFO} if defined $self->env->{PATH_INFO}; + } # fallback to REQUEST_URI if nothing found # we have to decode it, according to PSGI specs. - if (defined $self->request_uri) { - $path ||= $self->_url_decode($self->request_uri); + # however, we should cut off the trailing query_string + + if ($path eq '' && defined $request_uri) { + # $request_uri =~ s/\?.*$//; + $path = $self->_url_decode($request_uri, !$unencode_slashes); } raise core_request => "Cannot resolve path" if not $path; @@ -478,10 +495,16 @@ sub _build_method { } sub _url_decode { - my ($self, $encoded) = @_; + my ($self, $encoded, $allow_encoded_slashes) = @_; my $clean = $encoded; $clean =~ tr/\+/ /; - $clean =~ s/%([a-fA-F0-9]{2})/pack "H2", $1/eg; + if ($allow_encoded_slashes) { + # don't pack %2F + $clean =~ s/%([a-fA-F013-9][a-fA-F0-9]|2[a-eA-e0-9])/pack "H2", $1/eg; + } + else { + $clean =~ s/%([a-fA-F0-9]{2})/pack "H2", $1/eg; + } return $clean; } diff --git a/lib/Dancer/Route.pm b/lib/Dancer/Route.pm index 2a7f2c4e1..b8c5a8567 100644 --- a/lib/Dancer/Route.pm +++ b/lib/Dancer/Route.pm @@ -123,6 +123,15 @@ sub match { # IT WILL MOVE VERY SOON $request->{_route_pattern} = $self->pattern; + # decode encoded slashes if appropriate + my $allow_encoded_slashes = setting('allow_encoded_slashes') || 'Off'; + if ( $allow_encoded_slashes eq 'On' ) + { + foreach my $i ( 0 .. $#values ) { + $values[$i] =~ s,%2[Ff],/,g; + } + } + # named tokens my @tokens = @{$self->{_params} || []}; diff --git a/t/02_request/19_encoded_slashes.t b/t/02_request/19_encoded_slashes.t new file mode 100644 index 000000000..aa661a361 --- /dev/null +++ b/t/02_request/19_encoded_slashes.t @@ -0,0 +1,33 @@ +use Test::More; + +plan tests => 2; + +{ + use Dancer ':tests'; +diag setting('allow_encoded_slashes'); + + get '/:foo/baz' => sub { + param 'foo'; + }; + + get '/:foo/:bar/baz' => sub { + param 'foo'; + }; +} + +use Dancer::Test; +use Dancer::Config 'setting'; + +# CGI.pm used by HTTP::Server::Simple is the real culprit, so we cannot test the 'Off' mode here +# +# # set allow_encoded_slashes => 'Off'; +# setting('allow_encoded_slashes' => 'Off'); +# response_content_is [GET => '/42%2F42/baz'], '42'; + +# set allow_encoded_slashes => 'On'; +setting('allow_encoded_slashes' => 'On'); +response_content_is [GET => '/42%2F42/baz'], '42/42'; + +# set allow_encoded_slashes => 'NoDecode'; +setting('allow_encoded_slashes' => 'NoDecode'); +response_content_is [GET => '/42%2F42/baz'], '42%2F42'; From 1f61bbe90c770513eb4293255fc558fae303e737 Mon Sep 17 00:00:00 2001 From: Markus Jansen Date: Wed, 3 Sep 2014 16:18:16 +0200 Subject: [PATCH 2/2] Small but essential fix for AllowEncodedSlashes - fixes also PATH_INFO. --- lib/Dancer/Request.pm | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/Dancer/Request.pm b/lib/Dancer/Request.pm index 824e2a6ba..ad463e701 100644 --- a/lib/Dancer/Request.pm +++ b/lib/Dancer/Request.pm @@ -467,7 +467,17 @@ sub _build_path { if ($path eq '' && defined $request_uri) { # $request_uri =~ s/\?.*$//; - $path = $self->_url_decode($request_uri, !$unencode_slashes); + my ( $request_uri_path, $query_string ) = + $request_uri =~ /^([^?]+)(\?.*)$/ ? ( $1, $2 ) : ( $request_uri, '' ); + $path = $self->_url_decode($request_uri_path, !$unencode_slashes); + + # change PATH_INFO in retrospect if we can safely do so + my $path_info = $self->env->{PATH_INFO} || ''; + my $unencoded_request_uri_path = $self->_url_decode($request_uri_path); + if ( $unencoded_request_uri_path ne $path && + $unencoded_request_uri_path eq $path_info ) { + $self->env->{PATH_INFO} = $path; + } } raise core_request => "Cannot resolve path" if not $path; @@ -500,7 +510,7 @@ sub _url_decode { $clean =~ tr/\+/ /; if ($allow_encoded_slashes) { # don't pack %2F - $clean =~ s/%([a-fA-F013-9][a-fA-F0-9]|2[a-eA-e0-9])/pack "H2", $1/eg; + $clean =~ s/%([a-fA-F013-9][a-fA-F0-9]|2[a-eA-E0-9])/pack "H2", $1/eg; } else { $clean =~ s/%([a-fA-F0-9]{2})/pack "H2", $1/eg;