Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[BrowserKit] fixed BC Break for HTTP_HOST header#26244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:2.8frombrizzz:ticket_22933
Nov 26, 2018

Conversation

@brizzz
Copy link
Contributor

@brizzzbrizzz commentedFeb 20, 2018
edited
Loading

QA
Branch?2.7, 2.8, 3.x, 4.x
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes/no
Fixed tickets#22933
LicenseMIT
Doc PRn/a

The situation well described in the original issue. I will add only:

I propose a compromise solution: add to HTTP_HOST header power to override URI when it is relative.

Proposed solution:

  • if the request URI is relative, then use the HTTP_HOST header passed to Client::request() to generate an absolute URI
  • if the request URI is absolute, then ignore the HTTP_HOST header (as it now works)
  • do the same with HTTPS server parameter

Profit:

Before:

$client->setServerParameters(['HTTP_HOST' => 'example.com']);$client->request('GET', '/');$this->assertEquals('http://example.com/', $client->getRequest()->getUri());$client->request('GET', '/', [], [], ['HTTP_HOST' => 'example.com']);$this->assertEquals('http://localhost/', $client->getRequest()->getUri());

Fixed (see last line):

$client->setServerParameters(['HTTP_HOST' => 'example.com']);$client->request('GET', '/');$this->assertEquals('http://example.com/', $client->getRequest()->getUri());$client->request('GET', '/', [], [], ['HTTP_HOST' => 'example.com']);$this->assertEquals('http://example.com/', $client->getRequest()->getUri());

@MathieuMindid
Copy link

Thanks for the fix.
As mentioned in#26257 the problems also exist in last version (3.x and 4.x)

@brizzz
Copy link
ContributorAuthor

Yes, this fix (if its correct) should be merged to all maintained versions. I have edited description to clarify.

@fabpotfabpot modified the milestones:2.7,2.8May 28, 2018
@fabpotfabpot changed the base branch from2.7 to2.8May 28, 2018 06:06
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I finally reviewed this PR, it makes sense to me!

@brizzz
Copy link
ContributorAuthor

this is great news!

@fabpot
Copy link
Member

Thank you@brizzz.

@fabpotfabpot merged commit8c4a594 intosymfony:2.8Nov 26, 2018
fabpot added a commit that referenced this pull requestNov 26, 2018
This PR was merged into the 2.8 branch.Discussion----------[BrowserKit] fixed BC Break for HTTP_HOST header| Q             | A| ------------- | ---| Branch?       | 2.7, 2.8, 3.x, 4.x| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes/no| Fixed tickets |#22933| License       | MIT| Doc PR        | n/aThe situation well described in the original issue. I will add only:-#10549 - makes server parameters to take precedence over URI.-#16265 - partially revererts#10549. Makes server parameters do not affect URI. But this is only true for `Client::request()`. It is still possible to set host for URI by `Client::setServerParameters()` when URI is realative (see examples below).I propose a compromise solution: add to HTTP_HOST header power to override URI when it is relative.Proposed solution:- if the request URI is relative, then use the HTTP_HOST header passed to Client::request() to generate an absolute URI- if the request URI is absolute, then ignore the HTTP_HOST header (as it now works)- do the same with HTTPS server parameterProfit:- fix BC Break- the documentation will be correct  -http://symfony.com/doc/2.8/routing/hostname_pattern.html#testing-your-controllers  -https://symfony.com/doc/2.8/testing.html#testing-configurationBefore:```$client->setServerParameters(['HTTP_HOST' => 'example.com']);$client->request('GET', '/');$this->assertEquals('http://example.com/', $client->getRequest()->getUri());$client->request('GET', '/', [], [], ['HTTP_HOST' => 'example.com']);$this->assertEquals('http://localhost/', $client->getRequest()->getUri());```Fixed (see last line):```$client->setServerParameters(['HTTP_HOST' => 'example.com']);$client->request('GET', '/');$this->assertEquals('http://example.com/', $client->getRequest()->getUri());$client->request('GET', '/', [], [], ['HTTP_HOST' => 'example.com']);$this->assertEquals('http://example.com/', $client->getRequest()->getUri());```Commits-------8c4a594 [BrowserKit] fixed BC Break for HTTP_HOST header; implemented same behaviour for HTTPS server parameter
@Naktibalda
Copy link
Contributor

Naktibalda commentedNov 27, 2018
edited
Loading

It broke my tests, so it is a BC-break.
I was posting Host header to localhost to see if some header functionality is working.

@c33s
Copy link

@nicolas-grekas does this PR really make sense? this PR broke flexible testing of domain specific routes complete or is there a way to do this after the PR? see#32791

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

2.8

Development

Successfully merging this pull request may close these issues.

7 participants

@brizzz@MathieuMindid@fabpot@Naktibalda@c33s@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp