8000 merged branch tiraeth/master-browserkit-redirect (PR #8025) · webmozart/symfony@da6f190 · GitHub
[go: up one dir, main page]

Skip to content

Commit da6f190

Browse files
committed
merged branch tiraeth/master-browserkit-redirect (PR symfony#8025)
This PR was squashed before being merged into the master branch (closes symfony#8025). Discussion ---------- [BrowserKit] should not follow redirects if status code is not 30x | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Currently, BrowserKit operates incorrectly. It follows "redirect" when `Location` header is present, but having just the header is not enough to perform redirection. [RFC-2616](http://tools.ietf.org/html/rfc2616#section-14.30) precisely says that the redirection should be performed only with `30x` status codes. This PR fixes the incorrect behaviour of BrowserKit and make it consist with both the RFC document and with other clients, used for example with Behat. I've found the issue while testing my application with Behat. I was returning `Location` header with `HTTP 201/Created` status code and was surprised that BrowserKit follows the redirection. This PR is for 2.3 version (master) of Symfony. Commits ------- 8f54da7 [BrowserKit] should not follow redirects if status code is not 30x
2 parents 1f8a1b7 + 8f54da7 commit da6f190

File tree

4 files changed

+43
-1
lines changed

4 files changed

+43
-1
lines changed

UPGRADE-2.3.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,3 +263,17 @@ Console
263263
```
264264
if (OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) { ... }
265265
```
266+
267+
BrowserKit
268+
----------
269+
270+
* If you are receiving responses with non-3xx Status Code and Location header
271+
please be aware that you won't be able to use auto-redirects on these kind
272+
of responses.
273+
274+
If you are correctly passing 3xx Status Code with Location header, you
275+
don't have to worry about the change.
276+
277+
If you were using responses with Location header and non-3xx Status Code,
278+
you have to update your code to manually create another request to URL
279+
grabbed from the Location header.

src/Symfony/Component/BrowserKit/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ CHANGELOG
44
2.3.0
55
-----
66

7+
* [BC BREAK] `Client::followRedirect()` won't redirect responses with
8+
a non-3xx Status Code and `Location` header anymore, as per
9+
http://tools.ietf.org/html/rfc2616#section-14.30
10+
711
* added `Client::getInternalRequest()` and `Client::getInternalResponse()` to
812
have access to the BrowserKit internal request and response objects
913

src/Symfony/Component/BrowserKit/Client.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,13 @@ public function request($method, $uri, array $parameters = array(), array $files
328328

329329
$this->cookieJar->updateFromResponse($this->internalResponse, $uri);
330330

331-
$this->redirect = $this->internalResponse->getHeader('Location');
331+
$status = $this->internalResponse->getStatus();
332+
333+
if ($status >= 300 && $status < 400) {
334+
$this->redirect = $this->internalResponse->getHeader('Location');
335+
} else {
336+
$this->redirect = null;
337+
}
332338

333339
if ($this->followRedirects && $this->redirect) {
334340
return $this->crawler = $this->followRedirect();

src/Symfony/Component/BrowserKit/Tests/ClientTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,24 @@ public function testFollowRedirect()
348348
$client->request('GET', 'http://www.example.com/foo/foobar');
349349

350350
$this->assertEquals('http://www.example.com/redirected', $client->getRequest()->getUri(), '->followRedirect() automatically follows redirects if followRedirects is true');
351+
352+
$client = new TestClient();
353+
$client->setNextResponse(new Response('', 201, array('Location' => 'http://www.example.com/redirected')));
354+
$client->request('GET', 'http://www.example.com/foo/foobar');
355+
356+
$this->assertEquals(' 8000 http://www.example.com/foo/foobar', $client->getRequest()->getUri(), '->followRedirect() does not follow redirect if HTTP Code is not 30x');
357+
358+
$client = new TestClient();
359+
$client->setNextResponse(new Response('', 201, array('Location' => 'http://www.example.com/redirected')));
360+
$client->followRedirects(false);
361+
$client->request('GET', 'http://www.example.com/foo/foobar');
362+
363+
try {
364+
$client->followRedirect();
365+
$this->fail('->followRedirect() throws a \LogicException if the request did not respond with 30x HTTP Code');
366+
} catch (\Exception $e) {
367+
$this->assertInstanceof('LogicException', $e, '->followRedirect() throws a \LogicException if the request did not respond with 30x HTTP Code');
368+
}
351369
}
352370

353371
public function testFollowRedirectWithMaxRedirects()

0 commit comments

Comments
 (0)
< 10C4 template id="site-details-dialog">
0