Bug #67504 for Net-DNS: Net::DNS::Packet->data doesn't parse answer/additional/authority

Fri Apr 15 20:24:33 2011 leonerd-cpan [...] leonerd.org.uk - Ticket created

Subject:

Net::DNS::Packet->data doesn't parse answer/additional/authority

Directly printing a Packet works. eval: Net::DNS::Resolver->new->query("www.cpan.org")->print; ... ;; ANSWER SECTION (3 records) www.cpan.org. 86031 IN CNAME cpan-www.develooper.com. cpan-www.develooper.com. 6831 IN CNAME cpan-global.l.develooper.org. cpan-global.l.develooper.org. 60 IN A 212.117.177.118 ;; AUTHORITY SECTION (4 records) ... However, I'm trying to serialise a response packet to send it over a pipe between two processes. This doesn't: eval: my $data = Net::DNS::Resolver->new->query("www.cpan.org")->data; "\xf6\x97\x81\x80\x00\x01\x00\x00\x00\x00\x00\x00\x03www\x04cpan\x03org\x00\x00\x01\x00\x01" eval: Net::DNS::Packet->new(\$data)->print ... ;; ANSWER SECTION (0 records) ;; AUTHORITY SECTION (0 records) Having read sub Net::DNS::Packet::data, I believe this is because it doesn't correctly parse the other fields, before trying to pack the data again. However, the following workaround seems to work OK: eval: my $q = Net::DNS::Resolver->new->query("www.cpan.org"); eval: $q->answer '3' eval: $q->authority '4' eval: $q->additional '0' eval: my $data = $q->data; "\xa1\x84\x81\x80\x00\x01\x00\x03\x00\x04\x00\x00\x03www\x04cpan\x03org\x00\x00\x01\x00\x01\xc0\x0c\x00\x05\x00\x01\x00\x01Q\x80\x00\x19\x08cpan-www\ndevelooper\x03com\x00\xc0*\x00\x05\x00\x01\x00\x00\x1c \x00\e\x0bcpan-global\x01l\ndevelooper\xc0\x15\xc0O\x00\x01\x00\x01\x00\x00\x00<\x00\x04\xd4u\xb1v\xc0[\x00\x02\x00\x01\x00\x01Q\x80\x00\x14\x03ns2\x03p20\x06dynect\x03net\x00\xc0[\x00\x02\x00\x01\x00\x01Q\x80\x00\x06\x03ns1\xc0\x8a\xc0[\x00\x02\x00\x01\x00\x01Q\x80\x00\x06\x03ns4\xc0\x8a\xc0[\x00\x02\x00\x01\x00\x01Q\x80\x00\x06\x03ns3\xc0\x8a" eval: Net::DNS::Packet->new(\$data)->print ;; HEADER SECTION ;; id = 41348 ;; qr = 1 opcode = QUERY aa = 0 tc = 0 rd = 1 ;; ra = 1 ad = 0 cd = 0 rcode = NOERROR ;; qdcount = 1 ancount = 3 nscount = 4 arcount = 0 ;; QUESTION SECTION (1 record) ;; www.cpan.org. IN A ;; ANSWER SECTION (3 records) www.cpan.org. 86400 IN CNAME cpan-www.develooper.com. cpan-www.develooper.com. 7200 IN CNAME cpan-global.l.develooper.org. cpan-global.l.develooper.org. 60 IN A 212.117.177.118 ;; AUTHORITY SECTION (4 records) eval: Net::DNS::Packet->new( \$q->data )->print ;; HEADER SECTION ;; id = 13956 ;; qr = 1 opcode = QUERY aa = 0 tc = 0 rd = 1 ;; ra = 1 ad = 0 cd = 0 rcode = NOERROR ;; qdcount = 1 ancount = 3 nscount = 4 arcount = 0 ;; QUESTION SECTION (1 record) ;; www.cpan.org. IN A ;; ANSWER SECTION (3 records) www.cpan.org. 86283 IN CNAME cpan-www.develooper.com. cpan-www.develooper.com. 7083 IN CNAME cpan-global.l.develooper.org. cpan-global.l.develooper.org. 60 IN A 212.117.177.118 ;; AUTHORITY SECTION (4 records) -- Paul Evans

Mon Apr 18 23:16:41 2011 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

Net::DNS is a stub resolver library whose purpose is to send query packets to a configured list of nameservers and receive the corresponding response packet object from which the RR data can be extracted using the object methods. It is emphatically NOT intended to be a general-purpose DNS packet manipulation library. Retransmission of packets is not a requirement for a stub resolver. Parsing of received packet buffers is deferred until the $packet->answer method is called. Other sections are handled in a similar manner. Packet sections which are not used by the application need not be parsed at all. This is expected behaviour, not a bug. On Fri Apr 15 20:24:33 2011, PEVANS wrote: Show quoted text

> Directly printing a Packet works. >

Because each component is parsed when it is extracted from the packet (using $packet->answer and similar methods). Show quoted text

> > However, I'm trying to serialise a response packet to send it over a > pipe between two processes. This doesn't: > > eval: my $data = Net::DNS::Resolver->new->query("www.cpan.org")->data; >

"\xf6\x97\x81\x80\x00\x01\x00\x00\x00\x00\x00\x00\x03www\x04cpan\x03org\x00\x00\x01\x00\x01" Show quoted text

The present implementation does not support this use case at all. Even if it did, the contents of the packet would be converted into RR objects and immediately reconverted back into binary packet format. Decoding would then need to be done a second time in the destination process. A more efficient solution might be to capture the raw packet buffer and pipe that to the destination process. The transferred packet buffer can then be decoded (once only) using existing methods. my $buffer = ... my $packet = new Net::DNS::Packet( \$buffer ); if ( $packet ) { my @answer = $packet->answer; ... } This is an application issue, not a Net::DNS bug. --Dick

Mon Apr 18 23:16:43 2011 The RT System itself - Status changed from 'new' to 'open'

Tue Apr 19 12:16:07 2011 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Mon Apr 18 23:16:41 2011, rwfranks@acm.org wrote: Show quoted text

> Parsing of received packet buffers is deferred until the $packet-

> >answer

> method is called. Other sections are handled in a similar manner. > Packet > sections which are not used by the application need not be parsed at > all. This is expected behaviour, not a bug.

Then why does ->answer fill in the answer/additional/authority sections -if- they have already been parsed, but not before? my $pkt = Net::DNS::Resolver->new->query(....) $data = $pkt->data; # contains only the question $pkt->answer; $pkt->additional; $pkt->authority; # in void context, just for side effects $data = $pkt->data; # now contains all the sections Theabove workaround is what my code currently contains. If the intention is not to do this, then why does sub Net::DNS::Packet::data contain: $header->ancount( scalar @{$self->{answer}} ); $header->nscount( scalar @{$self->{authority}} ); $header->arcount( scalar @{$self->{additional}} ); followed by: foreach my $component ( $header, @{$self->{question}}, @{$self->{answer}}, @{$self->{authority}}, @{$self->{additional}} ) { If it's only ever for the question, then don't bother filling in the answer/authority/additional fields "if they've been parsed but not if not". Either it always should, or it never should. The fact that the behaviour of one pure accessing method (->data) changes depending on whether or not some other (supposedly) pure accessing methods (->answer, ->authority, ->additional) are called is surely a bug, regardless. Show quoted text

> > However, I'm trying to serialise a response packet to send it over a > > pipe between two processes. This doesn't:

Show quoted text

> The present implementation does not support this use case at all.

Again, I observe that if I put those three void-context calls it happens to work fine. There's surely a really trivial "parse the response sections in ->data if they're present" code fix here... Show quoted text

> Even if it did, the contents of the packet would be converted into RR > objects and immediately reconverted back into binary packet format. > Decoding would then need to be done a second time in the destination > process.

I'm aware of that; see side note below. Show quoted text

> A more efficient solution might be to capture the raw packet buffer > and > pipe that to the destination process. The transferred packet buffer > can > then be decoded (once only) using existing methods. > > my $buffer = ... > my $packet = new Net::DNS::Packet( \$buffer ); > if ( $packet ) { > my @answer = $packet->answer; > ... > }

That's an interesting idea - how might I capture that buffer? I couldn't see a way using Net::DNS::Resolver. ----- As a side note: The reason I want to send the raw DNS packet bytes over the pipe between these two processes, is that most of the time Net::LibResolv will be installed, so usually the child process actually calls res_query(3), which just returns that raw DNS packet in a bytestring efficiently from the C code. That goes over the pipe to be parsed using Net::DNS in the controlling parent process. Net::DNS::Resolver is only used in the child as a fallback if Net::LibResolv is missing; perhaps because of an inability to compile and install XS code. -- Paul Evans

Wed Apr 20 02:55:20 2011 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

Paul, On Tue Apr 19 12:16:07 2011, PEVANS wrote: Show quoted text

> On Mon Apr 18 23:16:41 2011, rwfranks@acm.org wrote:

> > Parsing of received packet buffers is deferred until the $packet-

> > >answer

> > method is called. Other sections are handled in a similar manner. > > Packet > > sections which are not used by the application need not be parsed at > > all. This is expected behaviour, not a bug.

> > Then why does ->answer fill in the answer/additional/authority > sections -if- they have already been parsed, > but not before?

Data lives (unparsed) in the raw packet buffer until answer/authority/additional is called. The corresponding RR section in the Net::DNS::Packet instance is populated and answer etc. returns a list of RR references. Show quoted text

> The fact that the behaviour of one pure accessing method (->data) > changes depending on whether or not some > other (supposedly) pure accessing methods (->answer, ->authority, > ->additional) are called is surely a bug, > regardless.

$packet->data does not force parsing, because as I explained before, this is not a use case that Net::DNS needs to support. As it is also horribly inefficient, there is little motivation for mission-creep in that direction. $packet->data is provided for generating images of fully populated packets objects to be sent to a nameserver. Retransmission of inbound packets is out of scope for a stub resolver, no requirement, so not implemented. This view should be reinforced by raising a fatal program error exception. What you are doing may be interesting but lies beyond the remit of Net::DNS. Show quoted text

> > A more efficient solution might be to capture the raw packet buffer > > and > > pipe that to the destination process. The transferred packet buffer > > can > > then be decoded (once only) using existing methods. > > > > my $buffer = ... > > my $packet = new Net::DNS::Packet( \$buffer ); > > if ( $packet ) { > > my @answer = $packet->answer; > > ... > > }

> > That's an interesting idea - how might I capture that buffer? I > couldn't see a way using Net::DNS::Resolver.

There is currently no legitimate way to do this. However, you could abuse Net::DNS some more, and fish out the reference to the raw data. This breaks encapsulation and, as it is not part of the published interface, it could change or disappear without notice. This may not be a good idea for widely deployed code. my $packet = $resolver->query( ... ); my $buffer = $packet->{buffer}; my $rawdata = $$buffer; Providing a good long-term solution which does not also fill up memory with a trail of unwanted raw packet buffers will require some careful thought. Regards --Dick

Thu Apr 21 07:43:54 2011 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Wed Apr 20 02:55:20 2011, rwfranks@acm.org wrote: Show quoted text

> Paul, > > On Tue Apr 19 12:16:07 2011, PEVANS wrote: >

> > On Mon Apr 18 23:16:41 2011, rwfranks@acm.org wrote:

> > > Parsing of received packet buffers is deferred until the $packet-

> > > >answer

> > > method is called. Other sections are handled in a similar manner. > > > Packet > > > sections which are not used by the application need not be parsed at > > > all. This is expected behaviour, not a bug.

> > > > Then why does ->answer fill in the answer/additional/authority > > sections -if- they have already been parsed, > > but not before?

> > Data lives (unparsed) in the raw packet buffer until > answer/authority/additional is called. The corresponding RR section in > the Net::DNS::Packet instance is populated and answer etc. returns a > list of RR references.

Oops, this was misworded. I meant to ask why does ->data fill these sections? Show quoted text

> > The fact that the behaviour of one pure accessing method (->data) > > changes depending on whether or not some > > other (supposedly) pure accessing methods (->answer, ->authority, > > ->additional) are called is surely a bug, > > regardless.

> > $packet->data does not force parsing, because as I explained before, > this is not a use case that Net::DNS needs to support. As it is also > horribly inefficient, there is little motivation for mission-creep in > that direction.

The question here is: Should the byte buffer returned by ->data include the answer/authority/additional fields? If yes => currently it doesn't unless they've been parsed first, so sub data {} should do that. If no => currently it will do if they have been parsed first, so the parts of code responsible for this behavior in sub data {} should be removed. The current "sometimes-yes-sometimes-no" inconsistency seems to me a bug, I can't see it any other way. Show quoted text

> > That's an interesting idea - how might I capture that buffer? I > > couldn't see a way using Net::DNS::Resolver.

> > There is currently no legitimate way to do this. > > However, you could abuse Net::DNS some more, and fish out the reference > to the raw data. > > This breaks encapsulation and, as it is not part of the published > interface, it could change or disappear without notice. This may not be > a good idea for widely deployed code. > > my $packet = $resolver->query( ... ); > my $buffer = $packet->{buffer}; > my $rawdata = $$buffer; > > Providing a good long-term solution which does not also fill up memory > with a trail of unwanted raw packet buffers will require some careful > thought.

It appears to me as the code stands, I basically have four options: 0. Don't change anything, and continue to rely on the side-effect that ->answer etc.. will prefill the data buffers such that ->data now returns all the bytes. 1. Break the encapsulation of Net::DNS::Packet by using the code sample above. 2. Add a new method to Net::DNS::Packet, which would need to be integrated upstream: sub bufferbytes { return ${ shift->{buffer} } } 3. Abandon the use of Net::DNS::Resolver in the child process and use only Net::LibResolv. For reference, the code under consideration here is currently: http://cpansearch.perl.org/src/PEVANS/IO-Async-Resolver-DNS-0.01/lib/IO/Async/Resolver/DNS.pm What would you suggest here? It seems that, ideally, option 2 would look best. Adds a small simple interface to Net::DNS::Packet that directly and efficiently yields the answer I'm looking for. I don't like option 1. Option 0 means relying on the current side-effect to remain working (though unit tests easily determine if it is doing so). Failing any of these options, my only remaining choice would otherwise be to abandon using Net::DNS::Resolver at all. But I am hoping that is the last-resort choice. -- Paul Evans

Thu Apr 21 11:45:14 2011 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

On Thu Apr 21 07:43:54 2011, PEVANS wrote: Show quoted text

> What would you suggest here? It seems that, ideally, option 2 would > look best. Adds a small simple > interface to Net::DNS::Packet that directly and efficiently yields the > answer I'm looking for. I don't > like option 1. Option 0 means relying on the current side-effect to > remain working (though unit tests > easily determine if it is doing so). > > Failing any of these options, my only remaining choice would otherwise > be to abandon using > Net::DNS::Resolver at all. But I am hoping that is the last-resort > choice.

Option 0 just forces parsing to happen and will always remain working because it uses the existing interface. Option 1 was offered as an interim workaround which avoids the parsing. Option 2 is a non-starter. Your original complaint was that the object behaviour was wrong, and from your standpoint it is. This is well demonstrated by your workaround. Fixing this without changing the published interface is probably the best way forward. I will revisit the code to see if there is an efficient way of doing this. --Dick

Thu Dec 08 11:55:42 2011 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

The following patch should solve the problem nicely, on the understanding that you have not forced parsing either directly, or indirectly by calling string, printing or turning on debug flag. *** /home/rwf/src/Net-DNS-0.66/lib/Net/DNS/Packet.pm 2009-12-30 11:01:39.000000000 +0000 --- ./Packet.pm 2011-12-08 16:34:37.172417186 +0000 *************** *** 138,143 **** --- 138,146 ---- sub data { my $self = shift; + + return ${$self->{buffer}} if $self->{buffer}; # retransmit raw packet + my $data = ''; my $header = $self->{header};

Mon Feb 27 13:22:58 2012 NLNETLABS [...] cpan.org - Correspondence added

Closing because fix is in the current release.

Mon Feb 27 13:22:59 2012 NLNETLABS [...] cpan.org - Status changed from 'open' to 'resolved'