[Routing] fix URL generation with look-around requirements #27601
Conversation
|
ping @schmittjoh since you had some interest into these also. |
src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
Outdated
Show resolved
Hide resolved
|
@nicolas-grekas , yeah, this is actually a pull-request from us. I've talked with Nasim, and she has suggested a slightly different fix now. Instead of turning off the validation entirely for the variable, we just remove validation of the look-around parts. This has the advantage that the value itself is still validated. |
|
Comments are applied:
|
|
@nicolas-grekas @stof Ready for another review of this PR? |
|
From a performance PoV, I think it would be better to perform this regex transformation at compile time, by adding a new (optional) index in the token (if it is not there, we would use the unmodified pattern). But this would probably require doing that change in master only, considering it as a new feature (which could be fine. the original issue #10548 was tagged as an enhancement, not as a bug). |
nicolas-grekas
changed the title
nicolas-grekas
changed the title
nicolas-grekas
force-pushed
the
ticket_10548
branch
3 times, most recently
from
9aa09e1
to
4042e1b
Compare
nicolas-grekas
force-pushed
the
ticket_10548
branch
2 times, most recently
from
578eb1b
to
1e9c795
Compare
nicolas-grekas
force-pushed
the
ticket_10548
branch
from
1e9c795
to
c474451
Compare
|
Thank you @nasimnabavi. |
… (nasimnabavi) This PR was merged into the 3.4 branch. Discussion ---------- [Routing] fix URL generation with look-around requirements | Q | A | ------------- | --- | Branch? | 2.8 up to 4.1 for bug fixes | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #10548 | License | MIT | Doc PR | If you have a non-catchable pattern in requirements like f.e. a positive lookahead (.+(?=/foo/)), the generator will not accept the parameter as the parameter itself cannot fulfil the requirement, but only matches in the context of the entire path. This fix looks for lookAround in the path and ignores checking the requirements if any lookAround exists. Commits ------- c474451 [Routing] fix URL generation with look-around requirements
If you have a non-catchable pattern in requirements like f.e. a positive lookahead (.+(?=/foo/)), the generator will not accept the parameter as the parameter itself cannot fulfil the requirement, but only matches in the context of the entire path.
This fix looks for lookAround in the path and ignores checking the requirements if any lookAround exists.