Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[BrowserKit] Fixed server HTTP_HOST port uri conversion#11356
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
stof commentedJul 9, 2014
The issue is that you specify |
stof commentedJul 9, 2014
hmm, actually, it does include it. It is SERVER_NAME which does not include it. |
bcremer commentedJul 9, 2014
So this means thiscommit should be reverted? We stumbled upon this error using goutte`s history functions: |
bcremer commentedJul 10, 2014
Any news here, or anything I can help to get this merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you remove the spaces around the dot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed.
fabpot commentedJul 23, 2014
👍 |
1 similar comment
romainneutron commentedJul 23, 2014
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
a preg_quote is required aroundparse_url($uri, PHP_URL_HOST).$port, because:
var_dump(parse_url('abc.+def:9090',PHP_URL_HOST));string(8)"abc.+def"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nicolas-grekas
I'm not sure if I can follow you.
You propose to change the code to:
if ($port = parse_url($uri, PHP_URL_PORT)) { $port = ':'.$port;}$uri = preg_replace('{^(https?\://)'.preg_quote(parse_url($uri, PHP_URL_HOST).$port).'}', '${1}'.$server['HTTP_HOST'], $uri);May you please provide me with a failing testcase like the following so I can verify the problem?
$client = new TestClient();$server = array('HTTP_HOST' => 'www.example.com:8000');$parameters = array();$files = array();$client->request('GET', 'http://example.com', $parameters, $files, $server);$this->assertEquals('http://www.example.com:8000', $client->getRequest()->getUri(), '->request() uses HTTP_HOST to add port');fabpot commentedJul 25, 2014
Closing in favor of#11469 |
…cremer, fabpot)This PR was merged into the 2.3 branch.Discussion---------- [BrowserKit] Fixed server HTTP_HOST port uri conversion| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#11356| License | MIT| Doc PR | n/aSee#11356Commits-------103fd88 [BrowserKit] refactor code and fix unquoted regexf401ab9 Fixed server HTTP_HOST port uri conversion
This will fix URI conversion using the HTTP_HOST server header.
Steps to reproduce:
We discovered this issue duringmink testing usinggoutte on a non standard port.