Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

'redirect' decodes the encoded url #846

Open
berekuk opened this Issue · 4 comments

4 participants

@berekuk

I think it's a bug.

package redirtest;
use Dancer ':syntax';

get '/' => sub {
    # returns foo: bar=
    # (older versions of (Plack? Dancer?) lose the '=' sign too, because of their different interpretation of a=b=value cgi params)
    return "foo: ".params->{foo};
};

get '/redir' => sub {
    # will redirect to /?foo=bar=&
    redirect '/?foo=bar%3D%26';
};

true;
@racke
Owner

The correct syntax should be:

redirect uri_for('/', {foo => 'bar=&'});

Regards
Racke

@berekuk

But this is still a bug, right? uri_for works because it adds the full http://$hostport to the url, it's just a coincidence.

Or are you saying that I should never pass relative urls to redirect (in which case docs should be fixed), or that there's some hidden meaning to the current behavior?

@yanick yanick referenced this issue from a commit
@yanick yanick add additional test for #846 c7905f7
@yanick
Owner

The problem lies in the interaction of URI and uri_for(). uri_for() passes the path to $uri->path(), which does escape everything (so 'foo&bar%26' becomes 'foo%26bar%26). If uri_for() has the $dont_escape, everything is re-unescaped (and becomes 'foo&bar&'.. oops)

I have to think about the repercussions, but the solution might lie in pre-emptively escaping the '%' before we go through the URI transformation...

@hh10k

I found the temporary solution to this bug is to always pass an absolute URL to redirect. _redirect() in Dancer.pm doesn't modify the URL if it appears to start with a scheme.

get '/redir' => sub {
    # Will correctly redirect to /?foo=bar%3D%26
    redirect request->uri_base.'/?foo=bar%3D%26';
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.