Skip to content

Commit cf26336

Browse files
dbudbu
committed
redirection to different domain must not keep previous port
1 parent 76730e3 commit cf26336

File tree

4 files changed

+47
-55
lines changed

4 files changed

+47
-55
lines changed

‎CHANGELOG.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
### Fixed
66

7-
- Fixes false positive circular detection in RedirectPlugin in cases when target location does not contain path
7+
- [RedirectPlugin] Fixed handling of redirection to different domain with default port
8+
- [RedirectPlugin] Fixed false positive circular detection in RedirectPlugin in cases when target location does not contain path
89

910
## 2.5.0 - 2021-11-26
1011

‎spec/Plugin/RedirectPluginSpec.php

-50
Original file line numberDiff line numberDiff line change
@@ -506,54 +506,4 @@ public function it_throws_circular_redirection_exception_on_alternating_redirect
506506
$promise->shouldReturnAnInstanceOf(HttpRejectedPromise::class);
507507
$promise->shouldThrow(CircularRedirectionException::class)->duringWait();
508508
}
509-
510-
public function it_redirects_http_to_https(
511-
UriInterface $uri,
512-
UriInterface $uriRedirect,
513-
RequestInterface $request,
514-
ResponseInterface $responseRedirect,
515-
RequestInterface $modifiedRequest,
516-
ResponseInterface $finalResponse,
517-
Promise $promise
518-
) {
519-
$responseRedirect->getStatusCode()->willReturn(302);
520-
$responseRedirect->hasHeader('Location')->willReturn(true);
521-
$responseRedirect->getHeaderLine('Location')->willReturn('https://my-site.com/original');
522-
523-
$request->getUri()->willReturn($uri);
524-
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
525-
$uri->__toString()->willReturn('http://my-site.com/original');
526-
$uri->withPath('/original')->willReturn($uri);
527-
$uri->withFragment('')->willReturn($uri);
528-
$uri->withQuery('')->willReturn($uri);
529-
530-
$uri->withScheme('https')->willReturn($uriRedirect);
531-
$uriRedirect->withHost('my-site.com')->willReturn($uriRedirect);
532-
$uriRedirect->withPath('/original')->willReturn($uriRedirect);
533-
$uriRedirect->withFragment('')->willReturn($uriRedirect);
534-
$uriRedirect->withQuery('')->willReturn($uriRedirect);
535-
$uriRedirect->__toString()->willReturn('https://my-site.com/original');
536-
537-
$modifiedRequest->getUri()->willReturn($uriRedirect);
538-
$modifiedRequest->getMethod()->willReturn('GET');
539-
540-
$next = function (RequestInterface $receivedRequest) use ($request, $responseRedirect) {
541-
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
542-
return new HttpFulfilledPromise($responseRedirect->getWrappedObject());
543-
}
544-
};
545-
546-
$first = function (RequestInterface $receivedRequest) use ($modifiedRequest, $promise) {
547-
if (Argument::is($modifiedRequest->getWrappedObject())->scoreArgument($receivedRequest)) {
548-
return $promise->getWrappedObject();
549-
}
550-
};
551-
552-
$promise->getState()->willReturn(Promise::FULFILLED);
553-
$promise->wait()->shouldBeCalled()->willReturn($finalResponse);
554-
555-
$finalPromise = $this->handleRequest($request, $next, $first);
556-
$finalPromise->shouldReturnAnInstanceOf(HttpFulfilledPromise::class);
557-
$finalPromise->wait()->shouldReturn($finalResponse);
558-
}
559509
}

‎src/Plugin/RedirectPlugin.php

+26-1
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,31 @@ private function createUri(ResponseInterface $redirectResponse, RequestInterface
231231
}
232232

233233
$uri = $originalRequest->getUri();
234+
235+
// Redirections can either use an absolute uri or a relative reference https://www.rfc-editor.org/rfc/rfc3986#section-4.2
236+
// If relative, we need to check if we have an absolute path or not
237+
238+
$path = array_key_exists('path', $parsedLocation) ? $parsedLocation['path'] : '';
239+
if (!array_key_exists('host', $parsedLocation) && '/' !== $location[0]) {
240+
// this is a relative-path reference
241+
$originalPath = $uri->getPath();
242+
if ('' === $path) {
243+
$path = $originalPath;
244+
} elseif (($pos = strrpos($originalPath, '/')) !== false) {
245+
$path = substr($originalPath, 0, $pos+1).$path;
246+
} else {
247+
$path = '/'.$path;
248+
}
249+
/* replace '/./' or '/foo/../' with '/' */
250+
$re = ['#(/\./)#', '#/(?!\.\.)[^/]+/\.\./#'];
251+
for( $n=1; $n>0; $path=preg_replace( $re, '/', $path, -1, $n ) ) {
252+
if (null === $path) {
253+
throw new HttpException(sprintf('Failed to resolve Location %s', $location), $originalRequest, $redirectResponse);
254+
}
255+
}
256+
}
234257
$uri = $uri
235-
->withPath(array_key_exists('path', $parsedLocation) ? $parsedLocation['path'] : '')
258+
->withPath($path)
236259
->withQuery(array_key_exists('query', $parsedLocation) ? $parsedLocation['query'] : '')
237260
->withFragment(array_key_exists('fragment', $parsedLocation) ? $parsedLocation['fragment'] : '')
238261
;
@@ -247,6 +270,8 @@ private function createUri(ResponseInterface $redirectResponse, RequestInterface
247270

248271
if (array_key_exists('port', $parsedLocation)) {
249272
$uri = $uri->withPort($parsedLocation['port']);
273+
} elseif (array_key_exists('host', $parsedLocation)) {
274+
$uri = $uri->withPort(null);
250275
}
251276

252277
return $uri;

‎tests/Plugin/RedirectPluginTest.php

+19-3
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,26 @@ function () {}
2626
)->wait();
2727
}
2828

29+
public function provideRedirections(): array
30+
{
31+
return [
32+
'no path on target' => ["https://example.com/path?query=value", "https://example.com?query=value", "https://example.com?query=value"],
33+
'root path on target' => ["https://example.com/path?query=value", "https://example.com/?query=value", "https://example.com/?query=value"],
34+
'redirect to query' => ["https://example.com", "https://example.com?query=value", "https://example.com?query=value"],
35+
'redirect to different domain without port' => ["https://example.com:8000", "https://foo.com?query=value", "https://foo.com?query=value"],
36+
'network-path redirect, preserve scheme' => ["https://example.com:8000", "//foo.com/path?query=value", "https://foo.com/path?query=value"],
37+
'absolute-path redirect, preserve host' => ["https://example.com:8000", "/path?query=value", "https://example.com:8000/path?query=value"],
38+
'relative-path redirect, append' => ["https://example.com:8000/path/", "sub/path?query=value", "https://example.com:8000/path/sub/path?query=value"],
39+
'relative-path on non-folder' => ["https://example.com:8000/path/foo", "sub/path?query=value", "https://example.com:8000/path/sub/path?query=value"],
40+
'relative-path moving up' => ["https://example.com:8000/path/", "../other?query=value", "https://example.com:8000/other?query=value"],
41+
'relative-path with ./' => ["https://example.com:8000/path/", "./other?query=value", "https://example.com:8000/path/other?query=value"],
42+
'relative-path with //' => ["https://example.com:8000/path/", "other//sub?query=value", "https://example.com:8000/path/other//sub?query=value"],
43+
'relative-path redirect with only query' => ["https://example.com:8000/path", "?query=value", "https://example.com:8000/path?query=value"],
44+
];
45+
}
46+
2947
/**
30-
* @testWith ["https://example.com/path?query=value", "https://example.com?query=value", "https://example.com?query=value"]
31-
* ["https://example.com/path?query=value", "https://example.com/?query=value", "https://example.com/?query=value"]
32-
* ["https://example.com", "https://example.com?query=value", "https://example.com?query=value"]
48+
* @dataProvider provideRedirections
3349
*/
3450
public function testTargetUriMappingFromLocationHeader(string $originalUri, string $locationUri, string $targetUri): void
3551
{

0 commit comments

Comments
 (0)