From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI autolearn=ham autolearn_force=no version=3.4.4 Received: from alyss.skarnet.org (alyss.skarnet.org [95.142.172.232]) by inbox.vuxu.org (Postfix) with SMTP id 0E40025DE9 for ; Tue, 30 Jul 2024 10:02:41 +0200 (CEST) Received: (qmail 3261 invoked by uid 89); 30 Jul 2024 08:03:07 -0000 Mailing-List: contact supervision-help@list.skarnet.org; run by ezmlm Sender: Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Received: (qmail 3254 invoked from network); 30 Jul 2024 08:03:07 -0000 From: "Laurent Bercot" To: "George Matsumura" , supervision@list.skarnet.org Subject: Re: [PATCH] Resolve PROTOREMOTEHOST or PROTOLOCALHOST when only one is specified Date: Tue, 30 Jul 2024 08:02:41 +0000 Message-Id: In-Reply-To: <20240730071112.30487-2-gorg@gorgnet.net> References: <20240730071112.30487-2-gorg@gorgnet.net> Reply-To: "Laurent Bercot" User-Agent: eM_Client/10.0.3266.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable >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 + !!loca= lname, !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