[Validator] Deprecated "checkDNS" option in Url constraint #25516

ro0NL · 2017-12-15T13:50:03Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	yes
Fixed tickets	#23538
License	MIT
Doc PR	symfony/symfony-docs#8921

nicolas-grekas · 2017-12-18T19:49:55Z

src/Symfony/Component/Validator/Constraints/UrlValidator.php

@@ -90,6 +90,8 @@ public function validate($value, Constraint $constraint)
                throw new InvalidOptionsException(sprintf('Invalid value for option "checkDNS" in constraint %s', get_class($constraint)), array('checkDNS'));
            }

+            @trigger_error(sprintf('The "checkDNS" option in "%s" is deprecated since 4.1 and will be removed in 5.0.', Url::class), E_USER_DEPRECATED);


since version 4.1
I'm wondering: should we add a few words why? like eg
The "checkDNS" option in "%s" is deprecated since 4.1 and will be removed in 5.0. It's too fragile to be relied upon.

the notice should be before the above "if", isn't it?

Not sure if this is too fragile, but the false positive rate is so high that it's not reliable. I think reliable is perhaps better.

nicolas-grekas · 2017-12-20T09:36:45Z

src/Symfony/Component/Validator/Constraints/UrlValidator.php

@@ -73,6 +73,8 @@ public function validate($value, Constraint $constraint)
        }

        if ($constraint->checkDNS) {
+            @trigger_error(sprintf('The "checkDNS" option in "%s" is deprecated since version 4.1 and will be removed in 5.0. Its false positive rate is too high to be relied upon.', Url::class), E_USER_DEPRECATED);


I would just suggest a dash: false-positive

xabbuh · 2017-12-22T14:29:18Z

UPGRADE-4.1.md

@@ -30,6 +30,7 @@ Validator

 * The `Email::__construct()` 'strict' property is deprecated and will be removed in 5.0. Use 'mode'=>"strict" instead.
 * Calling `EmailValidator::__construct()` method with a boolean parameter is deprecated and will be removed in 5.0, use `EmailValidator("strict")` instead.
+ * Deprecated `checkDNS` option in `Url` constraint. It will be removed in 5.0.


Deprecated the checkDNS option in the Url [...]

xabbuh · 2017-12-22T14:30:00Z

UPGRADE-5.0.md

@@ -29,7 +29,7 @@ Validator

 * The `Email::__construct()` 'strict' property has been removed. Use 'mode'=>"strict" instead.
 * Calling `EmailValidator::__construct()` method with a boolean parameter has been removed, use `EmailValidator("strict")` instead.
-
+ * Removed `checkDNS` option in `Url` constraint.


Removed the checkDNS option from the Url constraint.

xabbuh · 2017-12-22T14:30:13Z

src/Symfony/Component/Validator/CHANGELOG.md

+4.1.0
+-----
+
+ * Deprecated `checkDNS` option in `Url` constraint. It will be removed in 5.0.


Deprecated the checkDNS option in the Url [...]

xabbuh · 2017-12-22T14:30:58Z

src/Symfony/Component/Validator/Constraints/Url.php

@@ -21,18 +21,57 @@
 */
 class Url extends Constraint
 {
+    /**
+     * @deprecated since version 4.1, to be removed in 5.0


since Symfony 4.1 (see #25565)

ro0NL · 2017-12-22T14:50:19Z

Forgot to deprecate $dnsMessage option. Now updated. Should it be mentioned explicitly? In general it's only enabled thru $checkDNS so fine to me as is.

xabbuh · 2017-12-22T15:10:44Z

As people may forgot to not use the dnsMessage option when dropping checkDNS it might be good to deprecate it explicitly too.

ro0NL · 2017-12-23T12:45:12Z

Deprecation now moved to constraint constructor.

xabbuh · 2017-12-26T20:19:31Z

UPGRADE-4.1.md

@@ -30,6 +30,7 @@ Validator

 * The `Email::__construct()` 'strict' property is deprecated and will be removed in 5.0. Use 'mode'=>"strict" instead.
 * Calling `EmailValidator::__construct()` method with a boolean parameter is deprecated and will be removed in 5.0, use `EmailValidator("strict")` instead.
+ * Deprecated the `checkDNS` and `dnsMessage` options in the `Url` constraint. They will be removed in 5.0.


[...] options of the [...]

xabbuh · 2017-12-26T20:19:43Z

UPGRADE-5.0.md

@@ -29,7 +29,7 @@ Validator

 * The `Email::__construct()` 'strict' property has been removed. Use 'mode'=>"strict" instead.
 * Calling `EmailValidator::__construct()` method with a boolean parameter has been removed, use `EmailValidator("strict")` instead.
-
+ * Removed the `checkDNS` and `dnsMessage` options in the `Url` constraint.


[...] options from the [...]

xabbuh · 2017-12-26T20:19:52Z

src/Symfony/Component/Validator/CHANGELOG.md

+4.1.0
+-----
+
+ * Deprecated the `checkDNS` and `dnsMessage` options in the `Url` constraint. They will be removed in 5.0.


[...] options of the [...]

fabpot · 2017-12-31T04:43:17Z

Thank you @ro0NL.

iisisrael · 2018-08-17T18:48:15Z

I'm actually sad to see this go. Adding the functionality back into our project as a customized extension was not difficult, but I think it odd that there was so little discussion around the deprecation. If the underlying functionality is deemed inconclusive because of false positives, that's something that should be understood by the consuming application. We know that, and use this functionality as just one piece of a greater validation strategy. Having it as an option was just that, an option.

iisisrael · 2018-08-17T18:49:22Z

Also, the same functionality continues to exist undeprecated as the checkHost and checkMX options in the EmailValidator.

This PR was merged into the 5.0-dev branch. Discussion ---------- [Validator] Remove checkDNS option in Url | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no  | Deprecations? | no | Tests pass? | yes  | Fixed tickets | #...  | License | MIT | Doc PR | symfony/symfony-docs#...  See #25516 Commits ------- 885703f [Validator] Remove checkDNS option in Url

carsonbot added Status: Needs Review Validator Feature Deprecation labels Dec 15, 2017

nicolas-grekas reviewed Dec 18, 2017

View reviewed changes

nicolas-grekas approved these changes Dec 20, 2017

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Dec 20, 2017

nicolas-grekas added this to the 4.1 milestone Dec 20, 2017

xabbuh reviewed Dec 22, 2017

View reviewed changes

ro0NL mentioned this pull request Dec 22, 2017

Removed checkDNS option in Url constraint symfony/symfony-docs#8921

Closed

[Validator] Deprecated "checkDNS" option in Url constraint

63df13d

xabbuh reviewed Dec 26, 2017

View reviewed changes

ro0NL added 3 commits December 27, 2017 12:43

Update UPGRADE-4.1.md

17554c9

Update UPGRADE-5.0.md

b9f77eb

Update CHANGELOG.md

3a38b81

fabpot approved these changes Dec 31, 2017

View reviewed changes

fabpot closed this in 38b946e Dec 31, 2017

ro0NL deleted the checkdnsrr branch December 31, 2017 07:10

javiereguiluz mentioned this pull request Jan 3, 2018

[Validator] Url and Email validator possible bug #25666

Closed

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged

ro0NL mentioned this pull request Oct 1, 2018

[Validator] Email validation false-positive due to relative host #28665

Closed

ro0NL mentioned this pull request May 29, 2019

[Validator] Remove checkDNS option in Url #31681

Merged

fabpot mentioned this pull request Jul 20, 2022

[Validator] add checkTldDns option to UrlValidator #46921

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Validator] Deprecated "checkDNS" option in Url constraint #25516

[Validator] Deprecated "checkDNS" option in Url constraint #25516

ro0NL commented Dec 15, 2017 •

edited

Loading

nicolas-grekas Dec 18, 2017 •

edited

Loading

nicolas-grekas Dec 18, 2017

fabpot Dec 18, 2017

nicolas-grekas Dec 20, 2017

xabbuh Dec 22, 2017 •

edited

Loading

xabbuh Dec 22, 2017

xabbuh Dec 22, 2017

xabbuh Dec 22, 2017

ro0NL commented Dec 22, 2017

xabbuh commented Dec 22, 2017

ro0NL commented Dec 23, 2017

xabbuh Dec 26, 2017

xabbuh Dec 26, 2017

xabbuh Dec 26, 2017

fabpot commented Dec 31, 2017

iisisrael commented Aug 17, 2018

iisisrael commented Aug 17, 2018 •

edited

Loading

[Validator] Deprecated "checkDNS" option in Url constraint #25516

[Validator] Deprecated "checkDNS" option in Url constraint #25516

Conversation

ro0NL commented Dec 15, 2017 • edited Loading

nicolas-grekas Dec 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xabbuh Dec 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL commented Dec 22, 2017

xabbuh commented Dec 22, 2017

ro0NL commented Dec 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Dec 31, 2017

iisisrael commented Aug 17, 2018

iisisrael commented Aug 17, 2018 • edited Loading

ro0NL commented Dec 15, 2017 •

edited

Loading

nicolas-grekas Dec 18, 2017 •

edited

Loading

xabbuh Dec 22, 2017 •

edited

Loading

iisisrael commented Aug 17, 2018 •

edited

Loading