Add Socket::IPAddress.valid?

[straight-shoota]

This patch adds utility methods IPAddress.valid? as well as .valid_v4? and .valid_v6? which enable testing whether a string describes a valid IP address.

A practical use case is validation of a cookie's Domain attribute which can either be a domain name or an IPv4 address. I'm sure it can be useful for other purposes as well.

The implementation actually tries to parse the string into an InAddr or In6Addr and returns true if that was successful. This can probably be more performant with a custom implementation, but for now I've just adapted what was already used for the constructor.

[Blacksmoke16]

👍 It'll definitely migrate the logic for https://athenaframework.org/Validator/Constraints/IP/ to use this verus the current ugly regexes.

I also tested the implementation against the spec suite for the constraint and was all green.

old  10.79k ( 92.72µs) (± 2.18%)  34.7kB/op  48.16× slower
new 519.40k (  1.93µs) (± 1.92%)  2.19kB/op        fastest

EDIT: Added benchmark between the two

[straight-shoota]

I realized there is already Socket.ip? which is essentially equivalent to the new Socket::IPAddress.valid?.
Should we stick with the existing name, i.e. rename valid_v4? and valid_v6? to ip4?, ip6? and move them to Socket namespace?
Or do we keep the proposed names and deprecate Socket.ip?.

Socket.ip? is definitely shorter and still pretty unambiguous. Socket::IPAddress.valid? is more explicit, though.

[Blacksmoke16]

@straight-shoota I think it'd make more sense to have them on the actual IPAddress type.

[Blacksmoke16]

@straight-shoota Any plans on revisiting this?

[straight-shoota]

I think we should go with the proposed action to add the new IPAddress.valid? methods and deprecate Socket.ip? (I added a commit for that).
This PR just needs a review.

[sdogruyol]

Thank you @straight-shoota 🙏

[zw963]

Hi, when run code HTTP::Client.get "https://getip.pub", i get following error come from std-lib.

In /home/zw963/Crystal/share/crystal/src/openssl/ssl/socket.cr:15:11

 15 | {% if LibSSL.has_method?(:ssl_get0_param) %}
      ^
Warning: expanding macro


There was a problem expanding macro 'macro_140443166686832'

Called macro defined in /home/zw963/Crystal/share/crystal/src/openssl/ssl/socket.cr:15:11

 15 | {% if LibSSL.has_method?(:ssl_get0_param) %}

Which expanded to:

    1 | 
    2 |             param = LibSSL.ssl_get0_param(@ssl)
    3 | 
 >  4 |             if ::Socket.ip?(hostname)
    5 |               unless LibCrypto.x509_verify_param_set1_ip_asc(param, hostname) == 1
    6 |                 raise OpenSSL::Error.new("X509_VERIFY_PARAM_set1_ip_asc")
    7 |               end
    8 |             else
    9 |               unless LibCrypto.x509_verify_param_set1_host(param, hostname, 0) == 1
   10 |                 raise OpenSSL::Error.new("X509_VERIFY_PARAM_set1_host")
   11 |               end
   12 |             end
   13 |           
Warning: Deprecated Socket.ip?. Use `IPAddress.valid?` instead

A total of 1 warnings were found.

https://github.com/crystal-lang/crystal/blob/master/src/openssl/ssl/socket.cr#L18