Skip to content

Commit 0db0c12

Browse files
sagikazarmarksagikazarmark
authored
Fix redirect from http -> https (#43)
* fix(RedirectPlugin): use full URLs for comparing request redirections * chore(*): updated CHANGELOG * Fix test, and add test for http -> https
2 parents 94e22b5 + 0eefc2c commit 0db0c12

File tree

3 files changed

+92
-25
lines changed

3 files changed

+92
-25
lines changed

‎CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@
22

33
## 1.3.0 - Unreleased
44

5+
### Added
56
- Add HttpClientPool client to leverage load balancing and fallback mechanism [see the documentation](http://docs.php-http.org/en/latest/components/client-common.html) for more details
67

8+
### Changed
9+
- RedirectPlugin: use the full URL instead of the URI to properly keep track of redirects
10+
711
## 1.2.1 - 2016-07-26
812

913
### Changed

‎spec/Plugin/RedirectPluginSpec.php

+82-19
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,19 @@ function it_redirects_on_302(
3636
$responseRedirect->hasHeader('Location')->willReturn(true);
3737
$responseRedirect->getHeaderLine('Location')->willReturn('/redirect');
3838

39-
$request->getRequestTarget()->willReturn('/original');
4039
$request->getUri()->willReturn($uri);
4140
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
41+
$uri->__toString()->willReturn('/original');
4242

4343
$uri->withPath('/redirect')->willReturn($uriRedirect);
4444
$uriRedirect->withFragment('')->willReturn($uriRedirect);
4545
$uriRedirect->withQuery('')->willReturn($uriRedirect);
46+
$uriRedirect->__toString()->willReturn('/redirect');
4647

47-
$modifiedRequest->getRequestTarget()->willReturn('/redirect');
48+
$modifiedRequest->getUri()->willReturn($uriRedirect);
4849
$modifiedRequest->getMethod()->willReturn('GET');
4950

51+
5052
$next = function (RequestInterface $receivedRequest) use($request, $responseRedirect) {
5153
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
5254
return new FulfilledPromise($responseRedirect->getWrappedObject());
@@ -67,7 +69,7 @@ function it_redirects_on_302(
6769
$finalPromise->wait()->shouldReturn($finalResponse);
6870
}
6971

70-
function it_use_storage_on_301(UriInterface $uriRedirect, RequestInterface $request, RequestInterface $modifiedRequest)
72+
function it_use_storage_on_301(UriInterface $uri, UriInterface $uriRedirect, RequestInterface $request, RequestInterface $modifiedRequest)
7173
{
7274
$this->beAnInstanceOf('spec\Http\Client\Common\Plugin\RedirectPluginStub');
7375
$this->beConstructedWith($uriRedirect, '/original', '301');
@@ -76,12 +78,15 @@ function it_use_storage_on_301(UriInterface $uriRedirect, RequestInterface $requ
7678
throw new \Exception('Must not be called');
7779
};
7880

79-
$request->getRequestTarget()->willReturn('/original');
81+
$request->getUri()->willReturn($uri);
82+
$uri->__toString()->willReturn('/original');
8083
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
8184

82-
$modifiedRequest->getRequestTarget()->willReturn('/redirect');
85+
$modifiedRequest->getUri()->willReturn($uriRedirect);
8386
$modifiedRequest->getMethod()->willReturn('GET');
8487

88+
$uriRedirect->__toString()->willReturn('/redirect');
89+
8590
$this->handleRequest($request, $next, function () {});
8691
}
8792

@@ -98,8 +103,8 @@ function it_stores_a_301(
98103
$this->beAnInstanceOf('spec\Http\Client\Common\Plugin\RedirectPluginStub');
99104
$this->beConstructedWith($uriRedirect, '', '301');
100105

101-
$request->getRequestTarget()->willReturn('/301-url');
102106
$request->getUri()->willReturn($uri);
107+
$uri->__toString()->willReturn('/301-url');
103108

104109
$responseRedirect->getStatusCode()->willReturn('301');
105110
$responseRedirect->hasHeader('Location')->willReturn(true);
@@ -111,9 +116,11 @@ function it_stores_a_301(
111116

112117
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
113118

114-
$modifiedRequest->getRequestTarget()->willReturn('/redirect');
119+
$modifiedRequest->getUri()->willReturn($uriRedirect);
115120
$modifiedRequest->getMethod()->willReturn('GET');
116121

122+
$uriRedirect->__toString()->willReturn('/redirect');
123+
117124
$next = function (RequestInterface $receivedRequest) use($request, $responseRedirect) {
118125
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
119126
return new FulfilledPromise($responseRedirect->getWrappedObject());
@@ -142,7 +149,8 @@ function it_replace_full_url(
142149
ResponseInterface $finalResponse,
143150
Promise $promise
144151
) {
145-
$request->getRequestTarget()->willReturn('/original');
152+
$request->getUri()->willReturn($uri);
153+
$uri->__toString()->willReturn('/original');
146154

147155
$responseRedirect->getStatusCode()->willReturn('302');
148156
$responseRedirect->hasHeader('Location')->willReturn(true);
@@ -158,9 +166,11 @@ function it_replace_full_url(
158166

159167
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
160168

161-
$modifiedRequest->getRequestTarget()->willReturn('/redirect');
169+
$modifiedRequest->getUri()->willReturn($uriRedirect);
162170
$modifiedRequest->getMethod()->willReturn('GET');
163171

172+
$uriRedirect->__toString()->willReturn('/redirect');
173+
164174
$next = function (RequestInterface $receivedRequest) use($request, $responseRedirect) {
165175
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
166176
return new FulfilledPromise($responseRedirect->getWrappedObject());
@@ -179,15 +189,16 @@ function it_replace_full_url(
179189
$this->handleRequest($request, $next, $first);
180190
}
181191

182-
function it_throws_http_exception_on_no_location(RequestInterface $request, ResponseInterface $responseRedirect)
192+
function it_throws_http_exception_on_no_location(RequestInterface $request, UriInterface $uri, ResponseInterface $responseRedirect)
183193
{
184194
$next = function (RequestInterface $receivedRequest) use($request, $responseRedirect) {
185195
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
186196
return new FulfilledPromise($responseRedirect->getWrappedObject());
187197
}
188198
};
189199

190-
$request->getRequestTarget()->willReturn('/original');
200+
$request->getUri()->willReturn($uri);
201+
$uri->__toString()->willReturn('/original');
191202
$responseRedirect->getStatusCode()->willReturn('302');
192203
$responseRedirect->hasHeader('Location')->willReturn(false);
193204

@@ -196,15 +207,16 @@ function it_throws_http_exception_on_no_location(RequestInterface $request, Resp
196207
$promise->shouldThrow('Http\Client\Exception\HttpException')->duringWait();
197208
}
198209

199-
function it_throws_http_exception_on_invalid_location(RequestInterface $request, ResponseInterface $responseRedirect)
210+
function it_throws_http_exception_on_invalid_location(RequestInterface $request, UriInterface $uri, ResponseInterface $responseRedirect)
200211
{
201212
$next = function (RequestInterface $receivedRequest) use($request, $responseRedirect) {
202213
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
203214
return new FulfilledPromise($responseRedirect->getWrappedObject());
204215
}
205216
};
206217

207-
$request->getRequestTarget()->willReturn('/original');
218+
$request->getUri()->willReturn($uri);
219+
$uri->__toString()->willReturn('/original');
208220
$responseRedirect->getHeaderLine('Location')->willReturn('scheme:///invalid');
209221

210222
$responseRedirect->getStatusCode()->willReturn('302');
@@ -256,7 +268,8 @@ function it_switch_method_for_302(
256268
ResponseInterface $finalResponse,
257269
Promise $promise
258270
) {
259-
$request->getRequestTarget()->willReturn('/original');
271+
$request->getUri()->willReturn($uri);
272+
$uri->__toString()->willReturn('/original');
260273

261274
$responseRedirect->getStatusCode()->willReturn('302');
262275
$responseRedirect->hasHeader('Location')->willReturn(true);
@@ -270,7 +283,8 @@ function it_switch_method_for_302(
270283
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
271284
$modifiedRequest->getUri()->willReturn($uriRedirect);
272285

273-
$modifiedRequest->getRequestTarget()->willReturn('/redirect');
286+
$modifiedRequest->getUri()->willReturn($uriRedirect);
287+
$uriRedirect->__toString()->willReturn('/redirect');
274288
$modifiedRequest->getMethod()->willReturn('POST');
275289
$modifiedRequest->withMethod('GET')->willReturn($modifiedRequest);
276290

@@ -303,7 +317,8 @@ function it_clears_headers(
303317
) {
304318
$this->beConstructedWith(['preserve_header' => ['Accept']]);
305319

306-
$request->getRequestTarget()->willReturn('/original');
320+
$request->getUri()->willReturn($uri);
321+
$uri->__toString()->willReturn('/original');
307322

308323
$responseRedirect->getStatusCode()->willReturn('302');
309324
$responseRedirect->hasHeader('Location')->willReturn(true);
@@ -316,7 +331,8 @@ function it_clears_headers(
316331

317332
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
318333

319-
$modifiedRequest->getRequestTarget()->willReturn('/redirect');
334+
$modifiedRequest->getUri()->willReturn($uriRedirect);
335+
$uriRedirect->__toString()->willReturn('/redirect');
320336
$modifiedRequest->getMethod()->willReturn('GET');
321337
$modifiedRequest->getHeaders()->willReturn(['Accept' => 'value', 'Cookie' => 'value']);
322338
$modifiedRequest->withoutHeader('Cookie')->willReturn($modifiedRequest);
@@ -347,8 +363,8 @@ function it_throws_circular_redirection_exception(UriInterface $uri, UriInterfac
347363
$this->beAnInstanceOf('spec\Http\Client\Common\Plugin\RedirectPluginStubCircular');
348364
$this->beConstructedWith(spl_object_hash((object)$first));
349365

350-
$request->getRequestTarget()->willReturn('/original');
351366
$request->getUri()->willReturn($uri);
367+
$uri->__toString()->willReturn('/original');
352368

353369
$responseRedirect->getStatusCode()->willReturn('302');
354370
$responseRedirect->hasHeader('Location')->willReturn(true);
@@ -360,7 +376,7 @@ function it_throws_circular_redirection_exception(UriInterface $uri, UriInterfac
360376

361377
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
362378
$modifiedRequest->getUri()->willReturn($uriRedirect);
363-
$modifiedRequest->getRequestTarget()->willReturn('/redirect');
379+
$uriRedirect->__toString()->willReturn('/redirect');
364380
$modifiedRequest->getMethod()->willReturn('GET');
365381

366382
$next = function (RequestInterface $receivedRequest) use($request, $responseRedirect) {
@@ -373,6 +389,53 @@ function it_throws_circular_redirection_exception(UriInterface $uri, UriInterfac
373389
$promise->shouldReturnAnInstanceOf('Http\Promise\RejectedPromise');
374390
$promise->shouldThrow('Http\Client\Common\Exception\CircularRedirectionException')->duringWait();
375391
}
392+
393+
function it_redirects_http_to_https(
394+
UriInterface $uri,
395+
UriInterface $uriRedirect,
396+
RequestInterface $request,
397+
ResponseInterface $responseRedirect,
398+
RequestInterface $modifiedRequest,
399+
ResponseInterface $finalResponse,
400+
Promise $promise
401+
) {
402+
$responseRedirect->getStatusCode()->willReturn('302');
403+
$responseRedirect->hasHeader('Location')->willReturn(true);
404+
$responseRedirect->getHeaderLine('Location')->willReturn('https://my-site.com/original');
405+
406+
$request->getUri()->willReturn($uri);
407+
$request->withUri($uriRedirect)->willReturn($modifiedRequest);
408+
$uri->__toString()->willReturn('http://my-site.com/original');
409+
410+
$uri->withScheme('https')->willReturn($uriRedirect);
411+
$uriRedirect->withHost('my-site.com')->willReturn($uriRedirect);
412+
$uriRedirect->withPath('/original')->willReturn($uriRedirect);
413+
$uriRedirect->withFragment('')->willReturn($uriRedirect);
414+
$uriRedirect->withQuery('')->willReturn($uriRedirect);
415+
$uriRedirect->__toString()->willReturn('https://my-site.com/original');
416+
417+
$modifiedRequest->getUri()->willReturn($uriRedirect);
418+
$modifiedRequest->getMethod()->willReturn('GET');
419+
420+
$next = function (RequestInterface $receivedRequest) use($request, $responseRedirect) {
421+
if (Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
422+
return new FulfilledPromise($responseRedirect->getWrappedObject());
423+
}
424+
};
425+
426+
$first = function (RequestInterface $receivedRequest) use($modifiedRequest, $promise) {
427+
if (Argument::is($modifiedRequest->getWrappedObject())->scoreArgument($receivedRequest)) {
428+
return $promise->getWrappedObject();
429+
}
430+
};
431+
432+
$promise->getState()->willReturn(Promise::FULFILLED);
433+
$promise->wait()->shouldBeCalled()->willReturn($finalResponse);
434+
435+
$finalPromise = $this->handleRequest($request, $next, $first);
436+
$finalPromise->shouldReturnAnInstanceOf('Http\Promise\FulfilledPromise');
437+
$finalPromise->wait()->shouldReturn($finalResponse);
438+
}
376439
}
377440

378441
class RedirectPluginStub extends RedirectPlugin

‎src/Plugin/RedirectPlugin.php

+6-6
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,9 @@ public function __construct(array $config = [])
134134
public function handleRequest(RequestInterface $request, callable $next, callable $first)
135135
{
136136
// Check in storage
137-
if (array_key_exists($request->getRequestTarget(), $this->redirectStorage)) {
138-
$uri = $this->redirectStorage[$request->getRequestTarget()]['uri'];
139-
$statusCode = $this->redirectStorage[$request->getRequestTarget()]['status'];
137+
if (array_key_exists((string) $request->getUri(), $this->redirectStorage)) {
138+
$uri = $this->redirectStorage[(string) $request->getUri()]['uri'];
139+
$statusCode = $this->redirectStorage[(string) $request->getUri()]['status'];
140140
$redirectRequest = $this->buildRedirectRequest($request, $uri, $statusCode);
141141

142142
return $first($redirectRequest);
@@ -157,14 +157,14 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
157157
$this->circularDetection[$chainIdentifier] = [];
158158
}
159159

160-
$this->circularDetection[$chainIdentifier][] = $request->getRequestTarget();
160+
$this->circularDetection[$chainIdentifier][] = (string) $request->getUri();
161161

162-
if (in_array($redirectRequest->getRequestTarget(), $this->circularDetection[$chainIdentifier])) {
162+
if (in_array((string) $redirectRequest->getUri(), $this->circularDetection[$chainIdentifier])) {
163163
throw new CircularRedirectionException('Circular redirection detected', $request, $response);
164164
}
165165

166166
if ($this->redirectCodes[$statusCode]['permanent']) {
167-
$this->redirectStorage[$request->getRequestTarget()] = [
167+
$this->redirectStorage[(string) $request->getUri()] = [
168168
'uri' => $uri,
169169
'status' => $statusCode,
170170
];

0 commit comments

Comments
 (0)