NLnetLabs / ldns

LDNS is a DNS library that facilitates DNS tool programming
https://nlnetlabs.nl/ldns
BSD 3-Clause "New" or "Revised" License
292 stars 97 forks source link

1.8.1 release broke parsing of TAB-separated HINFO #147

Closed FGasper closed 2 years ago

FGasper commented 2 years ago

With 1.8.1:

> perl -MDNS::LDNS -e'DNS::LDNS::RR->new(qq<foo\t12345\tIN\tHINFO\t"hohum" "weirdo">) or die'

> perl -MDNS::LDNS -e'DNS::LDNS::RR->new(qq<foo\t12345\tIN\tHINFO\t"hohum"\t"weirdo">) or die'
Died at -e line 1.

With 1.7.1:

> perl -MDNS::LDNS -e'DNS::LDNS::RR->new(qq<foo\t12345\tIN\tHINFO\t"hohum"\t"weirdo">) or die'

DNS::LDNS being a simple Perl wrapper around the corresponding LDNS function, I’m fairly sure the breakage is from LDNS itself.

FGasper commented 2 years ago

Notably, this passes in 1.8.1:

> perl -MDNS::LDNS -e'DNS::LDNS::RR->new(qq<foo\t12345\tIN\tHINFO\thohum\tweirdo>) or die'
FGasper commented 2 years ago

git bisect shows that #139’s fix broke it:

60773ee1acd9e4f39cf5608147fcfb1798cb3778 is the first bad commit
commit 60773ee1acd9e4f39cf5608147fcfb1798cb3778
Author: Willem Toorop <willem@nlnetlabs.nl>
Date:   Thu Sep 9 12:17:17 2021 +0200

    Don't skip character after having parsed a quoted token

    Fix issue #139

 rr.c | 8 --------
 1 file changed, 8 deletions(-)
FGasper commented 2 years ago

C demonstration of the bug:

#include "ldns/rr.h"

#define RR_STR "foo\t12345\tIN\tHINFO\t\"hohum\"\t\"weirdo\""

int main() {
    ldns_rr *rr;

    ldns_status s = ldns_rr_new_frm_str(&rr, RR_STR, 123, NULL, NULL);

    printf("LDNS parse result: %d\n", s);

    return 0;
}
FGasper commented 2 years ago

The simplest fix would seem to be to revert 60773ee.