PerlDancer / Dancer2

Perl Dancer Next Generation (rewrite of Perl Dancer)
http://perldancer.org/
Other
542 stars 274 forks source link

redirect '/' doesn't always work as expected #1611

Closed gurnec closed 3 years ago

gurnec commented 3 years ago

When the root of a Dancer app is running under a non-root path on a website, e.g. with an app rooted at:

https://www.example.com/dancerapp/

I would expect (although perhaps I'm wrong?) that redirect '/' would redirect to the URL above, but instead it results in a Location: / header redirecting to the root of the website.

I believe the regex on line 1278 below should be m{^/(?!/)}.

https://github.com/PerlDancer/Dancer2/blob/b9c740f9cb849c1c3bc9bc9a5e3449411f346052/lib/Dancer2/Core/App.pm#L1273-L1284

(As a workaround one can use redirect '/?'.)

cromedome commented 3 years ago

I think what you actually want is redirect uri_for( '/' ), which would take into account what the base URI is for your application when determining what /.

Let me know so I can close this ticket or keep it open as needed. Thanks!

gurnec commented 3 years ago

If the current behavior is by design, then by all means please close the issue.

Since redirect '/login' redirects to /dancerapp/login, I had expected redirect '/' to redirect to /dancerapp/. Note that previous versions (I can try to find when this changed) of Dancer2 did redirect to /dancerapp/*. Of course it's your judgement call as to which behavior is preferred.

edit: In any case, thanks for the quick response! * edit2: Actually I'm not sure about that, there was a behavior change which I see in blame, but I don't recall exactly what it was.

gurnec commented 3 years ago

I did a little bit of digging:

In versions 0.300000 and earlier, redirect calls uri_for for all URLs which don't start with a scheme, e.g. don't start with https:.

This was modified in f80ec21, and in versions 0.300001 - 0.300004 URLs are used verbatim.

Then 80595d1 changed this to prepend /dancerapp for URLs starting with / and not //, but only for URLs of length 2+. I don't really have an opinion on what the "right" behavior is, although I suppose it would be nice if it were documented.

veryrusty commented 3 years ago

Sorry for taking so long to look at this @gurnec. You are correct; it's a bug.

Any path beginning with a single / should be treated the same. The regex should use a negative lookahead you provided (which is to ensure schemaless redirects do not get the scriptname appended).

gurnec commented 3 years ago

Thanks for the update!