-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 access to raw host in ipaddr module. #599
Conversation
* lib/ipaddr.rb (IPAddr#unmasked_addr): new method. [Fixes rubyGH-599]
An issue in rails (activerecord) showed up which indicates that there are at least some IPAddresses which continue to use the unmasked piece of the address. rails/rails#14857 This is apparently the difference between the "cidr" data type and the "inet" datatype. This change exposes an api so that the correct address type can be used.
The cidr format as specifies requries an unmasked address plus a mask. Currently, we are reporting a masked address plus a mask. This change correctly grabs the unmasked addr for a cidr and uses the masked address for inet types. This change depends on a change to the ruby lib/ipaddr.rb that is currently here. ruby/ruby#599 Thanks to @maciekkolodziej for reporting.
@knu what do you think? |
Have you looked at the ipaddress gem, which you might want to migrate to? It has been a candidate for a replacement for ipaddr in the standard library, and if we need to extend ipaddr I want to make sure API compatibility/similarity with ipaddress is taken into account. |
* lib/ipaddr.rb (IPAddr#unmasked_addr): new method. [Fixes rubyGH-599]
It seems to have a conflict now. Could you rebase this against master? |
As it has not been updated for a while and knu is suggesting to use ipaddress.gem instead, let me close this for now. Please reopen this after resolving conflicts. Thanks. |
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/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`
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/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` - This PR also sets @unmasked_addr within `set`, which covers a few additional edge cases with non-string arguments and with methods like `&` or `succ` that clone the IpAdrr and call `set` Co-authored-by: Jarek Radosz <jradosz@gmail.com>
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 `unmasked`. This was originally implemented back in ruby/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 new, unmasked IPAddr object. - As a result of the above, this version doesn't need to change the signature of `to_s` - This PR also sets @unmasked_addr within `set`, which covers a few additional edge cases with non-string arguments and with methods like `&` or `succ` that clone the IpAdrr and call `set` Co-authored-by: Jarek Radosz <jradosz@gmail.com>
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 `unmasked`. This was originally implemented back in ruby/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 new, unmasked IPAddr object. - As a result of the above, this version doesn't need to change the signature of `to_s` - This PR also sets @unmasked_addr within `set`, which covers a few additional edge cases with non-string arguments and with methods like `&` or `succ` that clone the IpAdrr and call `set` Co-authored-by: Jarek Radosz <jradosz@gmail.com>
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 `unmasked`. This was originally implemented back in ruby/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 new, unmasked IPAddr object. - As a result of the above, this version doesn't need to change the signature of `to_s` - This PR also sets @unmasked_addr within `set`, which covers a few additional edge cases with non-string arguments and with methods like `&` or `succ` that clone the IpAdrr and call `set` Co-authored-by: Jarek Radosz <jradosz@gmail.com>
An issue in rails (activerecord) showed up which indicates that there
are at least some IPAddresses which continue to use the unmasked piece
of the address.
rails/rails#14857
This is apparently the difference between the "cidr" data type and the
"inet" datatype. This change exposes an api so that the correct address
type can be used.