Hi Frank,
On Sun, 2022-04-03 at 19:49 -0400, Frank Ch. Eigler via Elfutils-devel wrote:
> commit 3a00f412b6554ba70f7b7e3d3aa6f67f278868af (HEAD -> master)
> Author: Frank Ch. Eigler
> Date: Sun Apr 3 19:42:48 2022 -0400
>
> debuginfod: use single ipv4+ipv6 microhttpd daemon configuration
>
> Use a single MHD_USE_DUAL_STACK mhd daemon. This way, the thread
> connection pool is not doubled, saving memory and better matching user
> expectations. A slight tweak to logging is required to pull IPv4
> remote addresses back out, and also to allow IPv6 ::-laden address
> forwarding through federation links.
>
> Signed-off-by: Frank Ch. Eigler
>
> diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
> index 38a389e7dfd3..578d951d0102 100644
> --- a/debuginfod/ChangeLog
> +++ b/debuginfod/ChangeLog
> @@ -1,3 +1,12 @@
> +2022-04-03 Frank Ch. Eigler
> +
> + * debuginfod.cxx (main): Use single dual-stack daemon setup,
> + rather than duplicate ipv4 and ipv6.
> + (conninfo, handle_buildid): Represent ipv4-mapped ipv6 addresses
> + in their native ipv4 form for logging and X-F-F: purposes.
> + * debuginfod-client.c (debuginfod_add_http_header): Tolerate
> + colons in http header values.
This looks ok. Some comments below, but nothing I think needs to
change.
> 2022-04-03 Frank Ch. Eigler
>
> * debuginfod.cxx (main): Use MHD_USE_EPOLL for libmicrohttpd, to
> diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
> index 024b09545d99..41ba88a56911 100644
> --- a/debuginfod/debuginfod-client.c
> +++ b/debuginfod/debuginfod-client.c
> @@ -1548,13 +1548,13 @@ int debuginfod_find_source(debuginfod_client *client,
> int debuginfod_add_http_header (debuginfod_client *client, const char*
> header)
> {
>/* Sanity check header value is of the form Header: Value.
> - It should contain exactly one colon that isn't the first or
> + It should contain at least one colon that isn't the first or
> last character. */
> - char *colon = strchr (header, ':');
> - if (colon == NULL
> - || colon == header
> - || *(colon + 1) == '\0'
> - || strchr (colon + 1, ':') != NULL)
> + char *colon = strchr (header, ':'); /* first colon */
> + if (colon == NULL /* present */
> + || colon == header /* not at beginning - i.e., have a header name */
> + || *(colon + 1) == '\0') /* not at end - i.e., have a value */
> +/* NB: but it's okay for a value to contain other colons! */
> return -EINVAL;
OK.
>struct curl_slist *temp = curl_slist_append (client->headers, header);
> diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
> index 99924d36f260..48e0f37b33f0 100644
> --- a/debuginfod/debuginfod.cxx
> +++ b/debuginfod/debuginfod.cxx
> @@ -1063,9 +1063,22 @@ conninfo (struct MHD_Connection * conn)
> sts = getnameinfo (so, sizeof (struct sockaddr_in), hostname, sizeof
> (hostname), servname,
> sizeof (servname), NI_NUMERICHOST | NI_NUMERICSERV);
>} else if (so && so->sa_family == AF_INET6) {
> -sts = getnameinfo (so, sizeof (struct sockaddr_in6), hostname, sizeof
> (hostname),
> - servname, sizeof (servname), NI_NUMERICHOST |
> NI_NUMERICSERV);
> +struct sockaddr_in6* addr6 = (struct sockaddr_in6*) so;
> +if (IN6_IS_ADDR_V4MAPPED(&addr6->sin6_addr)) {
> + struct sockaddr_in addr4;
> + memset (&addr4, 0, sizeof(addr4));
> + addr4.sin_family = AF_INET;
> + addr4.sin_port = addr6->sin6_port;
> + memcpy (&addr4.sin_addr.s_addr, addr6->sin6_addr.s6_addr+12,
> sizeof(addr4.sin_addr.s_addr));
> + sts = getnameinfo ((struct sockaddr*) &addr4, sizeof (addr4),
> + hostname, sizeof (hostname), servname, sizeof
> (servname),
> + NI_NUMERICHOST | NI_NUMERICSERV);
> +} else {
> + sts = getnameinfo (so, sizeof (struct sockaddr_in6), hostname, sizeof
> (hostname), NULL, 0,
> + NI_NUMERICHOST);
> +}
>}
That IN6_IS_ADDR_V4MAPPED is funny. Is that actually used in practice?
>if (sts != 0) {
> hostname[0] = servname[0] = '\0';
>}
> @@ -2008,13 +2021,26 @@ and will not query the upstream servers");
>
> MHD_CONNECTION_INFO_CLIENT_ADDRESS);
>struct sockaddr *so = u ? u->client_addr : 0;
>char hostname[256] = ""; // RFC1035
> - if (so && so->sa_family == AF_INET)
> + if (so && so->sa_family == AF_INET) {
> (void) getnameinfo (so, sizeof (struct sockaddr_in), hostname,
> sizeof (hostname), NULL, 0,
> NI_NUMERICHOST);
> - else if (so && so->sa_family == AF_INET6)
> -(void) getnameinfo (so, sizeof (struct sockaddr_in6), hostname,
> sizeof (hostname), NULL, 0,
> -NI_NUMERICHOST);
> -
>
