Awesome. Thank you for your contribution. Your code will definitely improve Symfony code base by fixing potential security error and many projects will benefit of that.

I made some tests. This (non-valid) URL isn't validated (which is great) http://example.com/exploit.html?hel lo, I think we should add it to the test suite.

I also tested this other URL that is considered as valid but is not: http://example.com/exploit.html?tçest this is a problem with many non-ascii chars. Do you plan to fix it? Or your fix (that actually fix some things) will remain as it? It may be kept as it as there is no security issue in that case, even browsers will interpret it as standard URL.

@Nek- , thanks for your review! I just extended test cases with your example (http://example.com/exploit.html?hel lo). As for non-ascii chars I'm not sure. But previously they were treated as valid: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php#L104

@e-moe it's valid as DNS but not as query string (where is must be url-encoded). But well, it's fine to me, I'm just noticing it.

oh, I see your point. If needed it can be fixed. Let's wait for other reviews and then we will be able to make a decision

@nicolas-grekas , I just fixed code based on your comments.

should I create new PR for 2.7 branch?

as for BC break - new constraint is more strict comparing to old version. but new one is closer to RFC

should it be considered as a BC break?

It's not a BC break! It's a fix.

Thank you @e-moe.

I have a template link field with Url validator. By template I mean http://www.domain.com/script.php?value=${PLACEHOLDER}.
After this pr Url validator triggers an error. With the real link it is okay, but that do you recommend for me? The only way is to write my own UrlTemplate validator?

@Aliance yes.

I'm sorry to read it breaks your code but this is the attempted behavior of the URL Validator. (here was a potential security issue that is now fixed)