8000 [Routing] Generate URLs in compliance with PHP_QUERY_RFC3986 by jameshalsall · Pull Request #19639 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Routing] Generate URLs in compliance with PHP_QUERY_RFC3986 #19639

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

[Routing] Generate URLs in compliance with PHP_QUERY_RFC3986 #19639

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0e6bf6
Update UrlGenerator::generate() so it encodes in compliance with PHP_…
Aug 16, 2016
4c9eb00
Fix breaking test under 5.3.x
Aug 16, 2016
389e446
Remove redundant code, fix UrlGenerator::testUrlEncoding()
jameshalsall Aug 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix breaking test under 5.3.x
  • Loading branch information
James Halsall committed Aug 16, 2016
commit 4c9eb0070917fbcc19519bd6ead605b050d17261
10 changes: 10 additions & 0 deletions src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,16 @@ public function testGenerateWithSpecialRouteName()

public function testUrlEncoding()
{
if (defined('PHP_QUERY_RFC3986')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was it merged with this that is not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is needed, that was only added in PHP 5.4

Copy link
Contributor

Choose a reason for hiding this comment

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

it was merged in master which requires 5.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was merged to 2.7

Copy link
Contributor Author
@jameshalsall jameshalsall Aug 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

oh, well it was targeted at 2.7 - I can see Fabien merged to master instead - I will send another PR shortly to remove the defined() checks

Copy link
Member

Choose a reason for hiding this comment

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

Because I made the change after the merge.

Copy link
Contributor

Choose a reason for hiding this comment

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

No you didn't do it for this file. See https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php#L328

Copy link
Member

Choose a reason for hiding this comment

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

oops, sorry about that. fixed now. Good catch as always :)

$expectedPath = '/app.php/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id'
.'/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id'
.'?query=%40%3A%5B%5D/%28%29%2A%27%22%20%2B%2C%3B-._~%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id';
} else {
$expectedPath = '/app.php/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id'
.'/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id'
.'?query=%40%3A%5B%5D/%28%29%2A%27%22+%2B%2C%3B-._%7E%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id';
}

// This tests the encoding of reserved characters that are used for delimiting of URI components (defined in RFC 3986)
// and other special ASCII chars. These chars are tested as static text path, variable path and query param.
$chars = '@:[]/()*\'" +,;-._~&$<>|{}%\\^`!?foo=bar#id';
Expand Down
0