Skip to content

[Validator] Use strict type in URL validator #27264

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

Merged
merged 1 commit into from
May 18, 2018

Conversation

mimol91
Copy link

@mimol91 mimol91 commented May 14, 2018

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Using checkDNS option with value true generate error Warning: checkdnsrr(): Type '1' not supported.
In SF 3.4 it was mark as depreciation and silently converted to ANY https://github.com/symfony/symfony/blob/v3.4.9/src/Symfony/Component/Validator/Constraints/UrlValidator.php#L79

Test are failing on Symfony\Component\HttpKernel\Tests\ControllerMetadata\ArgumentMetadataFactoryTest::testSignature1 - I think its not related

@mimol91 mimol91 requested a review from lyrixx as a code owner May 14, 2018 16:09
@mimol91 mimol91 changed the base branch from 3.4 to 4.0 May 14, 2018 16:09
@mimol91 mimol91 changed the title [Validator Usse strict type in url validator [Validator] Use strict type in URL validator May 14, 2018
@mimol91 mimol91 force-pushed the URL-validator-strict branch 2 times, most recently from b61ac0e to 4b00265 Compare May 14, 2018 16:15
@stof
Copy link
Member

stof commented May 14, 2018

I think the strict comparison should be added in the 3.4 branch already

@mimol91 mimol91 changed the base branch from 4.0 to 3.4 May 14, 2018 17:15
@mimol91 mimol91 force-pushed the URL-validator-strict branch from 4b00265 to 2400e71 Compare May 14, 2018 17:15
@ostrolucky
Copy link
Contributor

This is a BC break. Booleans are not relevant here, user can pass numeric strings.

@mimol91
Copy link
Author

mimol91 commented May 14, 2018

@ostrolucky
All of const CHECK_DNS_TYPE_ANY are not numeric. If someone pass numeric as string it will behave in this same way.

@ostrolucky
Copy link
Contributor

I see. All good then!

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone May 14, 2018
@fabpot
Copy link
Member

fabpot commented May 18, 2018

Thank you @mimol91.

@fabpot fabpot merged commit 2400e71 into symfony:3.4 May 18, 2018
fabpot added a commit that referenced this pull request May 18, 2018
This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] Use strict type in URL validator

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
 Fixed tickets | -
| License       | MIT
| Doc PR        | -

Using `checkDNS` option with value `true` generate error `Warning: checkdnsrr(): Type '1' not supported`.
In SF 3.4  it was mark as depreciation and silently converted to `ANY`  https://github.com/symfony/symfony/blob/v3.4.9/src/Symfony/Component/Validator/Constraints/UrlValidator.php#L79

~~Test are failing on `Symfony\Component\HttpKernel\Tests\ControllerMetadata\ArgumentMetadataFactoryTest::testSignature1` - I think its not related~~

Commits
-------

2400e71 use strict compare in url validator
This was referenced May 21, 2018
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.

7 participants