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

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

[George Matsumura via supervision]

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.
---
 src/conn-tools/s6-tcpserver-access.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conn-tools/s6-tcpserver-access.c b/src/conn-tools/s6-tcpserver-access.c
index 0bb1764..d528bd0 100644
--- a/src/conn-tools/s6-tcpserver-access.c
+++ b/src/conn-tools/s6-tcpserver-access.c
@@ -318,7 +318,7 @@ int main (int argc, char const *const *argv)
       blob[1].options = S6DNS_O_RECURSIVE ;
       data[1].rtype = S6DNS_T_PTR ;
     }
-    if (!localname && !remotelen && !s6dns_resolven_parse_g(blob + !!localname, !localname + !remotelen, &infinite))
+    if (!localname || !remotelen && !s6dns_resolven_parse_g(blob + !!localname, !localname + !remotelen, &infinite))
     {
       if (verbosity >= 3) strerr_warnwu2x("resolve IP addresses: ", s6dns_constants_error_str(errno)) ;
       if (flagfatal)
-- 
2.45.2

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

[Laurent Bercot]

>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