uri_for_route(): review which parameter values are accepted, and what should be URI-escaped

Summary

uri_for_route() interpolates route parameter values straight into the route's path spec and hands the assembled string to request->uri_for(). Nothing validates the value, and nothing escapes it beyond whatever URI->canonical happens to do to the finished string.

The result is that some values produce a URI which cannot match the very route it was generated from, one common character (%) is unrepresentable, and non-ASCII encoding is non-deterministic. I'd like the team's view on which values should be refused, which should be escaped, and what we're willing to break to get there.

This came out of reviewing a test-suite audit by Curtis "Ovid" Poe. The narrow part of it — that a value of 0 was rejected because the code tested for truth — is being fixed in a PR already, and the "accept 0, refuse undef and ''" behaviour isn't what I'm asking about here. This issue is the wider question that turned up while checking that fix.

How it works today

lib/Dancer2/Core/App.pm, in uri_for_route():

foreach my $param (@params) {
    $param =~ s{^([^\[]+).*}{$1}xms;
    my $value = $route_params->{$param};
    ...
    $string =~ s!\Q:$param\E(\[[^\]]+\])?!$value!xmsg;
}
...
return $self->request->uri_for( $string, $query_params, $dont_escape );

The value goes in raw. request->uri_for() then does $uri->path("$base/$part") and returns $uri->canonical, which escapes some characters in the assembled path but has no idea which parts of it came from user data.

Relevant constraint: a :param compiles to ([^/]+) (lib/Dancer2/Core/Route.pm:278) — one or more non-slash characters.

What actually happens

Generating a URI for get 'item' => '/item/:item_id' and then requesting the result back against the same app:

value passed	generated URI	matches its own route?
'abc'	/item/abc	yes
0	/item/0	yes
''	/item/	no — 404
'a/b'	/item/a/b	no — 404
'a b'	/item/a%20b	yes
'a?b=1'	/item/a%3Fb=1	yes
'a#frag'	/item/a%23frag	yes
'a%2Fb' (five literal chars)	/item/a%2Fb	no — 404
'..'	/item/..	yes (matched literally)
'a+b'	/item/a+b	yes
"caf\x{e9}"	/item/caf%E9	matches, but the parameter comes back as an undecoded byte, with Invalid UTF-8 in PATH_INFO warnings — see below

The '' and a/b rows are the clearest: we emit a URL that our own router rejects.

Would escaping fix it?

Comparing raw interpolation against escaping each value with uri_escape_utf8 before it goes in:

value	raw (today)	round-trips?	escaped	round-trips?
'a/b'	/item/a/b	no — 404	/item/a%2Fb	no — still 404
'100%'	/item/100%	yes (by luck)	/item/100%25	yes
'a%2Fb'	/item/a%2Fb	no — 404	/item/a%252Fb	yes
"café" (UTF8 flag set)	/item/caf%C3%A9	yes	/item/caf%C3%A9	yes
"caf\x{e9}" (flag not set)	/item/caf%E9	byte, + warnings	/item/caf%C3%A9	yes
'a b'	/item/a%20b	yes	/item/a%20b	yes
'abc'	/item/abc	yes	/item/abc	yes

Three things fall out of that:

Escaping does not rescue a slash. %2F is decoded back to / before route matching, so ([^/]+) still fails. We'd be swapping a visibly broken URL for an invisibly broken one. It isn't portable either — Apache 404s %2F unless AllowEncodedSlashes On, and various proxies normalise it — so this isn't something we can fix on our side alone.

Escaping is what makes % representable. Today a value containing a literal % is interpolated raw and then interpreted as an escape on the way back in. Escaped, a%2Fb becomes a%252Fb and round-trips to exactly the five characters that went in.

request->uri_for() does not double-escape. URI's path() accepts an already-escaped path and canonical leaves existing escapes alone — 100%25 stays 100%25, a%252Fb stays a%252Fb. That's convenient for us, and it is also precisely why a raw % is unsafe today: nothing is protecting it.

Non-ASCII is non-deterministic

The same logical value encodes differently depending on whether Perl's UTF8 flag happens to be set on the scalar:

UTF8 flag OFF   raw -> /item/caf%E9        escaped -> /item/caf%C3%A9
UTF8 flag ON    raw -> /item/caf%C3%A9     escaped -> /item/caf%C3%A9

URI encodes according to the string's internal representation, so "caf\x{e9}" written as a literal gives %E9 while the same text arriving from a decoded request gives %C3%A9. The flag depends on where the string came from — a DB handle's settings, a config file, a decoded parameter — and is invisible at the call site. %E9 is not valid UTF-8, and feeding that URL back in produces Invalid UTF-8 in PATH_INFO warnings and a mangled parameter.

uri_escape_utf8 collapses both cases to %C3%A9.

What I think the questions are

1. Should any values be refused outright? '' and anything containing / provably cannot match the route they were generated from, so arguably uri_for_route should die rather than hand back a URL that doesn't work. ('' is already being addressed in the PR mentioned above; / is not.)

2. Should values be escaped? I lean yes, with uri_escape_utf8 per value, on the grounds that a caller currently cannot write correct code: some characters are escaped for them and others aren't, a literal % can't be expressed at all, and the UTF-8 behaviour depends on an invisible flag.

3. What do we do about callers who already pre-encode? This is the real cost. Someone passing a%2Fb today, deliberately, to mean a/b, would start getting a%252Fb. That's row three of the second table: the same change is a fix or a regression depending on what the caller meant, and the value alone doesn't tell us. Needs at minimum a Changes note, possibly a deprecation cycle.

4. Is $dont_escape the right opt-out? It exists on both uri_for_route and uri_for, but it currently means "uri_unescape the entire finished URI", which is a blunter instrument than "leave my parameter values alone". If it's to be the escape hatch, its semantics probably need revisiting too.

5. Splat must be treated separately. Megasplat deliberately joins its elements with /:

   my $megasplat = join '/', @{ $splat_params->[$i] };
   

   so any slash restriction has to apply to named parameters only, not blanket to every substitution.

Reproducing

Both tables come from this, run against the repo with perl -Ilib:

use strict;
use warnings;
use Plack::Test;
use HTTP::Request::Common;
use URI::Escape qw< uri_escape_utf8 >;

{
    package UriApp;
    use Dancer2;
    use URI::Escape qw< uri_escape_utf8 >;
    set logger => 'null';

    get 'item' => '/item/:item_id' => sub {
        'MATCHED:' . route_parameters->get('item_id')
    };

    my $probe;
    sub set_probe { $probe = $_[0] }
    get '/raw'     => sub { request->uri_for( '/item/' . $probe ) };
    get '/escaped' => sub { request->uri_for( '/item/' . uri_escape_utf8($probe) ) };
}

my $test = Plack::Test->create( UriApp->to_app );

sub probe_uri {
    my $uri = shift;
    return 'n/a' unless $uri =~ m{^http://localhost(/.*)$};
    my $res = $test->request( GET $1 );
    return $res->code == 200 && $res->content =~ /^MATCHED:(.*)$/s
         ? "matched '$1'" : 'NO MATCH (' . $res->code . ')';
}

for my $v ( 'abc', 0, '', 'a/b', 'a b', 'a?b=1', 'a#frag', 'a%2Fb', '..', 'a+b', "caf\x{e9}" ) {
    UriApp::set_probe($v);
    for my $mode (qw< raw escaped >) {
        my $uri = $test->request( GET "/$mode" )->content;
        printf "%-10s %-8s -> %-32s %s\n", "'$v'", $mode, $uri, probe_uri($uri);
    }
}

Environment

• Dancer2 main at 23baea04
• perl 5.32.1
• URI 5.x, Plack::Test

Happy to write whichever way the team wants to go — but this one felt like it needed a decision on backwards compatibility before any code, hence an issue rather than a PR.

Start with uri_for_route() in lib/Dancer2/Core/App.pm and the :param constraint in lib/Dancer2/Core/Route.pm:278, then run the supplied Plack::Test reproduction. Resolve which named-parameter values are refused or escaped, how $dont_escape and splats behave, and what compatibility policy applies; done requires an agreed behavior with regression tests and a Changes note.