Bug #109152 for Net-DNS: Deprecated method make_query

Thu Nov 19 14:24:47 2015 rt [...] illuminated.co.uk - Ticket created

Subject:

Deprecated method make_query_packet breaks calling code

Net::DNS::Resolver::Programmable uses Net::DNS::Resolver::Base->make_query_packet - e.g line 194 my $query_packet = $self->make_query_packet(@_); As of version 1.03 of Net::DNS, that method reads: sub make_query_packet { ## historical &_make_query_packet; # uncoverable pod carp 'deprecated method; see RT#37104' unless $warned++; } The problem is that the socket object isn't actually being returned - what's being returned is the return value of carp. I haven't looked at Net::DNS in enough detail to understand why this method is being deprecated, but it seems at the very least that while it's being warned as deprecated it should nonetheless still work.

Fri Nov 20 10:07:44 2015 dcantrell [...] cpan.org - Correspondence added

Here's a patch: --- Base.pm.orig 2015-11-20 15:06:20.000000000 +0000 +++ Base.pm 2015-11-20 15:06:53.000000000 +0000 @@ -1259,8 +1259,8 @@ my $warned; sub make_query_packet { ## historical - &_make_query_packet; # uncoverable pod carp 'deprecated method; see RT#37104' unless $warned++; + &_make_query_packet; # uncoverable pod }

Fri Nov 20 10:07:45 2015 The RT System itself - Status changed from 'new' to 'open'

Fri Nov 20 21:02:22 2015 TODDR [...] cpan.org - Correspondence added

On Thu Nov 19 14:24:47 2015, SKINGTON wrote: Show quoted text

> Net::DNS::Resolver::Programmable uses Net::DNS::Resolver::Base-

> >make_query_packet - e.g line 194

I opened RT https://rt.cpan.org/Ticket/Display.html?id=109266 It would be really good if you could recommend in the ticket what they should be doing instead? Thanks!

Sun Nov 22 17:53:06 2015 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

On Fri Nov 20 21:02:22 2015, TODDR wrote: Show quoted text

> On Thu Nov 19 14:24:47 2015, SKINGTON wrote:

> > Net::DNS::Resolver::Programmable uses Net::DNS::Resolver::Base-

> > > make_query_packet - e.g line 194

> > I opened RT https://rt.cpan.org/Ticket/Display.html?id=109266 > > It would be really good if you could recommend in the ticket what they > should be doing instead? > > Thanks!

The recommendation is to refrain from diving into the code and (ab)using internal (undocumented) subroutines which may be changed or removed without notice.

Sun Nov 22 17:56:17 2015 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

On Fri Nov 20 10:07:44 2015, DCANTRELL wrote: Show quoted text

> Here's a patch: > > --- Base.pm.orig 2015-11-20 15:06:20.000000000 +0000 > +++ Base.pm 2015-11-20 15:06:53.000000000 +0000 > @@ -1259,8 +1259,8 @@ > my $warned; > > sub make_query_packet { ## historical > - &_make_query_packet; # uncoverable pod > carp 'deprecated method; see RT#37104' unless $warned++; > + &_make_query_packet; # uncoverable pod > }

Exactly! I apologise for the editing fumble

Sun Nov 22 23:53:11 2015 rt [...] illuminated.co.uk - Correspondence added

On Sun Nov 22 17:53:06 2015, rwfranks@acm.org wrote: Show quoted text

> On Fri Nov 20 21:02:22 2015, TODDR wrote:

> > On Thu Nov 19 14:24:47 2015, SKINGTON wrote:

> > > Net::DNS::Resolver::Programmable uses Net::DNS::Resolver::Base-

> > > > make_query_packet - e.g line 194

> > > > I opened RT https://rt.cpan.org/Ticket/Display.html?id=109266 > > > > It would be really good if you could recommend in the ticket what > > they > > should be doing instead? > > > > Thanks!

> > The recommendation is to refrain from diving into the code and > (ab)using internal (undocumented) subroutines which may be changed or > removed without notice.

Net::DNS::Resolver::Programmable has been using make_query_packet since at the latest 2007 (and hasn't been updated since, although that might be because the author reckoned they'd done all they needed to do.) Ray from Nominet asked for this method to be explicitly documented and supported, because it was useful to him, back in 2008. While the RT ticket https://rt.cpan.org/Ticket/Display.html?id=37104 ended at an impasse, there is no indication given that this functionality should be withdrawn - the most recent Changes for Net::DNS don't mention this deprecation, for instance. I appreciate the frustration in finding out that other people have been using what you thought was a private method. It's perfectly reasonable to say "Hey, you guys! Stop doing this!" in your code. Unfortunately (a curse in disguise?) Net::DNS is sufficiently-used that you probably can't get away with arbitrary stuff like this. I don't understand DNS very well, but at $WORK we use Net::DNS::Programmable for testing, and we'd prefer not to be locked into an old version of Net::DNS as a consequence. If we can work out a reasonable way for Net::DNS::Programmable to do its thing without meddling with things it should not wot, I'd be more than happy to apply to get co-maint on a useful but clearly neglected module.

Mon Nov 23 09:54:38 2015 NLNETLABS [...] cpan.org - Correspondence added

On Sun 22 Nov 2015 23:53:11, SKINGTON wrote: Show quoted text

> On Sun Nov 22 17:53:06 2015, rwfranks@acm.org wrote:

> > On Fri Nov 20 21:02:22 2015, TODDR wrote:

> > > On Thu Nov 19 14:24:47 2015, SKINGTON wrote:

> > > > Net::DNS::Resolver::Programmable uses Net::DNS::Resolver::Base-

> > > > > make_query_packet - e.g line 194

> > > > > > I opened RT https://rt.cpan.org/Ticket/Display.html?id=109266 > > > > > > It would be really good if you could recommend in the ticket what > > > they > > > should be doing instead?

I have done that. Show quoted text

> > > > > > Thanks!

> > > > The recommendation is to refrain from diving into the code and > > (ab)using internal (undocumented) subroutines which may be changed or > > removed without notice.

> > Net::DNS::Resolver::Programmable has been using make_query_packet > since at the latest 2007 (and hasn't been updated since, although that > might be because the author reckoned they'd done all they needed to > do.) Ray from Nominet asked for this method to be explicitly > documented and supported, because it was useful to him, back in 2008. > While the RT ticket https://rt.cpan.org/Ticket/Display.html?id=37104 > ended at an impasse, there is no indication given that this > functionality should be withdrawn - the most recent Changes for > Net::DNS don't mention this deprecation, for instance. > > I appreciate the frustration in finding out that other people have > been using what you thought was a private method. It's perfectly > reasonable to say "Hey, you guys! Stop doing this!" in your code. > Unfortunately (a curse in disguise?) Net::DNS is sufficiently-used > that you probably can't get away with arbitrary stuff like this.

Understood. That is why we warn about it first with the carp (which this time caused an unintentional bug). Note that the warning is emitted only once and not every time you use the method. Show quoted text

> I don't understand DNS very well, but at $WORK we use > Net::DNS::Programmable for testing, and we'd prefer not to be locked > into an old version of Net::DNS as a consequence. If we can work out a > reasonable way for Net::DNS::Programmable to do its thing without > meddling with things it should not wot, I'd be more than happy to > apply to get co-maint on a useful but clearly neglected module.

That would be wonderfull! We have another bugfix for the Net::DNS::Resolver::Programmable module: https://rt.cpan.org/Ticket/Display.html?id=95901 , It would be very nice if you could apply that too.

Tue Dec 08 16:57:44 2015 rwfranks [...] acm.org - Correspondence added

From:

rwfranks [...] acm.org

Fixed in 1.04

Tue Dec 08 17:15:19 2015 rt [...] illuminated.co.uk - Correspondence added

On Tue Dec 08 16:57:44 2015, rwfranks@acm.org wrote: Show quoted text

> Fixed in 1.04

Thanks, and I see that there have been other comments on related tickets that give me a better understanding about what's going on.

Wed Dec 09 04:17:22 2015 NLNETLABS [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #109152 for Net-DNS: Deprecated method make_query_packet breaks calling code