Skip to content

[HttpFoundation] Fixed absolute Request URI with default port #29256

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

Conversation

thomasbisignani
Copy link
Contributor

@thomasbisignani thomasbisignani commented Nov 18, 2018

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #29234
License MIT
Doc PR

This PR fixes the #29234 issue, the request URI with default port was not properly generated.

Example :

$request = Request::create('http://test.com:80/foo');
$request->server->set('REQUEST_URI', 'http://test.com:80/foo');

Before this fix, the $request->getUri() method returned http://test.com/:80/foo :

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://test.com/foo'
+'http://test.com/:80/foo'

@thomasbisignani thomasbisignani force-pushed the fix/httpfoundation-request-uri-with-default-port branch 2 times, most recently from 58224ba to dbde28e Compare November 18, 2018 23:28
@thomasbisignani thomasbisignani changed the title [HttpFoundation] Fixed request URI with default port [HttpFoundation] Fixed absolute Request URI with default port Nov 18, 2018
@thomasbisignani thomasbisignani force-pushed the fix/httpfoundation-request-uri-with-default-port branch from dbde28e to 310d4d1 Compare November 19, 2018 08:53
@thomasbisignani thomasbisignani force-pushed the fix/httpfoundation-request-uri-with-default-port branch from 310d4d1 to 2e8f480 Compare November 19, 2018 09:48
@thomasbisignani thomasbisignani force-pushed the fix/httpfoundation-request-uri-with-default-port branch from 2e8f480 to cddce2a Compare November 20, 2018 16:51
@fabpot
Copy link
Member

fabpot commented Nov 24, 2018

Thanks for fixing this bug @thomasbisignani.

@fabpot fabpot merged commit cddce2a into symfony:3.4 Nov 24, 2018
fabpot added a commit that referenced this pull request Nov 24, 2018
…ort (thomasbisignani)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpFoundation] Fixed absolute Request URI with default port

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29234
| License       | MIT
| Doc PR        |

This PR fixes the #29234 issue, the request URI with default port was not properly generated.

Example :

```php
$request = Request::create('http://test.com:80/foo');
$request->server->set('REQUEST_URI', 'http://test.com:80/foo');
```

Before this fix, the `$request->getUri()` method returned `http://test.com/:80/foo` :

```diff
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://test.com/foo'
+'http://test.com/:80/foo'
```

Commits
-------

cddce2a [HttpFoundation] Fixed absolute Request URI with default port
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants