Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add IPAddr#to_unmasked_string #3498

Conversation

composerinteralia
Copy link
Contributor

Prior to this commit there was no way to retrieve the unmasked address
for an IPAddr. For example, given the IPAddr below there is no
method or instance variable that would return "1.2.3.4".

IPAddr.new("1.2.3.4/8")

This poses a problem for Rails in supporting the PostgreSQL inet type.
Rails uses IPAddr for both the cidr and inet types, but at the moment
cannot fully support the inet type, which "accepts values with nonzero
bits to the right of the netmask". This came up originally in
rails/rails#14857, and more recently in
rails/rails#40138. The change in this commit
would allow us to support the inet type without moving to another
library.

This commit holds onto the address before the mask is applied, in a new
instance variable @unmasked_addr. It then exposes this via
to_unmasked_string.

This was originally implemented back in
#599, but it got stale and was
eventually closed. There are also a few differences with that PR:

  • That PR set up the instance variable inside of mask!, which meant
    that the new method was broken in the case of an IPAddr with no
    mask.
  • That PR introduced a method that returned a to_s-like value, whereas
    this one returns a to_string-like value. This seems to fit better
    with the method name. And if folks need to get the to_s value they
    can always wrap the return value in another IPAddr and call to_s
    on that.
  • As a result of the above, this version doesn't need to change the
    signature of to_s

Prior to this commit there was no way to retrieve the unmasked address
for an `IPAddr`. For example, given the `IPAddr` below there is no
method or instance variable that would return `"1.2.3.4"`.

```rb
IPAddr.new("1.2.3.4/8")
```

This poses a problem for Rails in supporting the PostgreSQL inet type.
Rails uses `IPAddr` for both the cidr and inet types, but at the moment
cannot fully support the inet type, which "accepts values with nonzero
bits to the right of the netmask". This came up originally in
rails/rails#14857, and more recently in
rails/rails#40138. The change in this commit
would allow us to support the inet type without moving to another
library.

This commit holds onto the address before the mask is applied, in a new
instance variable `@unmasked_addr`. It then exposes this via
`to_unmasked_string`.

This was originally implemented back in
ruby#599, but it got stale and was
eventually closed. There are also a few differences with that PR:

- That PR set up the instance variable inside of `mask!`, which meant
  that the new method was broken in the case of an `IPAddr` with no
  mask.
- That PR introduced a method that returned a `to_s`-like value, whereas
  this one returns a `to_string`-like value. This seems to fit better
  with the method name. And if folks need to get the `to_s` value they
  can always wrap the return value in another `IPAddr` and call `to_s`
  on that.
- As a result of the above, this version doesn't need to change the
  signature of `to_s`
@jeremyevans
Copy link
Contributor

Please submit this pull request to the ipaddr repository: https://github.com/ruby/ipaddr/pulls

@jeremyevans jeremyevans closed this Sep 1, 2020
@composerinteralia
Copy link
Contributor Author

composerinteralia commented Sep 1, 2020

Oops! Will do. Thanks!

ruby/ipaddr#26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants