[BrowserKit] should not follow redirects if status code is not 30x #8025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is definitely "just" a bug. So, we need to fix it in 2.1 (no need to create PRs for other branches). |
| @@ -328,7 +328,10 @@ public function request($method, $uri, array $parameters = array(), array $files | |||
|
|
|||
| $this->cookieJar->updateFromResponse($this->internalResponse, $uri); | |||
|
|
|||
| $this->redirect = $this->internalResponse->getHeader('Location'); | |||
| $status = $this->internalResponse->getStatus(); | |||
| $location = $this->internalResponse->getHeader('Location'); | |||
There was a problem hiding this comment.
you should get it only when the status code allows it to avoid useless work
| $this->redirect = $this->internalResponse->getHeader('Location'); | ||
| $status = $this->internalResponse->getStatus(); | ||
|
|
||
| if ($status >= 300 && $status < 400) { |
There was a problem hiding this comment.
HTTP spec says 3xx Redirection but available codes are 30x only and it's a common way of calling redirection status codes - 30x. The conditional, by the way, is a copy-paste from Guzzle.
There was a problem hiding this comment.
Then maybe the upgrade note should be adjusted?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, BrowserKit operates incorrectly. It follows "redirect" when
Locationheader is present, but having just the header is not enough to perform redirection. RFC-2616 precisely says that the redirection should be performed only with30xstatus 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
Locationheader withHTTP 201/Createdstatus code and was surprised that BrowserKit follows the redirection.This PR is for 2.3 version (master) of Symfony.