Regression with c-ares 1.21.0/1.22.0 in nodejs tests

[gjasny]

Hello,

part of the Debian package migration checks is an autopkgtest run where the newly built packages are tested within their consumers. In this case it's the node(js) package which now runs its internal test suite with c-ares 1.21.0 (and 1.22.0).

Unfortunately, there is a regression: Please see the "excuses" in the c-ares package tracking page. The i386 run for example fails with (any some more):

566s not ok 598 parallel/test-dns-resolveany
566s   ---
566s   duration_ms: 0.328
566s   severity: fail
566s   exitcode: 1
566s   stack: |-
566s     node:internal/process/promises:288
566s                 triggerUncaughtException(err, true /* fromPromise */);
566s                 ^
566s     
566s     AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
566s     + actual - expected ... Lines skipped
566s     
566s       [
566s         {
566s     ...
566s         {
566s           entries: [
566s     +       'v=spf1 ~allxyz\x00foo'
566s     -       'v=spf1 ~all',
566s     -       'xyz\x00foo'
566s           ],
566s     ...
566s           type: 'CAA'
566s         }
566s       ]
566s         at validateResults (/tmp/autopkgtest-lxc.j28ifl_u/downtmp/build.6KN/src/test/parallel/test-dns-resolveany.js:61:10)
566s         at Socket.<anonymous> (/tmp/autopkgtest-lxc.j28ifl_u/downtmp/build.6KN/src/test/parallel/test-dns-resolveany.js:51:3) {
566s       generatedMessage: true,
566s       code: 'ERR_ASSERTION',
566s       actual: [
566s         { address: '1.2.3.4', ttl: 123, type: 'A' },
566s         { address: '::42', ttl: 123, type: 'AAAA' },
566s         { exchange: 'foobar.com', priority: 42, type: 'MX' },
566s         { value: 'foobar.org', type: 'NS' },
566s         { entries: [ 'v=spf1 ~allxyz\x00foo' ], type: 'TXT' },
566s         { value: 'baz.org', type: 'PTR' },
566s         {
566s           nsname: 'ns1.example.com',
566s           hostmaster: 'admin.example.com',
566s           serial: 156696742,
566s           refresh: 900,
566s           retry: 900,
566s           expire: 1800,
566s           minttl: 60,
566s           type: 'SOA'
566s         },
566s         { critical: 128, issue: 'platynum.ch', type: 'CAA' }
566s       ],
566s       expected: [
566s         { type: 'A', address: '1.2.3.4', ttl: 123 },
566s         { type: 'AAAA', address: '::42', ttl: 123 },
566s         { type: 'MX', priority: 42, exchange: 'foobar.com' },
566s         { type: 'NS', value: 'foobar.org' },
566s         { type: 'TXT', entries: [ 'v=spf1 ~all', 'xyz\x00foo' ] },
566s         { type: 'PTR', value: 'baz.org' },
566s         {
566s           type: 'SOA',
566s           nsname: 'ns1.example.com',
566s           hostmaster: 'admin.example.com',
566s           serial: 156696742,
566s           refresh: 900,
566s           retry: 900,
566s           expire: 1800,
566s           minttl: 60
566s         },
566s         { type: 'CAA', critical: 128, issue: 'platynum.ch' }
566s       ],
566s       operator: 'deepStrictEqual'
566s     }
566s     
566s     Node.js v18.13.0
566s   ...

The test itself is located here.

Could you please take a look at this test with its failure and check if the test assumption is wrong or the new parser code has issues?

Thanks,
Gregor

CC: @addaleax

[bradh352]

This is an intentional change in c-ares. DNS TXT records, due to the wire format, must be split every 255 characters as the length indicator for a string is a single octet. This complicates things like DKIM record that my have say 2048bit keys as the old c-ares would return this as multiple TXT records even though it is a single record entry. So in c-ares we concatenate the strings in a single TXT entry. Separate actual TXT records are output independently.

As far as I am aware, there is no use-case for interpreting "strings" in a single text record independently.

[bradh352]

Some info, which says "If a published record contains multiple character-strings, then the record MUST be treated as if those strings are concatenated together without adding spaces":
https://labs.ripe.net/author/pgl/the-joy-of-txt/#:~:text=If%20a%20published%20record%20contains,v%3Dspf1%20....

Which is what c-ares is doing today, but was NOT previously.

[bradh352]

I also see test-dns-resolveany-bad-ancount is returning ARES_ETIMEOUT when it is expecting ARES_EBADRESP. I'd have to see what this test case is actually doing, but likely it is considered a critical parse failure and throws away the response if its malformed and retries the query in the new version. I'd think this test should check for not ARES_SUCCESS instead.

I don't think we can really revert this behavior since we're going to be implementing #620 which does require full message parsing up front to validate DNS cookies, so we can't just strip off the header to do the matching as we might want to throw away responses that look to be spoofed to wait on the legitimate response.

[bradh352]

hopefully with the 1.23.0 release it will fix any remaining issues

[bradh352]

closing this since 1.23.0 release is out and should resolve the remaining issues