[Validator] Add additional versions (`_NO_PUBLIC`, `_ONLY_PRIV` & `*_ONLY_RES`) in IP address constraint #51777

Ninos · 2023-09-28T21:38:22Z

Q	A
Branch?	6.4
Bug fix?	no
New feature?	yes
Deprecations?	no
License	MIT

Possibility to allow only private or only reserved ips.

Enhancement: Add *_NO_PUBLIC, *_ONLY_PRIV & *_ONLY_RES as possible versions in Ip constraint
Enhancement: Possibility to use all Ip versions in Cidr constraint

~~PS: In Ip constraint I changed the default version from V4 to ALL, think it's time to support also V6 by default :-) Otherwise we can revert that, but normally should not break much...~~

…constraint Added: MAC address constraint Changed: Default version filter from IPv4 to ALL (IPv4 & IPv6)

Fixed: Test for `Ip` constraint

nicolas-grekas

Thanks for the PR. Here are some comments. Changing the defaults look undesired to me since it would change the behavior of existing apps without notice.
Can you also please update the title of your PR so that it can be useful for readers in the auto-generated changelog when doing the release?

UPGRADE-6.4.md

src/Symfony/Component/Validator/CHANGELOG.md

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

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

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

xabbuh · 2023-10-02T08:11:09Z

I am not convinced that a MAC address validator is a common enough use case to be included in the Symfony core.

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

Ninos · 2023-10-05T06:38:58Z

Thanks for the PR. Here are some comments. Changing the defaults look undesired to me since it would change the behavior of existing apps without notice. Can you also please update the title of your PR so that it can be useful for readers in the auto-generated changelog when doing the release?

Thx for review, merged your commits, reverted your comments & changed title :-)

stof · 2023-10-05T09:42:22Z

If we want to change the default version, we would have to make it configurable in the validator:

make the version field of the constraint nullable and default to null
configure a default version in the validator (defaulting to V4 for BC reasons), that is used when the constraint version is null
add a configuration setting in FrameworkBundle to configure the default IP version.

But to me, this deserves a separate PR than this one adding new version constants. It deserves to have a separate discussion instead of being bundled in the same decision to merge or no (note that I would have separated the Mac constraint as well, to allow separating the decisions as well).

nicolas-grekas

LGTM with one comment about naming

nicolas-grekas · 2023-10-18T16:11:17Z

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

+
+        self::V4_ONLY_RES,
+        self::V6_ONLY_RES,
+        self::ALL_ONLY_RES,


RES is super cryptic, let's go with the full RESERVED word

Let's duplicate all consts to add these full-word names as aliases (PRIV also)

Ninos · 2023-11-04T04:33:00Z

Seems my responses are still on pending... I'll reply as seperate comment:

I used the same suffix, which was used as filter flag (e.g. FILTER_FLAG_NO_PRIV_RANGE, see also: https://www.php.net/manual/de/filter.filters.flags.php).

Also this constraint already contains constants with _PRIV & _RES suffix, see already existing constant *_ONLY_PRIV:
https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/Validator/Constraints/Ip.php#L35

We cannot just rename new constants to PRIVATE or RESERVED, this will be inconsistent. We could also rename old constrants to new suffix, but then we need to add additional complexity for backward-compatibility...

Seb33300 · 2023-11-04T16:24:40Z

We cannot just rename new constants to PRIVATE or RESERVED, this will be inconsistent. We could also rename old constants to new suffix, but then we need to add additional complexity for backward-compatibility...

@nicolas-grekas suggested to duplicate all names in his latest comment.

Ninos · 2023-11-05T04:18:19Z

@Seb33300 @nicolas-grekas @OskarStark renamed & deprecated old constants. Hope it's fine now :-)

… constraint

Ninos · 2023-11-06T08:30:50Z

I've also updated Cidr constraint to use possible versions defined in Ip constraint. Please check also :-)

… constraint

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

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

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

src/Symfony/Component/Validator/CHANGELOG.md

nicolas-grekas · 2023-11-20T16:47:14Z

(please rebase, don't merge - you can also squash as we won't keep the history)

Ninos · 2023-11-20T16:51:49Z

(please rebase, don't merge - you can also squash as we won't keep the history)

Ok now I have a problem.. :D Give me some min, may I'll create a new branch if possible to retag this MR :/

Ninos · 2023-11-20T17:19:20Z

@nicolas-grekas I've created a new branch based on 7.1, hope this is fine :( Sry for spamming, I'll try to rebase my mac addresses branch instead :)

See also: #52658

… MAC address (Ninos) This PR was merged into the 7.1 branch. Discussion ---------- [Validator] Add `MacAddress` constraint for validating MAC address | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | License | MIT Possibility to validate against mac address. See also past discussion: #51777 Commits ------- 06ccf62 [Validator] Add `MacAddress` constraint for validating MAC address

…_ONLY_PRIV` & `*_ONLY_RES`) in IP address & CIDR constraint (Ninos) This PR was squashed before being merged into the 7.1 branch. Discussion ---------- [Validator] Add additional versions (`*_NO_PUBLIC`, `*_ONLY_PRIV` & `*_ONLY_RES`) in IP address & CIDR constraint | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | License | MIT Possibility to allow no public, only private or only reserved ips. - Enhancement: Add `*_NO_PUBLIC`, `*_ONLY_PRIV` & `*_ONLY_RES` as possible versions in `Ip` constraint - Enhancement: Possibility to use all `Ip` versions in `Cidr` constraint See also old MR: #51777 Commits ------- 291ef1c [Validator] Add additional versions (`*_NO_PUBLIC`, `*_ONLY_PRIV` & `*_ONLY_RES`) in IP address & CIDR constraint

Ninos added 3 commits September 28, 2023 23:26

Enhancement: Add *_ONLY_PRIV & *_ONLY_RES as possible versions in IP …

a98f828

…constraint Added: MAC address constraint Changed: Default version filter from IPv4 to ALL (IPv4 & IPv6)

Updated: Changelog

9551dc3

Updated: UPGRADE-6.4.md

2b1e888

carsonbot added Status: Needs Review Deprecation Feature labels Sep 28, 2023

carsonbot added this to the 6.4 milestone Sep 28, 2023

Ninos added 6 commits September 28, 2023 23:40

CS

24c73a3

Added: Tests for Mac constraint

2200cd8

Fixed: Test for `Ip` constraint

Fixed: Tests for Mac constraint

3cb3119

Added: OnlyPrivate & OnlyReserved tests

99e03ea

Enhancement: OnlyPrivate & OnlyReserved tests

c11c261

CS

12f126e

nicolas-grekas added the Validator label Sep 29, 2023

carsonbot changed the title ~~Mac address & IP address constraint~~ [Validator] Mac address & IP address constraint Sep 29, 2023

nicolas-grekas reviewed Sep 29, 2023

View reviewed changes

Ninos and others added 8 commits October 5, 2023 08:11

Update src/Symfony/Component/Validator/Constraints/Mac.php

e4e6a24

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

Update src/Symfony/Component/Validator/Constraints/Ip.php

0e6cf5e

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

Update src/Symfony/Component/Validator/Constraints/MacValidator.php

0b7910f

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

Update src/Symfony/Component/Validator/Constraints/Mac.php

719be6c

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

Update src/Symfony/Component/Validator/Constraints/Mac.php

a6eb4f2

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

Update src/Symfony/Component/Validator/Constraints/Mac.php

4b4c5e4

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

Update src/Symfony/Component/Validator/Constraints/Mac.php

9cf46fc

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

CS

9775156

Ninos changed the title ~~[Validator] Mac address & IP address constraint~~ [Validator] Added mac address constraint & additional versions *_ONLY_PRIV & *_ONLY_RES in IP address constraint Oct 5, 2023

Ninos added 2 commits October 6, 2023 07:44

Removed: Mac constraint

646fd5f

[Validator] Remove Mac constraint

26e9f2a

fabpot modified the milestones: 6.4, 7.1 Oct 18, 2023

nicolas-grekas reviewed Oct 18, 2023

View reviewed changes

Ninos added 2 commits November 5, 2023 05:14

[Validator] Renamed constants to full-word names

cfff49a

[Validator] Renamed constants to full-word names

3f58f6b

[Validator] Possibility to use all Ip constraint versions in Cidr…

a0419db

… constraint

Ninos added 4 commits November 6, 2023 09:33

[Validator] Possibility to use all Ip constraint versions in Cidr…

ed09494

… constraint

[Validator] Possibility to use all Ip constraint versions in Cidr…

65b3184

… constraint

[Validator] Possibility to use all Ip constraint versions in Cidr…

b066087

… constraint

[Validator] Fixed tests & added *_NO_PUBLIC

1991107

Ninos changed the title ~~[Validator] Add additional versions (*_ONLY_PRIV & *_ONLY_RES) in IP address constraint~~ [Validator] Add additional versions (*_NO_PUBLIC, *_ONLY_PRIV & *_ONLY_RES) in IP address constraint Nov 6, 2023

[Validator] Fixed tests

cbdd81f

Ninos mentioned this pull request Nov 7, 2023

[Validator] Updated: Ip & Cidr constraints documentation symfony/symfony-docs#19117

Closed

[Validator] Added normalizer to Cidr + better string support

601809c

nicolas-grekas reviewed Nov 20, 2023

View reviewed changes

Merge branch '7.1' into constraints-networking

cc930ef

Ninos added 2 commits November 20, 2023 17:49

[Validator] Small fixes

0d12319

[Validator] Small fixes

b58bb03

Ninos mentioned this pull request Nov 20, 2023

[Validator] Add additional versions (*_NO_PUBLIC, *_ONLY_PRIV & *_ONLY_RES) in IP address & CIDR constraint #52658

Merged

Ninos closed this Nov 20, 2023

Ninos deleted the constraints-networking branch November 21, 2023 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Validator] Add additional versions (`_NO_PUBLIC`, `_ONLY_PRIV` & `*_ONLY_RES`) in IP address constraint #51777

[Validator] Add additional versions (`_NO_PUBLIC`, `_ONLY_PRIV` & `*_ONLY_RES`) in IP address constraint #51777

Ninos commented Sep 28, 2023 •

edited

Loading

nicolas-grekas left a comment

xabbuh commented Oct 2, 2023

Ninos commented Oct 5, 2023

stof commented Oct 5, 2023

nicolas-grekas left a comment

nicolas-grekas Oct 18, 2023

Ninos commented Nov 4, 2023

Seb33300 commented Nov 4, 2023

Ninos commented Nov 5, 2023

Ninos commented Nov 6, 2023

nicolas-grekas commented Nov 20, 2023

Ninos commented Nov 20, 2023

Ninos commented Nov 20, 2023 •

edited

Loading

[Validator] Add additional versions (*_NO_PUBLIC, *_ONLY_PRIV & *_ONLY_RES) in IP address constraint #51777

[Validator] Add additional versions (*_NO_PUBLIC, *_ONLY_PRIV & *_ONLY_RES) in IP address constraint #51777

Conversation

Ninos commented Sep 28, 2023 • edited Loading

nicolas-grekas left a comment

Choose a reason for hiding this comment

xabbuh commented Oct 2, 2023

Ninos commented Oct 5, 2023

stof commented Oct 5, 2023

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas Oct 18, 2023

Choose a reason for hiding this comment

Ninos commented Nov 4, 2023

Seb33300 commented Nov 4, 2023

Ninos commented Nov 5, 2023

Ninos commented Nov 6, 2023

nicolas-grekas commented Nov 20, 2023

Ninos commented Nov 20, 2023

Ninos commented Nov 20, 2023 • edited Loading

[Validator] Add additional versions (`_NO_PUBLIC`, `_ONLY_PRIV` & `*_ONLY_RES`) in IP address constraint #51777

[Validator] Add additional versions (`_NO_PUBLIC`, `_ONLY_PRIV` & `*_ONLY_RES`) in IP address constraint #51777

Ninos commented Sep 28, 2023 •

edited

Loading

Ninos commented Nov 20, 2023 •

edited

Loading