Re: [PATCH] Resolve PROTOREMOTEHOST or PROTOLOCALHOST when only one is specified

From: Laurent Bercot <ska-supervision_at_skarnet.org>
Date: Tue, 30 Jul 2024 08:02:41 +0000

>Previously, specification of PROTOLOCALHOST on the command line with the -l
>option would lead to PROTOREMOTEHOST not being resolved. This did not seem to
>be the intended behaviour, especially as the call to s6dns_resolven_parse_g
>works with any combination of the state of previous hostname definitions. This
>change alters the conditional to conduct the necessary resolutions in all
>cases. I apologize if I misinterpreted or was mistaken about anything.

  Your diagnosis is correct! Indeed, the call to s6dns_resolven_parse_g
must happen when at least one of (localname, remotelen) is 0, which
means they haven't yet been resolved another way.


>+ if (!localname || !remotelen && !s6dns_resolven_parse_g(blob + !!localname, !localname + !remotelen, &infinite))

  This, however, is incorrect: && has precedence over ||, so the call
would not happen when localname is 0. You want parentheses around
that ||. :)

  I've pushed a fix. Thanks for the report!

  s6-tcpserver-access.c is more spaghetti and difficult to read than
normally would be warranted for a program that should be simple on
paper, and I'm not surprised it's not exempt of bugs. Unfortunately,
it's in the hot path of new TCP connections from clients, and it
performs network resolutions such as DNS, so it needs to be optimized
for low latency prior to any other consideration. Thus, it's a maze
of tests and DNS queries are only performed when there's no other
option, and always in parallel when there are several of them. That's
why it looks like a mess with variables and conditions everywhere
which is a good breeding spot for the kind of bug you found.

--
  Laurent
Received on Tue Jul 30 2024 - 10:02:41 CEST

This archive was generated by hypermail 2.4.0 : Tue Jul 30 2024 - 10:03:09 CEST