Skip to content

Commit

Permalink
Immediately return if route regex didn't match.
Browse files Browse the repository at this point in the history
This is for Issue #1187.

It's not safe to look at `%+` without first having checked whether the regex
matched successfully or not.  Otherwise, we might see keys from a previous regex
match, because `%+` is only cleared after a successful match.

(perldoc perlvar confirms, "Note: "%-" and "%+" are tied views into a common
internal hash associated with the last successful regular expression.")

Dancer2 doesn't have problems here, as it already returns immediately if the
regex didn't match, the same way this change does.

Note: I think this would only happen via certain deployment methods, and if
there was an earlier route which used named captures.

In particular, we never had any problems in our large API app's comprehensive
test suite when we used Dancer::Test.  We've re-worked our testing framework to
use Plack::Test, partially in readyness to switch to Dancer2, and started
getting weird failures which were tracked down to incorrect route matching, and
this was the cause.
  • Loading branch information
bigpresh committed Jun 13, 2018
1 parent 28d2fdc commit 6292fce
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions lib/Dancer/Route.pm
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,13 @@ sub match {

my @values = $path =~ $self->{_compiled_regexp};

Dancer::Logger::core(" --> got ".
map { defined $_ ? $_ : 'undef' } @values)
if @values;
return unless @values;

Dancer::Logger::core(
" --> got " .
map { defined $_ ? $_ : 'undef' } @values
);


# if some named captures found, return captures
# no warnings is for perl < 5.10
Expand All @@ -103,7 +107,6 @@ sub match {
return $self->save_match_data($request, {captures => \%captures});
}

return unless @values;

# save the route pattern that matched
# TODO : as soon as we have proper Dancer::Internal, we should remove
Expand Down

0 comments on commit 6292fce

Please sign in to comment.