tailhook/dns-parser

Couple of comments on parsing

Closed this issue · 3 comments

Hi,

I was just doing some browsing and came across this crate; and I spotted some odds and ends that may not be quite right, or could be improved.

  • Parser treats time-to-live as u32. But RFC 1035 says that the TTL is signed, so this should be i32. RFC2181 section 8 insists that this is so:

It is hereby specified that a TTL value is an unsigned number, with a minimum value of 0, and a
maximum value of 2147483647... Implementations should treat TTL values received with the
most significant bit set as if the entire value received was zero.

  • RFC1035 says that response codes 6-15 are reserved, but the parser treats them as invalid. This isn't consistent with the treatment of large op codes, which the parser does treat as reserved (and not invalid)
  • Packet::parse() can, so should, use Vec::with_capacity() for both questions and answers.

I hope this is helpful rather than otherwise; if not, please feel free just to close.

Thanks for a review.

RFC1035 says that response codes 6-15 are reserved, but the parser treats them as invalid. This isn't consistent with the treatment of large op codes, which the parser does treat as reserved (and not invalid)

I see the following in the specification:

Z Reserved for future use. Must be zero in all queries
and responses.

I think the spec defines that if bits are non-zero the packet should be rejected rather than the bits ignored. Do you know any spec that uses them? (so we can find out what is better for compatibility) I think they were never used yet, so if we received non-zero bits, it's probably some UDP garbage rather than correct DNS packet.

Other improvements are fair enough. Hopefully, will fix them shortly. I'll keep the issue open until they are fixed.

Sure, the 3 "Z" bits should be zero. But I'm talking about the 4 RCODE bits.

Okay fixed.

I think that ttl should stay u32, except I've made it reset to zero if larger than i32::MAX. I think using unsigned variable for unsigned value is better, even if it's range of values doesn't match exactly.