Allow passing IPAddrs to allow_ip #444
Conversation
| @@ -33,7 +33,7 @@ class Middleware | |||
| # Adds an address to the set of IP addresses allowed to access Better | |||
| # Errors. | |||
| def self.allow_ip!(addr) | |||
| ALLOWED_IPS << IPAddr.new(addr) | |||
| ALLOWED_IPS << (addr.is_a?(IPAddr) ? addr : IPAddr.new(addr)) | |||
There was a problem hiding this comment.
What about to make it more safe? (code not tested)
case addr
when String then IPAddr.new(addr)
when IPAddr then addr
else raise 'Unknown IP address type. Please specify it as a IPAddr compatible string like "1.1.1.1" or pass IPAddr object.'
endThere was a problem hiding this comment.
IMHO it doesn't add much since IPAddr will already raise something like IPAddr::AddressFamilyError (address family must be specified) if you pass invalid input. I can add it if you prefer, though.
There was a problem hiding this comment.
I agree, it seems like the error raised by IPAddr would be sufficient. But also, this PR doesn't change the way that this error would be raised, currently it doesn't rescue from that error.
There was a problem hiding this comment.
I wasn't able to work past the error in my case without using the wrapped object. Something not liked about my CIDR string, '172.16.0.0/12' is perfectly valid CIDR range though, which can be parsed from web_console.allow_whitelisted_ips for example
I was able to work past it with #444, thanks for merging this, my report is in #451
|
@jdelStrother thanks for your contribution! |
BetterErrors/better_errors#444 is not in a release of the better_errors gem yet (no release since February 2019!)
Can we add support for passing IPAddrs directly to allow_ip?