Re: [PATCH] httpfs: add IPv6 support and modernize network logic

2026-01-26 Thread Samuel Thibault
Hello,

Gianluca Cannata, le sam. 24 janv. 2026 15:26:53 +0100, a ecrit:
> -error_t lookup_host (const char *host, struct sockaddr_storage *dest, 
> socklen_t *dest_len) 
> +error_t lookup_host (const char *host, struct sockaddr_storage *dest, 
> socklen_t *dest_len, int *socktype, int *protocol) 
>  {
> struct addrinfo hints;
> struct addrinfo *res;
> int s;
> -
> -   /* Buffer that holds a clean host, that is a host without square 
> brackets in case of a IPv6 address */
> -   char clean_host[256];
> -   const char *host_ptr = host;
> +   char *allocated_host = NULL;
>  
> if (!host || !dest || !dest_len)
> return EINVAL;
> @@ -52,18 +49,25 @@ error_t lookup_host (const char *host, s
>  * because getaddrinfo does not accept square brackets, but URLs do.
>  */
> if (host[0] == '[') {
> -   const char *end = strchr (host, ']');
> +   size_t host_len = strlen (host);
> +
> +   /* If it starts with a [, it must also ends with a ] */
> +   if (host[host_len - 1] != ']')
> +   return EINVAL;
> +
> +   /* We allocate only the necessary memory with a VLA (C99) */
> +   size_t clean_len = host_len - 2;
> +   char clean_host[clean_len + 1];

This is unused, since you just strdup below, which is fine:

> +
> +   /* Copy the content between square brackets */
> +   allocated_host = strndup (host + 1, host_len - 2);
> +   if (!allocated_host)
> +   return ENOMEM;
> +
> +   host = allocated_host;
> }

Please send a patch with the complete changes, so I can apply it in one
go.

Samuel



Re: [PATCH] httpfs: add IPv6 support and modernize network logic

2026-01-24 Thread Gianluca Cannata
Hi Samuel,

Thank you for your feedback.

I should have addressed all the required changes.

Let me know what you think.

Cheers,

Gianluca

Il giorno gio 22 gen 2026 alle ore 00:53 Samuel Thibault
 ha scritto:
>
> Hello,
>
> Thanks for this, it's indeed useful and well-contained for a
> contribution.
>
> Samuel
>
> Gianluca Cannata, le mer. 21 janv. 2026 09:31:37 +0100, a ecrit:
> > Index: httpfs/http.c
> > ===
> > --- httpfs.orig/http.c
> > +++ httpfs/http.c
> > @@ -34,14 +34,59 @@
> >  #include 
> >  #include 
> >
> > -/* do a DNS lookup for NAME and store result in *ENT */
> > -error_t lookup_host (char *url, struct hostent **ent)
> > +/* do a DNS lookup for HOST and store result in *DEST */
> > +error_t lookup_host (const char *host, struct sockaddr_storage *dest, 
> > socklen_t *dest_len)
> >  {
> > -   if ( (*ent = gethostbyname(url)) == NULL )
> > -   {
> > -   fprintf(stderr,"wrong host name\n");
> > +   struct addrinfo hints;
> > +   struct addrinfo *res;
> > +   int s;
> > +
> > +   /* Buffer that holds a clean host, that is a host without square 
> > brackets in case of a IPv6 address */
> > +   char clean_host[256];
>
> Rather than hard-coding 256, you can use strlen(host)-1. Then you won't
> have an ENAMETOOLONG error.
>
> > +   const char *host_ptr = host;
>
> You don't need a separate host_ptr variable. When using clean_host, you
> can as well just do host = clean_host.
>
> > +   if (!host || !dest || !dest_len)
> > return EINVAL;
> > +
> > +   /* IPv6 syntax transformation: e.g. [2001:db8::1] -> 2001:db8::1
> > +* because getaddrinfo does not accept square brackets, but URLs do.
> > +*/
> > +   if (host[0] == '[') {
> > +   const char *end = strchr (host, ']');
>
> If you have an opening bracket, the closing bracket should be the last
> character anyway, so you can as well directly check for ']' at the end,
> and return EINVAL otherwise.
>
> > +   if (end) {
> > +   size_t host_len = end - host - 1;
> > +   if (host_len >= sizeof (clean_host)) return 
> > ENAMETOOLONG;
> > +
> > +   strncpy (clean_host, host + 1, host_len);
>
> Better use memcpy.
>
> > +   clean_host[host_len] = '\0';
> > +   host_ptr = clean_host;
> > +   }
> > }
> > +
> > +   memset (&hints, 0, sizeof (struct addrinfo));
>
> better use sizeof(hints), which we then know for sure is correct.
>
> > +   hints.ai_family = AF_UNSPEC; /* IPv4 and IPv6 */
> > +   hints.ai_socktype = SOCK_STREAM; /* TCP */
> > +   /* AI_ADDRCONFIG: Asks for  record only if the host has IPv6 
> > connectivity.
> > +* AI_V4MAPPED: Maps IPv4 to IPv6 if necessary. */
> > +   hints.ai_flags = AI_ADDRCONFIG | AI_V4MAPPED;
> > +
> > +   /* DNS lookup */
> > +   s = getaddrinfo (host_ptr, NULL, &hints, &res);
> > +   if (s != 0) {
> > +   if (debug_flag)
> > +   fprintf (stderr, "DNS lookup failed for %s: %s\n", 
> > host_ptr, gai_strerror (s));
> > +
> > +   return ENOENT;
> > +   }
> > +
> > +   /* TODO: Shall we copy only the first result or iterate through 
> > possible multiple results */
>
> Ideally we'd try to connect to each result until one succeeds. But that
> will do for now.
>
> > +   if (res != NULL) {
> > +   memcpy (dest, res->ai_addr, res->ai_addrlen);
> > +   *dest_len = res->ai_addrlen;
> > +   dest->ss_family = res->ai_family;
>
> res->ai_addr.sa_family will already have filled dest->ss_family, no need
> to fill it again.
>
> > +   }
> > +
> > +   freeaddrinfo (res);
> > return 0;
> >  }
> >
> > @@ -52,8 +97,8 @@ error_t open_connection(struct netnode *
> >  * head. *HEAD_LEN indicates the header length, for pruning upto 
> > that */
> >
> > error_t err;
> > -   struct hostent *hptr;
> > -   struct sockaddr_in dest;
> > +   struct sockaddr_storage server_addr;
> > +   socklen_t addr_len;
> > ssize_t written;
> > size_t towrite;
> > char buffer[4096];
> > @@ -63,49 +108,39 @@ error_t open_connection(struct netnode *
> > char delimiters0[] = " ";
> > char delimiters1[] = "\n";
> >
> > -   bzero(&dest,sizeof(dest));
> > -   dest.sin_family = AF_INET;
> > -   dest.sin_port = htons (port);
> > -
> > -   if ( !strcmp(ip_addr,"0.0.0.0") )
> > -   {
> > -   /* connection is not through a proxy server
> > -* find IP addr. of remote server */
> > -   err = lookup_host (node->url, &hptr);
> > -   if (err)
> > -   {
> > -   fprintf(stderr,"Could not find IP addr 
> > %s\n",node->url);
> > -   return err;
> > -  

Re: [PATCH] httpfs: add IPv6 support and modernize network logic

2026-01-21 Thread Samuel Thibault
Hello,

Thanks for this, it's indeed useful and well-contained for a
contribution.

Samuel

Gianluca Cannata, le mer. 21 janv. 2026 09:31:37 +0100, a ecrit:
> Index: httpfs/http.c
> ===
> --- httpfs.orig/http.c
> +++ httpfs/http.c
> @@ -34,14 +34,59 @@
>  #include 
>  #include 
>  
> -/* do a DNS lookup for NAME and store result in *ENT */
> -error_t lookup_host (char *url, struct hostent **ent) 
> +/* do a DNS lookup for HOST and store result in *DEST */
> +error_t lookup_host (const char *host, struct sockaddr_storage *dest, 
> socklen_t *dest_len) 
>  {
> -   if ( (*ent = gethostbyname(url)) == NULL )
> -   {
> -   fprintf(stderr,"wrong host name\n");
> +   struct addrinfo hints;
> +   struct addrinfo *res;
> +   int s;
> +
> +   /* Buffer that holds a clean host, that is a host without square 
> brackets in case of a IPv6 address */
> +   char clean_host[256];

Rather than hard-coding 256, you can use strlen(host)-1. Then you won't
have an ENAMETOOLONG error.

> +   const char *host_ptr = host;

You don't need a separate host_ptr variable. When using clean_host, you
can as well just do host = clean_host.

> +   if (!host || !dest || !dest_len)
> return EINVAL;
> +
> +   /* IPv6 syntax transformation: e.g. [2001:db8::1] -> 2001:db8::1
> +* because getaddrinfo does not accept square brackets, but URLs do.
> +*/
> +   if (host[0] == '[') {
> +   const char *end = strchr (host, ']');

If you have an opening bracket, the closing bracket should be the last
character anyway, so you can as well directly check for ']' at the end,
and return EINVAL otherwise.

> +   if (end) {
> +   size_t host_len = end - host - 1;
> +   if (host_len >= sizeof (clean_host)) return 
> ENAMETOOLONG;
> +
> +   strncpy (clean_host, host + 1, host_len);

Better use memcpy.

> +   clean_host[host_len] = '\0';
> +   host_ptr = clean_host;
> +   }
> }
> +
> +   memset (&hints, 0, sizeof (struct addrinfo));

better use sizeof(hints), which we then know for sure is correct.

> +   hints.ai_family = AF_UNSPEC; /* IPv4 and IPv6 */
> +   hints.ai_socktype = SOCK_STREAM; /* TCP */
> +   /* AI_ADDRCONFIG: Asks for  record only if the host has IPv6 
> connectivity.
> +* AI_V4MAPPED: Maps IPv4 to IPv6 if necessary. */
> +   hints.ai_flags = AI_ADDRCONFIG | AI_V4MAPPED;
> +
> +   /* DNS lookup */
> +   s = getaddrinfo (host_ptr, NULL, &hints, &res);
> +   if (s != 0) {
> +   if (debug_flag)
> +   fprintf (stderr, "DNS lookup failed for %s: %s\n", 
> host_ptr, gai_strerror (s));
> +
> +   return ENOENT;
> +   }
> +
> +   /* TODO: Shall we copy only the first result or iterate through 
> possible multiple results */

Ideally we'd try to connect to each result until one succeeds. But that
will do for now.

> +   if (res != NULL) {
> +   memcpy (dest, res->ai_addr, res->ai_addrlen);
> +   *dest_len = res->ai_addrlen;
> +   dest->ss_family = res->ai_family;

res->ai_addr.sa_family will already have filled dest->ss_family, no need
to fill it again.

> +   }
> +
> +   freeaddrinfo (res);
> return 0;
>  }
>  
> @@ -52,8 +97,8 @@ error_t open_connection(struct netnode *
>  * head. *HEAD_LEN indicates the header length, for pruning upto that 
> */
> 
> error_t err;
> -   struct hostent *hptr;
> -   struct sockaddr_in dest;
> +   struct sockaddr_storage server_addr;
> +   socklen_t addr_len;
> ssize_t written;
> size_t towrite;
> char buffer[4096];
> @@ -63,49 +108,39 @@ error_t open_connection(struct netnode *
> char delimiters0[] = " ";
> char delimiters1[] = "\n";
>  
> -   bzero(&dest,sizeof(dest));
> -   dest.sin_family = AF_INET;
> -   dest.sin_port = htons (port);
> -
> -   if ( !strcmp(ip_addr,"0.0.0.0") )
> -   {
> -   /* connection is not through a proxy server
> -* find IP addr. of remote server */
> -   err = lookup_host (node->url, &hptr);
> -   if (err)
> -   {
> -   fprintf(stderr,"Could not find IP addr 
> %s\n",node->url);
> -   return err;
> -   }
> -   dest.sin_addr = *(struct in_addr *)hptr->h_addr;
> -   }
> -   else
> -   {
> -   /* connection is through the proxy server
> -* need not find IP of remote server
> -* find IP of the proxy server */
> -   if ( inet_aton(ip_addr,&dest.sin_addr) == 0 )
> -   {
> -   fprintf(stderr,"Invalid IP for proxy\n");
> -