Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BrowserKit] add non-standard port to HTTP_HOST server param #10078

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@kbond
Copy link
Contributor

commented Jan 20, 2014

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets https://github.com/Behat/MinkGoutteDriver/issues/25
License MIT
Doc PR n/a

This fix will allow using BrowserKit with php's webserver.

$server['HTTP_HOST'] = parse_url($uri, PHP_URL_HOST);
if (80 !== ($port = parse_url($uri, PHP_URL_PORT))) {

This comment has been minimized.

Copy link
@fabpot

fabpot Jan 20, 2014

Member

the parenthesis should be removed around the assignment.

This comment has been minimized.

Copy link
@fabpot

fabpot Jan 20, 2014

Member

If the port is empty, you're going to add a : at the end, which is not what you expect. So, you must check against 80 and the empty string.

This comment has been minimized.

Copy link
@jakzal

jakzal Jan 20, 2014

Member

which made two tests from the BrowserKit component fail.

$server['HTTP_HOST'] = parse_url($uri, PHP_URL_HOST);
if (80 !== ($port = parse_url($uri, PHP_URL_PORT))) {
$server['HTTP_HOST'] .= ':' . $port;

This comment has been minimized.

Copy link
@fabpot

fabpot Jan 20, 2014

Member

no spaces around the dot.

@cordoval

This comment has been minimized.

Copy link
Contributor

commented Jan 20, 2014

does this need also a regression test?

@kbond

This comment has been minimized.

Copy link
Contributor Author

commented Jan 20, 2014

I have simplified and added some tests.

@kbond

This comment has been minimized.

Copy link
Contributor Author

commented Jan 23, 2014

I suppose this should be merged to the 2.3 branch.

@fabpot

This comment has been minimized.

Copy link
Member

commented Jan 24, 2014

Thank you @kbond.

fabpot added a commit that referenced this pull request Jan 24, 2014

bug #10078 [BrowserKit] add non-standard port to HTTP_HOST server par…
…am (kbond)

This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closes #10078).

Discussion
----------

[BrowserKit] add non-standard port to HTTP_HOST server param

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | https://github.com/Behat/MinkGoutteDriver/issues/25
| License       | MIT
| Doc PR        | n/a

This fix will allow using BrowserKit with php's webserver.

Commits
-------

5f25639 [BrowserKit] add non-standard port to HTTP_HOST

@fabpot fabpot closed this Jan 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.