Cidrv4 validation is faulty

[hjwk]

• [x] I have looked at the documentation here first?
• [x] I have looked at the examples provided that may showcase my question here?

Package version eg. v9, v10:

v10

Issue, Question or Enhancement:

cidrv4 validation does not detect invalid cidrv4 such as 172.56.1.0/16 (as can be verified here https://ipduh.com/ip/cidr/)

Code sample, to showcase or reproduce:

package main

import (
	"fmt"
	"github.com/go-playground/validator/v10"
)

func main() {
	validate := validator.New()

	cidr := "172.56.1.1/16"

	err := validate.Var(cidr, "required,cidrv4")
	if err != nil {
		fmt.Println(err)
	} else {
		fmt.Println("Valid cidrv4")
	}
}

[martinlehoux]

Hi ! I think the doc explains that this module choice is to correct this issue in the parsing.

What is the level of confidence on your tool to validate CIDR blocks? Is that implementing the RFC?

And if we were to fix this, do I understand that a CIDR block address must be the start of the boundary?

// ParseCIDR parses s as a CIDR notation IP address and prefix length,
// like "192.0.2.0/24" or "2001:db8::/32", as defined in
// RFC 4632 and RFC 4291.
//
// It returns the IP address and the network implied by the IP and
// prefix length.
// For example, ParseCIDR("192.0.2.1/24") returns the IP address
// 192.0.2.1 and the network 192.0.2.0/24.

[hjwk]

Hi,

Not a super high level of confidence on the tool but I am confident that a CIDR block address must be the start of the boundary.

Source: RFC 4632

I guess the current validation is simply regexp based ?

[deankarn]

Happy to accept a PR to correct :)

[martinlehoux]

@deankarn I can do a PR, it's actually very little change, but a breaking change if I'm correct. Almost all tests in TestCIDRv4Validation assume the new rule to be false

So What do you think? Should we make the change and keep it for a new major?

[martinlehoux]

I made the PR so you can see what it really means

[mhristache]

I was using cidrv4 to validate that an IP address including a prefixlen was provided but it's no longer working after the changes implemented in #945. Was I using it wrong? Is there any other validator that can do such a check?

Also, the change was introduced in a v10 patch release even though it's mentioned as breaking in the commit title. I think it would be nice for more regards to backward compatibility when doing such changes.

Thanks

[the-hotmann]

I understand the approach, but also for me it breaks things I did not think it would/should break.

Now the subnet 192.168.178.10/24 in first place is an invalid subnet. I can totally understand this. But please instead of overwriting the cidrv4 option, please just add it as cidrv4-strict. Wouldn't this be the better approach?

I definitely think so!

• It would not break things
• gives people freedom of choice to use what they want

I would please you guys to bring back the old cidrv4 and name this cidrv4-strict, so everyone is happy :)