ruby-dns / net-dns

Net::DNS is a DNS library written in Ruby.
http://net-dns.rubyforge.org
Other
165 stars 43 forks source link

Any string with a number is an IP address? #5

Open weppos opened 14 years ago

weppos commented 14 years ago

Posted on behalf of Joshua Born and restored from http://rubyforge.org/forum/message.php?msg_id=91646.

've been using net-dns in an application, and everything was going very well -- it is certainly a lot better than resolv.rb. I've been noticing a bunch of lookups fail, however. Looking into it, I notice a rather annoying foible:

The make_query_packet method assumes that any string that has any numeral in it is an IP address. On my network here, we have LOTS of host names that have numerals in them. Because of this, there are many hostnames that when queried generate ArgumentErrors.

Looking at the code, there doesn't seem to be any way to override this. Am I'm missing something?

If not, I'll probably copy the gem locally and hack it to get rid of this behavior, but I thought I'd bring this to the attention of this project.

Is there any good reason why line 1109 of resolver.rb is

  when /\d/ # Contains a number, try to see if it's an IP or IPv6 address

instead of

  when /\d\.\d\.\d\.\d/

By: Marco Ceresa

Hi Joshua,

Let me dig into it, looks like it's definitely a bug. For what I remember I ported that "feature" directly from the Perl library, but I agree with you that checking for a number in a string is a dumb way to match an IP address. I'll fix it in current and let you know.

However if you're using an hostname with numbers, it should catch the exception at line 1113. Do you have a test that we can use to reproduce the issue?

Thanks, Marco

bluemonk commented 14 years ago

Fixed a problem with rescue ArgumentError (closed by df6be6cfc30e26cd73fe38a1dfb637da0ad3b804) and with IPAddr handling (closed by df6be6cfc30e26cd73fe38a1dfb637da0ad3b804)

ShogunPanda commented 12 years ago

This bug is still present on 0.7.1. The problem is that if, for example, I lookup for "match_1.dev", the call make_query_packet(argument, type, cls) will take around 10 seconds because of this check.

I think I found a smarter solution. Should we replace that implementation with this one?

def is_ip_address?(addr)
  catch(:valid_ip) do
    if /\A(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})\Z/ =~ addr
      throw(:valid_ip, true) if $~.captures.all? {|i| i.to_i < 256}
    else
      # IPv6 (normal)
      throw(:valid_ip, true) if /\A[\dA-Fa-f]{1,4}(:[\dA-Fa-f]{1,4})*\Z/ =~ addr
      throw(:valid_ip, true) if /\A[\dA-Fa-f]{1,4}(:[\dA-Fa-f]{1,4})*::([\dA-Fa-f]{1,4}(:[\dA-Fa-f]{1,4})*)?\Z/ =~ addr
      throw(:valid_ip, true) if /\A::([\dA-Fa-f]{1,4}(:[\dA-Fa-f]{1,4})*)?\Z/ =~ addr
      # IPv6 (IPv4 compat)
      throw(:valid_ip, true) if /\A[\dA-Fa-f]{1,4}(:[\dA-Fa-f]{1,4})*:/ =~ addr && valid_v4?($')
      throw(:valid_ip, true) if /\A[\dA-Fa-f]{1,4}(:[\dA-Fa-f]{1,4})*::([\dA-Fa-f]{1,4}(:[\dA-Fa-f]{1,4})*:)?/ =~ addr && valid_v4?($')
      throw(:valid_ip, true) if /\A::([\dA-Fa-f]{1,4}(:[\dA-Fa-f]{1,4})*:)?/ =~ addr && valid_v4?($')
    end

    false
  end
end

def make_query_packet(string, type, cls)
  if string.is_a?(IPAddr) then
    name = string.reverse
    type = Net::DNS::PTR
    @logger.warn "PTR query required for address #{string}, changing type to PTR"
  elsif is_ip_address?(string) # See if it's an IP or IPv6 address
    begin
      name = IPAddr.new(string.chomp(".")).reverse
      type = Net::DNS::PTR
    rescue ArgumentError
      name = string if valid? string
    end
  else
    name = string if valid? string
  end

  # Create the packet
  packet = Net::DNS::Packet.new(name, type, cls)

  if packet.query?
    packet.header.recursive = @config[:recursive] ? 1 : 0
  end

  # DNSSEC and TSIG stuff to be inserted here

  packet
end