redirection to different domain must not keep previous port #221
Conversation
dbu
force-pushed
the
fix-redirect-host
branch
from
c00ff8e to
ee39c1d
Compare
| /** | ||
| * @testWith ["https://example.com/path?query=value", "https://example.com?query=value", "https://example.com?query=value"] | ||
| * ["https://example.com/path?query=value", "https://example.com/?query=value", "https://example.com/?query=value"] | ||
| * ["https://example.com", "https://example.com?query=value", "https://example.com?query=value"] |
There was a problem hiding this comment.
moved to provider method to have descriptions of the cases
dbu
force-pushed
the
fix-redirect-host
branch
from
ee39c1d to
cf26336
Compare
|
@ostrolucky can you review this one please? trying to get all of the redirection right. i discovered that besides the domain/scheme/port topic, RFC 7231 also allows relative paths with all of the path semantics... |
dbu
force-pushed
the
fix-redirect-host
branch
from
cf26336 to
baacbe9
Compare
| // If relative, we need to check if we have an absolute path or not | ||
|
|
||
| $path = array_key_exists('path', $parsedLocation) ? $parsedLocation['path'] : ''; | ||
| if (!array_key_exists('host', $parsedLocation) && '/' !== $location[0]) { |
There was a problem hiding this comment.
what if $location has 0 length?
There was a problem hiding this comment.
oops. i assumed that parse_url('') would fail, but actually it gives an array with path => ''.
i add a check to fail if location is empty, as that would make no sense.
src/Plugin/RedirectPlugin.php
Outdated
|
|
||
| $path = array_key_exists('path', $parsedLocation) ? $parsedLocation['path'] : ''; | ||
| if (!array_key_exists('host', $parsedLocation) && '/' !== $location[0]) { | ||
| // this is a relative-path reference |
There was a problem hiding this comment.
what is a relative path reference? this is unclear here
src/Plugin/RedirectPlugin.php
Outdated
| } | ||
| /* replace '/./' or '/foo/../' with '/' */ | ||
| $re = ['#(/\./)#', '#/(?!\.\.)[^/]+/\.\./#']; | ||
| for( $n=1; $n>0; $path=preg_replace( $re, '/', $path, -1, $n ) ) { |
There was a problem hiding this comment.
you have som weird code style here. Need to be a bit more careful because this project doesn't have cs check
There was a problem hiding this comment.
oh indeed. copy-pasted from stackoverflow 🙈
i tried to make it a bit better
There was a problem hiding this comment.
also adding a cs-fixer now
dbu
force-pushed
the
fix-redirect-host
branch
3 times, most recently
from
666fceb to
1c6a640
Compare
dbu
force-pushed
the
fix-redirect-host
branch
from
1c6a640 to
509e513
Compare
|
thanks for the feedback. i updated all points you discovered |
What's in this PR?
Correctly handle non-absolute URLs in the redirect
Why?
Conform with RFC 7231 Location header specification
Checklist