Fix returning invalid ip address #3
Conversation
|
@nhinds can you review? |
pedrocarrico
force-pushed
the
fix-returning-invalid-ip-address
branch
from
5437722
to
24f2b17
Compare
lib/public_ip/service/simple.rb
Outdated
|
|
||
| ip_address = extract_ip(response) | ||
|
|
||
| unless IPV4_REGEX.match(ip_address) |
There was a problem hiding this comment.
About half of the services currently return my IPv6 address. This will now cause those services to fail instead of returning the correct address...
There was a problem hiding this comment.
Services from PublicIp.list_services that consistently return my IPv6 address:
:smart_ip:what_is_my_ip_address:i_can_haz_ip:ident_me:wtf_is_my_ip
There was a problem hiding this comment.
@nhinds That is interesting, I'll try to broaden the regex to allow both IPv4 and IPv6 addresses.
Thanks for the review 👍
pedrocarrico
force-pushed
the
fix-returning-invalid-ip-address
branch
3 times, most recently
from
2c91231
to
90d5658
Compare
|
@nhinds sorry to get back to you after so long hope you're still using the gem, can I get a re-review again before merging? I'll bump gem version when this hits master. |
pedrocarrico
force-pushed
the
fix-returning-invalid-ip-address
branch
from
90d5658
to
683cd00
Compare
| @@ -12,7 +12,8 @@ def self.headers | |||
| end | |||
|
|
|||
| def self.parse_ip_address(response_body) | |||
| Nokogiri::HTML(response_body).css('.ip div').text | |||
| doc = Nokogiri::HTML(response_body) | |||
| doc.at('h3:contains("Your Public IPv4 is: ")').text.strip.sub('Your Public IPv4 is: ', '') | |||
There was a problem hiding this comment.
When the page load fails, this line now fails with "undefined method text' for nil:NilClass" because .atreturns either the matchingElementornil. This error is not caught by PublicIp.get_ip` so it gets thrown all the way to callers.
Previously it would return "", because .css(...) returns a NodeSet which always implements .text.
There was a problem hiding this comment.
For me, that service returns "You have viewed this page 5 times. Your lookup limit has been reached." after calling it through PublicIp.get_ip a few times, which is probably why I'm getting errors here.
|
Other than the new error coming from the |
pedrocarrico
force-pushed
the
fix-returning-invalid-ip-address
branch
from
683cd00
to
ae8e6bc
Compare
|
@nhinds Ok, I managed to replicate the issue, I believe everything should be OK now. In the future I might add some service specific exceptions but for now this should be fine. |
|
Thanks, everything is working from my machine now -- Thanks for tackling this :) |
|
Thanks for the reviews @nhinds , pushed the gem with these changes. |
This explicitly returns a
PublipIp::Service::InvalidIpAddressexception when the extracted ip address is not a valid one.