diff mbox series

[FFmpeg-devel] libavformat/tcp: add local_addr/local_port for network option

Message ID 20230228101640.3337990-1-jack.wgm@gmail.com
State New
Headers show
Series [FFmpeg-devel] libavformat/tcp: add local_addr/local_port for network option | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

jackarain Feb. 28, 2023, 10:16 a.m. UTC 
Signed-off-by: jack <jack.wgm@gmail.com>
---
 libavformat/tcp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

Comments

Nicolas George Feb. 28, 2023, 10:57 a.m. UTC | #1 
jack (12023-02-28):
> Signed-off-by: jack <jack.wgm@gmail.com>
> ---
>  libavformat/tcp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)

Thanks for the patch

You neglected to update the documentation, and a few remarks below.

> 
> diff --git a/libavformat/tcp.c b/libavformat/tcp.c
> index a11ccbb913..598d61067e 100644
> --- a/libavformat/tcp.c
> +++ b/libavformat/tcp.c
> @@ -36,6 +36,8 @@ typedef struct TCPContext {
>      const AVClass *class;
>      int fd;
>      int listen;
> +    int local_port;
> +    char *local_addr;
>      int open_timeout;
>      int rw_timeout;
>      int listen_timeout;
> @@ -52,6 +54,8 @@ typedef struct TCPContext {
>  #define E AV_OPT_FLAG_ENCODING_PARAM
>  static const AVOption options[] = {
>      { "listen",          "Listen for incoming connections",  OFFSET(listen),         AV_OPT_TYPE_INT, { .i64 = 0 },     0,       2,       .flags = D|E },
> +    { "local_port",      "Local port",                                         OFFSET(local_port),     AV_OPT_TYPE_INT,    { .i64 = -1 },      -1, INT_MAX, .flags = D|E },
> +    { "local_addr",      "Local address",                                      OFFSET(local_addr),     AV_OPT_TYPE_STRING, { .str = NULL },     0,       0, .flags = D|E },
>      { "timeout",     "set timeout (in microseconds) of socket I/O operations", OFFSET(rw_timeout),     AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
>      { "listen_timeout",  "Connection awaiting timeout (in milliseconds)",      OFFSET(listen_timeout), AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
>      { "send_buffer_size", "Socket send buffer size (in bytes)",                OFFSET(send_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
> @@ -73,6 +77,39 @@ static const AVClass tcp_class = {
>  static void customize_fd(void *ctx, int fd)
>  {
>      TCPContext *s = ctx;
> +

> +    if (s->local_addr) {

s->local_addr || s->local_port

> +        struct addrinfo hints = { 0 }, *ai;
> +        int ret;
> +        int port = 0;
> +
> +        hints.ai_family = AF_UNSPEC;
> +        hints.ai_socktype = SOCK_STREAM;
> +
> +        if (s->local_port > 0)
> +            port = s->local_port;
> +

> +        ret = getaddrinfo(s->local_addr, "0", &hints, &ai);

The provided port should be used instead of this "0", and be a string,
and named service.

(The remote port should be a string too, alas.)

> +        if (ret) {

> +            av_log(ctx, AV_LOG_WARNING,
> +               "Failed to bind local addr: %s port: %d err: %s\n",
> +                s->local_addr, s->local_port, gai_strerror(ret));

Should be an error: failing to bind on the specified address can be a
security issue. And the error message is misleading.

> +        } else {

> +            if (ai->ai_family == AF_INET6) {
> +                struct sockaddr_in6 * sockaddr_v6 = (struct sockaddr_in6 *)ai->ai_addr;
> +                sockaddr_v6->sin6_port = htons(port);
> +            } else {
> +                struct sockaddr_in * sockaddr = (struct sockaddr_in *)ai->ai_addr;
> +                sockaddr->sin_port = htons(port);
> +            }

Do not distinguish between protocols: ai_addr already contains all that
is needed.

> +            ret = bind(fd, (struct sockaddr *)ai->ai_addr, (int)ai->ai_addrlen);
> +            if (ret) {
> +                av_log(ctx, AV_LOG_WARNING,
> +                "Failed to bind local addr: %s port: %d err: %s\n",
> +                    s->local_addr, s->local_port, gai_strerror(ret));
> +            }

You should be looping over the returned addresses.

> +        }
> +    }
>      /* Set the socket's send or receive buffer sizes, if specified.
>         If unspecified or setting fails, system default is used. */
>      if (s->recv_buffer_size > 0) {
> @@ -129,6 +166,13 @@ static int tcp_open(URLContext *h, const char *uri, int flags)
>              if (buf == endptr)
>                  s->listen = 1;
>          }
> +        if (av_find_info_tag(buf, sizeof(buf), "local_port", p)) {
> +            s->local_port = strtol(buf, NULL, 10);
> +        }
> +        if (av_find_info_tag(buf, sizeof(buf), "local_addr", p)) {
> +            av_freep(&s->local_addr);
> +            s->local_addr = av_strndup(buf, strlen(buf));
> +        }
>          if (av_find_info_tag(buf, sizeof(buf), "timeout", p)) {
>              s->rw_timeout = strtol(buf, NULL, 10);
>          }

Regards,
jackarain Feb. 28, 2023, 2:27 p.m. UTC | #2 
Done.

Nicolas George <george@nsup.org> 于2023年2月28日周二 18:57写道：

> jack (12023-02-28):
> > Signed-off-by: jack <jack.wgm@gmail.com>
> > ---
> >  libavformat/tcp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
>
> Thanks for the patch
>
> You neglected to update the documentation, and a few remarks below.
>
> >
> > diff --git a/libavformat/tcp.c b/libavformat/tcp.c
> > index a11ccbb913..598d61067e 100644
> > --- a/libavformat/tcp.c
> > +++ b/libavformat/tcp.c
> > @@ -36,6 +36,8 @@ typedef struct TCPContext {
> >      const AVClass *class;
> >      int fd;
> >      int listen;
> > +    int local_port;
> > +    char *local_addr;
> >      int open_timeout;
> >      int rw_timeout;
> >      int listen_timeout;
> > @@ -52,6 +54,8 @@ typedef struct TCPContext {
> >  #define E AV_OPT_FLAG_ENCODING_PARAM
> >  static const AVOption options[] = {
> >      { "listen",          "Listen for incoming connections",
> OFFSET(listen),         AV_OPT_TYPE_INT, { .i64 = 0 },     0,       2,
>  .flags = D|E },
> > +    { "local_port",      "Local port",
>        OFFSET(local_port),     AV_OPT_TYPE_INT,    { .i64 = -1 },      -1,
> INT_MAX, .flags = D|E },
> > +    { "local_addr",      "Local address",
>         OFFSET(local_addr),     AV_OPT_TYPE_STRING, { .str = NULL },
>  0,       0, .flags = D|E },
> >      { "timeout",     "set timeout (in microseconds) of socket I/O
> operations", OFFSET(rw_timeout),     AV_OPT_TYPE_INT, { .i64 = -1 },
>  -1, INT_MAX, .flags = D|E },
> >      { "listen_timeout",  "Connection awaiting timeout (in
> milliseconds)",      OFFSET(listen_timeout), AV_OPT_TYPE_INT, { .i64 = -1
> },         -1, INT_MAX, .flags = D|E },
> >      { "send_buffer_size", "Socket send buffer size (in bytes)",
>         OFFSET(send_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 },
>  -1, INT_MAX, .flags = D|E },
> > @@ -73,6 +77,39 @@ static const AVClass tcp_class = {
> >  static void customize_fd(void *ctx, int fd)
> >  {
> >      TCPContext *s = ctx;
> > +
>
> > +    if (s->local_addr) {
>
> s->local_addr || s->local_port
>
> > +        struct addrinfo hints = { 0 }, *ai;
> > +        int ret;
> > +        int port = 0;
> > +
> > +        hints.ai_family = AF_UNSPEC;
> > +        hints.ai_socktype = SOCK_STREAM;
> > +
> > +        if (s->local_port > 0)
> > +            port = s->local_port;
> > +
>
> > +        ret = getaddrinfo(s->local_addr, "0", &hints, &ai);
>
> The provided port should be used instead of this "0", and be a string,
> and named service.
>
> (The remote port should be a string too, alas.)
>
> > +        if (ret) {
>
> > +            av_log(ctx, AV_LOG_WARNING,
> > +               "Failed to bind local addr: %s port: %d err: %s\n",
> > +                s->local_addr, s->local_port, gai_strerror(ret));
>
> Should be an error: failing to bind on the specified address can be a
> security issue. And the error message is misleading.
>
> > +        } else {
>
> > +            if (ai->ai_family == AF_INET6) {
> > +                struct sockaddr_in6 * sockaddr_v6 = (struct
> sockaddr_in6 *)ai->ai_addr;
> > +                sockaddr_v6->sin6_port = htons(port);
> > +            } else {
> > +                struct sockaddr_in * sockaddr = (struct sockaddr_in
> *)ai->ai_addr;
> > +                sockaddr->sin_port = htons(port);
> > +            }
>
> Do not distinguish between protocols: ai_addr already contains all that
> is needed.
>
> > +            ret = bind(fd, (struct sockaddr *)ai->ai_addr,
> (int)ai->ai_addrlen);
> > +            if (ret) {
> > +                av_log(ctx, AV_LOG_WARNING,
> > +                "Failed to bind local addr: %s port: %d err: %s\n",
> > +                    s->local_addr, s->local_port, gai_strerror(ret));
> > +            }
>
> You should be looping over the returned addresses.
>
> > +        }
> > +    }
> >      /* Set the socket's send or receive buffer sizes, if specified.
> >         If unspecified or setting fails, system default is used. */
> >      if (s->recv_buffer_size > 0) {
> > @@ -129,6 +166,13 @@ static int tcp_open(URLContext *h, const char *uri,
> int flags)
> >              if (buf == endptr)
> >                  s->listen = 1;
> >          }
> > +        if (av_find_info_tag(buf, sizeof(buf), "local_port", p)) {
> > +            s->local_port = strtol(buf, NULL, 10);
> > +        }
> > +        if (av_find_info_tag(buf, sizeof(buf), "local_addr", p)) {
> > +            av_freep(&s->local_addr);
> > +            s->local_addr = av_strndup(buf, strlen(buf));
> > +        }
> >          if (av_find_info_tag(buf, sizeof(buf), "timeout", p)) {
> >              s->rw_timeout = strtol(buf, NULL, 10);
> >          }
>
> Regards,
>
> --
>   Nicolas George
>
diff mbox series

Patch

diff --git a/libavformat/tcp.c b/libavformat/tcp.c
index a11ccbb913..598d61067e 100644
--- a/libavformat/tcp.c
+++ b/libavformat/tcp.c
@@ -36,6 +36,8 @@  typedef struct TCPContext {
     const AVClass *class;
     int fd;
     int listen;
+    int local_port;
+    char *local_addr;
     int open_timeout;
     int rw_timeout;
     int listen_timeout;
@@ -52,6 +54,8 @@  typedef struct TCPContext {
 #define E AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
     { "listen",          "Listen for incoming connections",  OFFSET(listen),         AV_OPT_TYPE_INT, { .i64 = 0 },     0,       2,       .flags = D|E },
+    { "local_port",      "Local port",                                         OFFSET(local_port),     AV_OPT_TYPE_INT,    { .i64 = -1 },      -1, INT_MAX, .flags = D|E },
+    { "local_addr",      "Local address",                                      OFFSET(local_addr),     AV_OPT_TYPE_STRING, { .str = NULL },     0,       0, .flags = D|E },
     { "timeout",     "set timeout (in microseconds) of socket I/O operations", OFFSET(rw_timeout),     AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
     { "listen_timeout",  "Connection awaiting timeout (in milliseconds)",      OFFSET(listen_timeout), AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
     { "send_buffer_size", "Socket send buffer size (in bytes)",                OFFSET(send_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
@@ -73,6 +77,39 @@  static const AVClass tcp_class = {
 static void customize_fd(void *ctx, int fd)
 {
     TCPContext *s = ctx;
+
+    if (s->local_addr) {
+        struct addrinfo hints = { 0 }, *ai;
+        int ret;
+        int port = 0;
+
+        hints.ai_family = AF_UNSPEC;
+        hints.ai_socktype = SOCK_STREAM;
+
+        if (s->local_port > 0)
+            port = s->local_port;
+
+        ret = getaddrinfo(s->local_addr, "0", &hints, &ai);
+        if (ret) {
+            av_log(ctx, AV_LOG_WARNING,
+               "Failed to bind local addr: %s port: %d err: %s\n",
+                s->local_addr, s->local_port, gai_strerror(ret));
+        } else {
+            if (ai->ai_family == AF_INET6) {
+                struct sockaddr_in6 * sockaddr_v6 = (struct sockaddr_in6 *)ai->ai_addr;
+                sockaddr_v6->sin6_port = htons(port);
+            } else {
+                struct sockaddr_in * sockaddr = (struct sockaddr_in *)ai->ai_addr;
+                sockaddr->sin_port = htons(port);
+            }
+            ret = bind(fd, (struct sockaddr *)ai->ai_addr, (int)ai->ai_addrlen);
+            if (ret) {
+                av_log(ctx, AV_LOG_WARNING,
+                "Failed to bind local addr: %s port: %d err: %s\n",
+                    s->local_addr, s->local_port, gai_strerror(ret));
+            }
+        }
+    }
     /* Set the socket's send or receive buffer sizes, if specified.
        If unspecified or setting fails, system default is used. */
     if (s->recv_buffer_size > 0) {
@@ -129,6 +166,13 @@  static int tcp_open(URLContext *h, const char *uri, int flags)
             if (buf == endptr)
                 s->listen = 1;
         }
+        if (av_find_info_tag(buf, sizeof(buf), "local_port", p)) {
+            s->local_port = strtol(buf, NULL, 10);
+        }
+        if (av_find_info_tag(buf, sizeof(buf), "local_addr", p)) {
+            av_freep(&s->local_addr);
+            s->local_addr = av_strndup(buf, strlen(buf));
+        }
         if (av_find_info_tag(buf, sizeof(buf), "timeout", p)) {
             s->rw_timeout = strtol(buf, NULL, 10);
         }