[FFmpeg-devel] [PATCH] lavf/tcp: add resolve_hosts option

[Nicolas George]

Rodger Combs (12019-09-21):
> This can be used to resolve particular hosts offline without affecting
> HTTP headers, TLS SNI, or cross-domain redirects. It works similarly to
> curl's --resolve option, but without port-specific handling.
> ---
>  libavformat/tcp.c     | 27 +++++++++++++++++++++++++++
>  libavformat/version.h |  2 +-
>  2 files changed, 28 insertions(+), 1 deletion(-)

I personally do not like it much. Here are my objections:

- This could apply to any networking program, it should be in the libc.

- If it is done inside FFmpeg, it should work for all name resolution,
  including UDP.

- The syntax you chose makes it awkward for IPv6 because of multiple
  colons.

- You neglected the documentation.

> 
> diff --git a/libavformat/tcp.c b/libavformat/tcp.c
> index 2198e0f00e..e4c439ee56 100644
> --- a/libavformat/tcp.c
> +++ b/libavformat/tcp.c
> @@ -45,6 +45,7 @@ typedef struct TCPContext {
>  #if !HAVE_WINSOCK2_H
>      int tcp_mss;
>  #endif /* !HAVE_WINSOCK2_H */
> +    char *resolve_hosts;
>  } TCPContext;
>  
>  #define OFFSET(x) offsetof(TCPContext, x)
> @@ -60,6 +61,7 @@ static const AVOption options[] = {
>  #if !HAVE_WINSOCK2_H
>      { "tcp_mss",     "Maximum segment size for outgoing TCP packets",          OFFSET(tcp_mss),     AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
>  #endif /* !HAVE_WINSOCK2_H */
> +    { "resolve_hosts", "Comma-separated host resolutions, in the form host:ip", OFFSET(resolve_hosts), AV_OPT_TYPE_STRING, { .str = NULL },  0, 0,       .flags = D|E },
>      { NULL }
>  };
>  
> @@ -99,6 +101,27 @@ static void customize_fd(void *ctx, int fd)
>  #endif /* !HAVE_WINSOCK2_H */
>  }
>  
> +static int lookup_host(URLContext *h, char *hostname, size_t hostname_size)
> +{
> +    TCPContext *s = h->priv_data;
> +    if (hostname[0]) {
> +        size_t hostlen = strlen(hostname);

> +        for (const char *addr = s->resolve_hosts; addr; addr = strchr(addr, ','), addr && addr++) {
> +            if (!strncmp(addr, hostname, hostlen) && addr[hostlen] == ':') {
> +                addr += hostlen + 1;
> +                const char *end = strchr(addr, ',');
> +                size_t len = end ? end - addr : strlen(addr);
> +                if (len >= hostname_size - 1)
> +                    return AVERROR(ENOMEM);
> +                memcpy(hostname, addr, len);
> +                hostname[len] = 0;
> +                return 0;
> +            }
> +        }

I find the duplicated comma search awkward. I find this code structure
easier to understand:

	while (1) {
	    end = strchr(addr, ',');
	    len = end ? end - addr : strlen(addr);
	    /* or len = strcspn(addr, ","); */
	    /* process with len */
	    addr += len;
	    if (*addr)
	        break;
	    addr++;
	}

> +    }
> +    return 0;
> +}
> +
>  /* return non zero if error */
>  static int tcp_open(URLContext *h, const char *uri, int flags)
>  {
> @@ -120,6 +143,10 @@ static int tcp_open(URLContext *h, const char *uri, int flags)
>          av_log(h, AV_LOG_ERROR, "Port missing in uri\n");
>          return AVERROR(EINVAL);
>      }
> +
> +    if ((ret = lookup_host(h, hostname, sizeof(hostname))) < 0)
> +        return ret;
> +
>      p = strchr(uri, '?');
>      if (p) {
>          if (av_find_info_tag(buf, sizeof(buf), "listen", p)) {
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 2eb14659d0..d1dbb1e2d1 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -33,7 +33,7 @@
>  // Also please add any ticket numbers that you believe might be affected here
>  #define LIBAVFORMAT_VERSION_MAJOR  58
>  #define LIBAVFORMAT_VERSION_MINOR  32
> -#define LIBAVFORMAT_VERSION_MICRO 105
> +#define LIBAVFORMAT_VERSION_MICRO 106
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>                                                 LIBAVFORMAT_VERSION_MINOR, \

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190921/0147c7f1/attachment.sig>