Skip to content

Commit 5fae651

Browse files
dbudbu
committed
remove body on redirection
1 parent 92b7425 commit 5fae651

File tree

5 files changed

+145
-5
lines changed

5 files changed

+145
-5
lines changed

‎CHANGELOG.md

+7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
# Change Log
22

3+
## 2.6.0 - 2022-09-??
4+
5+
- [RedirectPlugin] Redirection of non GET/HEAD requests with a body now removes the body on follow-up requests.
6+
The plugin needs to find a PSR-7 stream implementation for this to work. If none is found, you can explicitly pass a
7+
PSR-17 StreamFactoryInterface in the `stream_factory` option.
8+
To keep sending the body in all cases, set the `stream_factory` option to null explicitly.
9+
310
## 2.5.1 - 2022-09-29
411

512
### Fixed

‎phpstan.neon.dist

+12
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,18 @@ parameters:
3636
count: 1
3737
path: src/Plugin/RedirectPlugin.php
3838

39+
# phpstan is confused by the optional dependencies. we check for existence first
40+
-
41+
message: "#^Method Http\\\\Client\\\\Common\\\\Plugin\\\\RedirectPlugin::guessStreamFactory\\(\\) should return Psr\\\\Http\\\\Message\\\\StreamFactoryInterface\\|null but returns Nyholm\\\\Psr7\\\\Factory\\\\Psr17Factory\\.$#"
42+
count: 1
43+
path: src/Plugin/RedirectPlugin.php
44+
45+
# phpstan is confused by the optional dependencies. we check for existence first
46+
-
47+
message: "#^Call to static method streamFor\\(\\) on an unknown class GuzzleHttp\\\\Psr7\\\\Utils\\.$#"
48+
count: 1
49+
path: src/Plugin/RedirectPlugin.php
50+
3951
-
4052
message: "#^Method Http\\\\Client\\\\Common\\\\Plugin\\\\RetryPlugin\\:\\:retry\\(\\) should return Psr\\\\Http\\\\Message\\\\ResponseInterface but returns mixed\\.$#"
4153
count: 1

‎spec/Plugin/RedirectPluginSpec.php

+8-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public function it_redirects_on_302(
3737
ResponseInterface $finalResponse,
3838
Promise $promise
3939
) {
40+
$this->beConstructedWith(['stream_factory' => null]);
4041
$responseRedirect->getStatusCode()->willReturn(302);
4142
$responseRedirect->hasHeader('Location')->willReturn(true);
4243
$responseRedirect->getHeaderLine('Location')->willReturn('/redirect');
@@ -81,6 +82,7 @@ public function it_use_storage_on_301(
8182
ResponseInterface $finalResponse,
8283
ResponseInterface $redirectResponse
8384
) {
85+
$this->beConstructedWith(['stream_factory' => null]);
8486
$request->getUri()->willReturn($uri);
8587
$uri->__toString()->willReturn('/original');
8688
$uri->withPath('/redirect')->willReturn($uriRedirect);
@@ -153,6 +155,7 @@ public function it_replace_full_url(
153155
ResponseInterface $finalResponse,
154156
Promise $promise
155157
) {
158+
$this->beConstructedWith(['stream_factory' => null]);
156159
$request->getUri()->willReturn($uri);
157160
$uri->__toString()->willReturn('/original');
158161

@@ -275,6 +278,7 @@ public function it_switch_method_for_302(
275278
ResponseInterface $finalResponse,
276279
Promise $promise
277280
) {
281+
$this->beConstructedWith(['stream_factory' => null]);
278282
$request->getUri()->willReturn($uri);
279283
$uri->__toString()->willReturn('/original');
280284

@@ -367,7 +371,10 @@ public function it_clears_headers(
367371
ResponseInterface $finalResponse,
368372
Promise $promise
369373
) {
370-
$this->beConstructedWith(['preserve_header' => ['Accept']]);
374+
$this->beConstructedWith([
375+
'preserve_header' => ['Accept'],
376+
'stream_factory' => null,
377+
]);
371378

372379
$request->getUri()->willReturn($uri);
373380
$uri->__toString()->willReturn('/original');

‎src/Plugin/RedirectPlugin.php

+65-3
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,20 @@
44

55
namespace Http\Client\Common\Plugin;
66

7+
use GuzzleHttp\Psr7\Utils;
78
use Http\Client\Common\Exception\CircularRedirectionException;
89
use Http\Client\Common\Exception\MultipleRedirectionException;
910
use Http\Client\Common\Plugin;
1011
use Http\Client\Exception\HttpException;
12+
use Http\Discovery\Psr17FactoryDiscovery;
1113
use Http\Promise\Promise;
14+
use Nyholm\Psr7\Factory\Psr17Factory;
1215
use Psr\Http\Message\RequestInterface;
1316
use Psr\Http\Message\ResponseInterface;
17+
use Psr\Http\Message\StreamFactoryInterface;
18+
use Psr\Http\Message\StreamInterface;
1419
use Psr\Http\Message\UriInterface;
20+
use Symfony\Component\OptionsResolver\Options;
1521
use Symfony\Component\OptionsResolver\OptionsResolver;
1622

1723
/**
@@ -101,13 +107,19 @@ final class RedirectPlugin implements Plugin
101107
*/
102108
private $circularDetection = [];
103109

110+
/**
111+
* @var StreamFactoryInterface|null
112+
*/
113+
private $streamFactory;
114+
104115
/**
105116
* @param array{'preserve_header'?: bool|string[], 'use_default_for_multiple'?: bool, 'strict'?: bool} $config
106117
*
107118
* Configuration options:
108119
* - preserve_header: True keeps all headers, false remove all of them, an array is interpreted as a list of header names to keep
109120
* - use_default_for_multiple: Whether the location header must be directly used for a multiple redirection status code (300)
110121
* - strict: When true, redirect codes 300, 301, 302 will not modify request method and body
122+
* - stream_factory: If set, must be a PSR-17 StreamFactoryInterface - if not set, we try to discover one
111123
*/
112124
public function __construct(array $config = [])
113125
{
@@ -116,17 +128,22 @@ public function __construct(array $config = [])
116128
'preserve_header' => true,
117129
'use_default_for_multiple' => true,
118130
'strict' => false,
131+
'stream_factory' => null,
119132
]);
120133
$resolver->setAllowedTypes('preserve_header', ['bool', 'array']);
121134
$resolver->setAllowedTypes('use_default_for_multiple', 'bool');
122135
$resolver->setAllowedTypes('strict', 'bool');
136+
$resolver->setAllowedTypes('stream_factory', [StreamFactoryInterface::class, 'null']);
123137
$resolver->setNormalizer('preserve_header', function (OptionsResolver $resolver, $value) {
124138
if (is_bool($value) && false === $value) {
125139
return [];
126140
}
127141

128142
return $value;
129143
});
144+
$resolver->setDefault('stream_factory', function (Options $options) use ($config): ?StreamFactoryInterface {
145+
return $this->guessStreamFactory();
146+
});
130147
$options = $resolver->resolve($config);
131148

132149
$this->preserveHeader = $options['preserve_header'];
@@ -137,6 +154,8 @@ public function __construct(array $config = [])
137154
$this->redirectCodes[301]['switch'] = false;
138155
$this->redirectCodes[302]['switch'] = false;
139156
}
157+
158+
$this->streamFactory = $options['stream_factory'];
140159
}
141160

142161
/**
@@ -170,7 +189,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
170189

171190
$this->circularDetection[$chainIdentifier][] = (string) $request->getUri();
172191

173-
if (in_array((string) $redirectRequest->getUri(), $this->circularDetection[$chainIdentifier])) {
192+
if (in_array((string) $redirectRequest->getUri(), $this->circularDetection[$chainIdentifier], true)) {
174193
throw new CircularRedirectionException('Circular redirection detected', $request, $response);
175194
}
176195

@@ -186,19 +205,62 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
186205
});
187206
}
188207

208+
/**
209+
* The default only needs to be determined if no value is provided.
210+
*/
211+
public function guessStreamFactory(): ?StreamFactoryInterface
212+
{
213+
if (class_exists(Psr17FactoryDiscovery::class)) {
214+
try {
215+
return Psr17FactoryDiscovery::findStreamFactory();
216+
} catch (\Throwable $t) {
217+
// ignore and try other options
218+
}
219+
}
220+
if (class_exists(Psr17Factory::class)) {
221+
return new Psr17Factory();
222+
}
223+
if (class_exists(Utils::class)) {
224+
return new class() implements StreamFactoryInterface {
225+
public function createStream(string $content = ''): StreamInterface
226+
{
227+
return Utils::streamFor($content);
228+
}
229+
230+
public function createStreamFromFile(string $filename, string $mode = 'r'): StreamInterface
231+
{
232+
throw new \RuntimeException('Internal error: this method should not be needed');
233+
}
234+
235+
public function createStreamFromResource($resource): StreamInterface
236+
{
237+
throw new \RuntimeException('Internal error: this method should not be needed');
238+
}
239+
};
240+
}
241+
242+
return null;
243+
}
244+
189245
private function buildRedirectRequest(RequestInterface $originalRequest, UriInterface $targetUri, int $statusCode): RequestInterface
190246
{
191247
$originalRequest = $originalRequest->withUri($targetUri);
192248

193-
if (false !== $this->redirectCodes[$statusCode]['switch'] && !in_array($originalRequest->getMethod(), $this->redirectCodes[$statusCode]['switch']['unless'])) {
249+
if (false !== $this->redirectCodes[$statusCode]['switch'] && !in_array($originalRequest->getMethod(), $this->redirectCodes[$statusCode]['switch']['unless'], true)) {
194250
$originalRequest = $originalRequest->withMethod($this->redirectCodes[$statusCode]['switch']['to']);
251+
if ('GET' === $this->redirectCodes[$statusCode]['switch']['to'] && $this->streamFactory) {
252+
// if we found a stream factory, remove the request body. otherwise leave the body there.
253+
$originalRequest = $originalRequest->withoutHeader('content-type');
254+
$originalRequest = $originalRequest->withoutHeader('content-length');
255+
$originalRequest = $originalRequest->withBody($this->streamFactory->createStream());
256+
}
195257
}
196258

197259
if (is_array($this->preserveHeader)) {
198260
$headers = array_keys($originalRequest->getHeaders());
199261

200262
foreach ($headers as $name) {
201-
if (!in_array($name, $this->preserveHeader)) {
263+
if (!in_array($name, $this->preserveHeader, true)) {
202264
$originalRequest = $originalRequest->withoutHeader($name);
203265
}
204266
}

‎tests/Plugin/RedirectPluginTest.php

+53-1
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44

55
namespace Plugin;
66

7+
use GuzzleHttp\Psr7\Utils;
78
use Http\Client\Common\Exception\CircularRedirectionException;
89
use Http\Client\Common\Plugin\RedirectPlugin;
910
use Http\Promise\FulfilledPromise;
11+
use Nyholm\Psr7\Factory\Psr17Factory;
1012
use Nyholm\Psr7\Request;
1113
use Nyholm\Psr7\Response;
1214
use PHPUnit\Framework\TestCase;
@@ -27,6 +29,56 @@ function () {}
2729
)->wait();
2830
}
2931

32+
public function testPostGetDropRequestBody(): void
33+
{
34+
$response = (new RedirectPlugin())->handleRequest(
35+
new Request('POST', 'https://example.com/path', ['Content-Type' => 'text/plain', 'Content-Length' => '10'], (new Psr17Factory())->createStream('hello test')),
36+
function (RequestInterface $request) {
37+
$this->assertSame(10, $request->getBody()->getSize());
38+
$this->assertTrue($request->hasHeader('Content-Type'));
39+
$this->assertTrue($request->hasHeader('Content-Length'));
40+
41+
return new FulfilledPromise(new Response(302, ['Location' => 'https://example.com/other']));
42+
},
43+
function (RequestInterface $request) {
44+
$this->assertSame('GET', $request->getMethod());
45+
$this->assertSame(0, $request->getBody()->getSize());
46+
$this->assertFalse($request->hasHeader('Content-Type'));
47+
$this->assertFalse($request->hasHeader('Content-Length'));
48+
49+
return new FulfilledPromise(new Response(200, ['uri' => $request->getUri()->__toString()]));
50+
}
51+
)->wait();
52+
53+
$this->assertSame('https://example.com/other', $response->getHeaderLine('uri'));
54+
}
55+
56+
public function testPostGetNoFactory(): void
57+
{
58+
// We explicitly set the stream factory to null. Same happens if no factory can be found.
59+
// In this case, the redirect will leave the body alone.
60+
$response = (new RedirectPlugin(['stream_factory' => null]))->handleRequest(
61+
new Request('POST', 'https://example.com/path', ['Content-Type' => 'text/plain', 'Content-Length' => '10'], (new Psr17Factory())->createStream('hello test')),
62+
function (RequestInterface $request) {
63+
$this->assertSame(10, $request->getBody()->getSize());
64+
$this->assertTrue($request->hasHeader('Content-Type'));
65+
$this->assertTrue($request->hasHeader('Content-Length'));
66+
67+
return new FulfilledPromise(new Response(302, ['Location' => 'https://example.com/other']));
68+
},
69+
function (RequestInterface $request) {
70+
$this->assertSame('GET', $request->getMethod());
71+
$this->assertSame(10, $request->getBody()->getSize());
72+
$this->assertTrue($request->hasHeader('Content-Type'));
73+
$this->assertTrue($request->hasHeader('Content-Length'));
74+
75+
return new FulfilledPromise(new Response(200, ['uri' => $request->getUri()->__toString()]));
76+
}
77+
)->wait();
78+
79+
$this->assertSame('https://example.com/other', $response->getHeaderLine('uri'));
80+
}
81+
3082
public function provideRedirections(): array
3183
{
3284
return [
@@ -60,6 +112,6 @@ function (RequestInterface $request) {
60112
}
61113
)->wait();
62114
$this->assertInstanceOf(ResponseInterface::class, $response);
63-
$this->assertEquals($targetUri, $response->getHeaderLine('uri'));
115+
$this->assertSame($targetUri, $response->getHeaderLine('uri'));
64116
}
65117
}

0 commit comments

Comments
 (0)