diff mbox

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

Message ID 20190921175913.55990-1-rodger.combs@gmail.com
State Withdrawn, archived
Headers show

Commit Message

Rodger Combs Sept. 21, 2019, 5:59 p.m. UTC
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(-)

Comments

Nicolas George Sept. 21, 2019, 6:37 p.m. UTC | #1
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,
diff mbox

Patch

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;
+            }
+        }
+    }
+    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, \