Skip to content

[Router] added \Z as regex end for route requirement #37714

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

Closed
wants to merge 1 commit into from

Conversation

zlodes
Copy link
Contributor

@zlodes zlodes commented Jul 31, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR symfony/symfony-docs#...

In previous PR (#37711) I added /A and /z, but $ in regex is /z + /Z.

Thanks @rvitaliy: #37711 (comment)

References:
^ and $: https://www.pcre.org/original/doc/html/pcrepattern.html#TOC1
\A, \z, \Z: https://www.pcre.org/original/doc/html/pcrepattern.html#SEC5

The \A, \Z, and \z assertions differ from the traditional circumflex and dollar (described in the next section) in that they only ever match at the very start and end of the subject string, whatever options are set. Thus, they are independent of multiline mode. These three assertions are not affected by the PCRE_NOTBOL or PCRE_NOTEOL options, which affect only the behaviour of the circumflex and dollar metacharacters. However, if the startoffset argument of pcre_exec() is non-zero, indicating that matching is to start at a point other than the beginning of the subject, \A can never match. The difference between \Z and \z is that \Z matches before a newline at the end of the string as well as at the very end, whereas \z matches only at the end.

@nicolas-grekas
Copy link
Member

This looks like unneeded precaution to me. New lines are not legit in URLs.

@nicolas-grekas nicolas-grekas added this to the next milestone Aug 1, 2020
@@ -563,7 +563,7 @@ private function sanitizeRequirement(string $key, string $regex)

if ('$' === substr($regex, -1)) {
$regex = substr($regex, 0, -1);
} elseif (\strlen($regex) - 2 === strpos($regex, '\\z')) {
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to the PR: couldn't this be written as this?
} elseif (false !== strpos($regex, '\\z', -2)) {

Copy link
Contributor Author

@zlodes zlodes Aug 3, 2020

Choose a reason for hiding this comment

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

@nicolas-grekas tried strpos and strrpos:

strpos(): Offset not contained in string in .... (strpos doesn't accept negative offset)
strrpos(): Offset is greater than the length of haystack string

@zlodes
Copy link
Contributor Author

zlodes commented Aug 3, 2020

#37714 (comment) ⬆️

@zlodes zlodes closed this Aug 3, 2020
@zlodes zlodes deleted the route-requirements-add-regex-Z branch August 3, 2020 09:43
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020
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.

3 participants