-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection][HttpClient][Routing] Reject vertical tab in URIs #59511
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
src/Symfony/Component/DependencyInjection/Tests/EnvVarProcessorTest.php
Outdated
Show resolved
Hide resolved
32e04f7
to
a35e6ae
Compare
a35e6ae
to
e5c0383
Compare
Why exclude vertical tabs? |
Like horizontal tabs and other control characters, this char should not appear in a URL. We already have a check for control chars, this one can be added for completeness? |
Vertical tabs are not mentioned on the URL spec: https://url.spec.whatwg.org/ |
The URL spec mentions that C0 controls are forbidden, and |
I can't see where the vertical tab needs this special handling. Please link to the exact sentences you have in mind? |
I don't think the vertical tab needs a special check, the proposed patch is not a good idea as explained by your comments. Reading the spec again, I'm wondering if we should still ensure the host doesn't contain C0 control chars, as stated in https://url.spec.whatwg.org/#forbidden-host-code-point ? There are check that ensure there are no leading/trailing C0 chars in the URL, but it does not check the host if I understand correctly |
I'm not sure. Which problem would that solve? |
The code would closer follow the living standard. Indeed parse_url seems to handle it but the output seems "wrong" as the spec says it should be invalid. |
Let's close yes. We don't need a spec-compliant parser here. There are third party libs for that. |
Follows:
Also,
\v
is not supported in the@testWith
annotation, thus converting them to data providers.