May accidentally match a route if %+ was already populated

[bigpresh]

In Dancer::Route->match, if %+ has any keys in it, then we will consider the route a match:

sub match {
    my ($self, $request) = @_;

    my $method = lc($request->method);

    my $path   = $request->path_info;

    Dancer::Logger::core(
        sprintf "Trying to match '%s %s' against /%s/ (generated from '%s')",
            $request->method, $path, $self->{_compiled_regexp}, $self->pattern
    );

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

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

    # if some named captures found, return captures
    # no warnings is for perl < 5.10
    if (my %captures =
        do { no warnings; %+ }
      )
    {
        Dancer::Logger::core(
            "  --> captures are: " . join(", ", keys(%captures)))
          if keys %captures;
        return $self->save_match_data($request, {captures => \%captures});
    }

    return unless @values;

    # save the route pattern that matched

Note that, if %+ had keys, then we'll call return $self->save_match_data($request, {captures => \%captures}); before checking if the route actually matched.

But, %+ only gets cleared after a successful match, so it's not safe to rely on it without first checking that the regex did match.

It may be as simple as moving the return unless @values; up a little so it's checked before looking at %+.

We should also confirm whether this is present in Dancer2.

Found by a colleague, @skington.

[bigpresh]

I've looked at Dancer2, and it won't have this problem, because in Dancer2::Core::Route->match we check for the match first:

sub match {
    my ( $self, $request ) = @_; 

    if ( $self->has_options ) { 
        return unless $self->validate_options($request);
    }   

    my @values = $request->path =~ $self->regexp;

    return unless @values;

    # if some named captures are found, return captures
    # no warnings is for perl < 5.10
    # - Note no @values implies no named captures
    if (my %captures =
        do { no warnings; %+ }
      )   
    {   
        return $self->_match_data( { captures => \%captures } );
    } 
...

So, that backs up the "just return unless @values before looking at %+" idea.