Make the IPv4|6 regex much more strict

[daenney]

This will correctly match IPv4 and IPv6 addresses and discard invalid ones.

[daenney]

There's something wrong with rspec(-puppet) and Ruby 2.1.5:

/home/travis/build/puppetlabs/puppetlabs-stdlib/vendor/bundle/ruby/2.1.0/gems/rspec-core-2.99.2/lib/rspec/core/hooks.rb:512:in `extract_scope_from': You must explicitly give a scope (:each, :all, or :suite) when using symbols as metadata for a hook. (ArgumentError)

[daenney]

Though much more strict it still passes all the tests that we had defined so this Won't Break Anything™️ while properly rejecting anything that's not an IP address according to the specs.

Also, the previous IPv6 regex did not support addresses with a :: in there it would seem. Which is totally valid. What is curious though is that the tests seem fine with it... I would expect things to have failed.

[daenney]

Should I extend this to support CIDR matching for IPv4 and 6 too? Both the old and the new regexes don't support notations like ::1/128 for example.

[daenney]

Poke @hunner and @HelenCampbell

[daenney]

@DavidS Do you know why the tests explode on Ruby 2.1.5?

[DavidS]

@daenney it seems that a change somewhere causes rspec-core 2.99 to be installed. That seems to cause all sorts of issues.

[DavidS]

@daenney I flushed travis' gem caches for stdlib, and rekicked your cells. This seems to have fixed the issue. I do not completely understand why, tho.

[DavidS]

Thanks for the patch, but the Compat types are specifically designed to match the behaviour of the validate_* functions bug-for-bug. See https://tickets.puppetlabs.com/browse/MODULES-3534 for this specific set.

Feel free to add properly designed IP types as Stdlib::IPv4Address, Stdlib::IPv6Address and Stdlib::IPAddress.

[igalic]

has anyone considered asking @thrnio for merging their module into stdlib? https://github.com/thrnio/puppet-ip
(or maybe just depend on it?)

[hunner]

I don't know that persisting bugs is a good option. Okay, so it'll be a slight migration hurdle for people with broken manifests; it's still a bug and should be fixed. No one should be taking advantage of this anyway and so shouldn't actually run into it.

[DavidS]

@hunner the whole point of the ::compat:: types is bug-for-bug compatibility. There is no reason to NOT add properly designed IP types as Stdlib::IPv4Address, Stdlib::IPv6Address and Stdlib::IPAddress, other than nobody's currently doing the work for them.

[hunner]

Oh I see I see, it's not just fixing them, but specifically making the change somewhere other than Stdlib::Compat::*

[DavidS]

exactly.

On 19 October 2016 at 18:48, Hunter Haugen notifications@github.com wrote:

Oh I see I see, it's not just fixing them, but specifically making the
change somewhere other than Stdlib::Compat::*

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#660 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABsQDJsa3k9ZBhNNqtN2X7Rsmi8RUyNks5q1lfdgaJpZM4KLxKQ
.