-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Routing] Generate URLs in compliance with PHP_QUERY_RFC3986 #19637
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
jameshalsall
commented
Aug 16, 2016
•
edited
Loading
edited
Q | A |
---|---|
Branch? | master |
Bug fix? | no |
New feature? | yes |
BC breaks? | no? |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #19625 |
License | MIT |
Doc PR | reference to the documentation PR, if any |
…tyle in LintCommand (chalasr) This PR was merged into the 3.2-dev branch. Discussion ---------- [Yaml] Avoid using both Input/Output and SymfonyStyle in LintCommand | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ Fixed some inconsistencies/mistakes from the original YamLintCommand. Commits ------- dd84b7f [Yaml] Avoid using both Input/Output and SymfonyStyle in LintCommand
…te shared services. (hhamon) This PR was merged into the 3.2-dev branch. Discussion ---------- [DependencyInjection] deprecate access to private shared services. | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | symfony#19117 | License | MIT | Doc PR | ~ Commits ------- 4ed70c4 [DependencyInjection] deprecate access to private shared services. Fixes issue symfony#19117.
…ctors (Ener-Getick) This PR was merged into the 3.2-dev branch. Discussion ---------- [Serializer] Allow to use easily static constructors | Q | A | ------------- | --- | Branch? | "master" | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#19027 (comment) | License | MIT | Doc PR | - This PR allows to simply use static constructors to instantiate objects with the serializer by extending the default normalizers. Commits ------- 9be6484 [Serializer] Allow to use easily static constructors
This PR was merged into the 2.8 branch. Discussion ---------- fixed test | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Commits ------- bac531c fixed test
* 2.7: [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For [Console] Decouple SymfonyStyle from TableCell
* 2.8: [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For fixed test [Console] Decouple SymfonyStyle from TableCell
* 3.0: [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For fixed test [Console] Decouple SymfonyStyle from TableCell
* 3.1: [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For fixed test [Console] Decouple SymfonyStyle from TableCell
* 2.7: [HttpKernel] Inline ValidateRequestListener logic into HttpKernel fixed HttpKernel dependencies after symfony#18688 Conflicts: src/Symfony/Component/HttpKernel/composer.json
This PR was merged into the 2.8 branch. Discussion ---------- [FrameworkBundle] Fix fixtures | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#19173 symfony#19223 | License | MIT | Doc PR | - Commits ------- d175477 [FrameworkBundle] Fix fixtures
* 2.8: [FrameworkBundle] Fix fixtures [HttpKernel] Inline ValidateRequestListener logic into HttpKernel fixed HttpKernel dependencies after symfony#18688 Conflicts: src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_services.txt src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/definition_1.txt src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/definition_2.txt src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/event_dispatcher_1_events.txt src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/legacy_synchronized_service_definition_1.txt src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/legacy_synchronized_service_definition_2.txt src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/parameter.txt src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/route_collection_1.txt src/Symfony/Bundle/FrameworkBundle/composer.json src/Symfony/Component/HttpKernel/composer.json
* 3.0: [FrameworkBundle] Fix fixtures [HttpKernel] Inline ValidateRequestListener logic into HttpKernel fixed HttpKernel dependencies after symfony#18688 Conflicts: src/Symfony/Component/HttpKernel/HttpKernel.php src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php
* 2.7: [ci] Upgrade phpunit wrapper deps
* 2.8: [ci] Upgrade phpunit wrapper deps
* 3.0: [ci] Upgrade phpunit wrapper deps
…y removing deprecated factory (HeahDude) This PR was merged into the 3.1 branch. Discussion ---------- [DoctrineBridge] fixed DoctrineChoiceLoaderTest by removing deprecated factory | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ Commits ------- 3f86eae [DoctrineBridge] fixed DoctrineChoiceLoaderTest by removing deprecated factory
…HeahDude) This PR was merged into the 3.0 branch. Discussion ---------- [Form] fixed ChoiceTypeTest after symfony#17822 | Q | A | ------------- | --- | Branch? | 3.0 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ Commits ------- 777c193 [Form] fixed ChoiceTypeTest after symfony#17822
* 3.0: [Form] fixed ChoiceTypeTest after symfony#17822
* 3.1: [Form] fixed ChoiceTypeTest after symfony#17822 [DoctrineBridge] fixed DoctrineChoiceLoaderTest by removing deprecated factory [ci] Upgrade phpunit wrapper deps [FrameworkBundle] Fix fixtures [HttpKernel] Inline ValidateRequestListener logic into HttpKernel fixed HttpKernel dependencies after symfony#18688
…iler (javiereguiluz) This PR was squashed before being merged into the 3.1 branch (closes symfony#18934). Discussion ---------- Fixed some issues of the AccessDecisionManager profiler | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#19022 https://github.com/symfony/symfony-standard/issues/968 schmittjoh/JMSSecurityExtraBundle#207 | License | MIT | Doc PR | - Commits ------- 082f1b5 Fixed some issues of the AccessDecisionManager profiler
This PR was merged into the 3.2-dev branch. Discussion ---------- Fix merge issues | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- 7701fea Fix merge issues
This PR was merged into the 3.2-dev branch. Discussion ---------- [Console] fix input stream related tests | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Commits ------- 1cca740 [Console] fix input stream related tests
…places TagAwareRedisAdapter) (nicolas-grekas) This PR was merged into the 3.2-dev branch. Discussion ---------- [Cache] Add generic TagAwareAdapter wrapper (replaces TagAwareRedisAdapter) | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#6858 This PR replaces TagAwareRedisAdapter introduced in symfony#19047 by a generic `TagAwareAdapter` that works with any two adapters. Commits ------- 288308b [Cache] Add generic TagAwareAdapter wrapper (replaces TagAwareRedisAdapter)
…eniedException (Nicofuma) This PR was merged into the 3.2-dev branch. Discussion ---------- [Security] Expose the required roles in AccessDeniedException | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT Nowadays it is more and more common to protect some sensitive actions and part of a website using 2FA or some re-authentication mechanism (per example, on Github you have to enter your password again when you add an ssh key). But currently, in Symfony, it is really hard to implement without having to duplicate the logic, provide an explicit list of URLs to protect or hack into the security component. A good way to achieve that would be to add a special role (like IS_AUTHENTICATED_FULLY) and use it in the access map. But it requires us to be able to have a custom logic in an ExceptionListener depending on the roles behind an AccessDeniedException. With this patch we could write an ExceptionListener of this kind (a similar logic could also be used in an AccessDeniedHandler): ```php public function onKernelException(GetResponseForExceptionEvent $event) { $exception = $event->getException(); do { if ($exception instanceof AccessDeniedException) { foreach ($exception->getAttributes() as $role) { if ($role === 'IS_AUTHENTICATED_2FA' && !$this->accessDecisionManager->decide($this->tokenStorage->getToken(), $role, $exception->getObject())) { // Start 2FA } } } } while (null !== $exception = $exception->getPrevious()); } ``` Replaces symfony#18661 Commits ------- 6618c18 [Security] Expose the required roles in AccessDeniedException
… exceptions on duplicates (Alex Pott) This PR was squashed before being merged into the 3.2-dev branch (closes symfony#19529). Discussion ---------- Add Yaml::PARSE_EXCEPTION_ON_DUPLICATE to throw exceptions on duplicates | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#19526 | License | MIT | Doc PR | Commits ------- cb362f2 Add Yaml::PARSE_EXCEPTION_ON_DUPLICATE to throw exceptions on duplicates
…leofeyer) This PR was squashed before being merged into the 3.2-dev branch (closes symfony#19511). Discussion ---------- Use a dedicated exception in the file locator | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This PR adds a dedicated `FileLocatorFileNotFoundException` class to the file locator, so it is possible to catch file locator exceptions separately from invalid argument exceptions. ```php try { foreach ($container->get('file_locator')->locate('file.php', null, false) as $file) { include $file; } } catch (\InvalidArgumentException $e) { // this will catch both file locator exceptions as well as // any invalid argument exception thrown in an included file } ``` With the dedicated exceptions, we could do this: ```php try { foreach ($container->get('file_locator')->locate('file.php', null, false) as $file) { include $file; } } catch (FileLocatorFileNotFoundException $e) { // this will only ignore file locator exceptions } ``` Commits ------- ee4adaa Use a dedicated exception in the file locator
* 2.7: [travis] fix after box updates Console: Fix indentation of Help: section of txt usage help [Intl] Update ICU data to 57.1 [Config] Improved test Added class existence check if is_subclass_of() fails in compiler passes
* 2.8: [travis] fix after box updates Console: Fix indentation of Help: section of txt usage help [Intl] Update ICU data to 57.1 [Config] Improved test Added class existence check if is_subclass_of() fails in compiler passes
* 3.1: [travis] fix after box updates Console: Fix indentation of Help: section of txt usage help [Intl] Update ICU data to 57.1 [Config] Improved test Added class existence check if is_subclass_of() fails in compiler passes
* 2.7: [appveyor] Fix cache handling
* 2.8: [appveyor] Fix cache handling
* 3.1: [appveyor] Fix cache handling
…olm) This PR was squashed before being merged into the 3.2-dev branch (closes symfony#19602). Discussion ---------- [Workflow] Updated link to the documentation | Q | A | ------------- | --- | Branch? | "master" | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | The link to the docs was broken because of the big documentation restructure a couple of weeks back. Commits ------- d96e861 [Workflow] Updated link to the documentation
This PR was merged into the 3.2-dev branch. Discussion ---------- [Yaml] update changelog and upgrade files | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#19504 | License | MIT | Doc PR | Commits ------- 3cd2932 [Yaml] update changelog and upgrade files
…grekas) This PR was merged into the 3.2-dev branch. Discussion ---------- [Cache] Drop TaggedCacheItemInterface | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Introducing this interface was a mistake, we should drop it: the CacheItem class is final, and it can be instantiated only through a cache adapter. Which means there is no way to reuse this interface in any meaningful way as far as extensibility/interoperability is concerned. Commits ------- 624890b [Cache] Drop TaggedCacheItemInterface
@@ -266,7 +266,13 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa | |||
$fragment = isset($extra['_fragment']) ? $extra['_fragment'] : ''; | |||
unset($extra['_fragment']); | |||
|
|||
if ($extra && $query = http_build_query($extra, '', '&')) { | |||
if (defined('PHP_QUERY_RFC3986')) { |
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.
In which PHP version was it added?
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.
5.4, the reason I've done it this way is to ease merging of upstream changes on 2.x branch
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.
5.4 (the test can be removed in Symfony >= 3.0).
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.
The reason I asked was that you submitted the PR on master :) If this is a bug fix, this should be merged on 2.7 instead first.
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.
And if this is for master, then the constant is always defined.
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.
You're right :)
Isn't it a bug fix instead of a new feature? |
I'm not so sure, it could have unintended side-effects depending on people's expectations of the |
Closed in favour of #19639 which is rebased on 2.7 |