diff mbox

[FFmpeg-devel] Adding a flag to give user the option to have ffmpeg fail instead of warn when mismatches are found in rtmp url stream or application names.

Message ID 20190925185708.70924-1-unique.will.martin@gmail.com
State New
Headers show

Commit Message

William Martin Sept. 25, 2019, 6:57 p.m. UTC
From: Will Martin <will.martin@verizondigitalmedia.com>

Motivation: When running multiple rtmp ingest on the same machine on the same port, users may want to explicitly forbid mismatched rtmp streams from successfully completing handshakes. This patch allows for such enforcement
Signed-off-by: Will Martin <will.martin@verizondigitalmedia.com>
---
 libavformat/librtmp.c   |  2 ++
 libavformat/rtmpproto.c | 24 ++++++++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

Comments

Reino Wijnsma Sept. 25, 2019, 9:36 p.m. UTC | #1
On 2019-09-25T20:57:08+0200, William Martin <unique.will.martin@gmail.com> wrote:
> +            av_log(s, AV_LOG_ERROR, "App field don't match up: %s <-> %s. "
Although I'm not a native English speaker, since "field" isn't plural, I'm pretty confident "don't" should be "doesn't".

-- Reino
Carl Eugen Hoyos Sept. 25, 2019, 10:14 p.m. UTC | #2
Am Mi., 25. Sept. 2019 um 21:04 Uhr schrieb William Martin
<unique.will.martin@gmail.com>:
>
> From: Will Martin <will.martin@verizondigitalmedia.com>
>
> Motivation: When running multiple rtmp ingest on the same machine on the same port, users may want to explicitly forbid mismatched rtmp streams from successfully completing handshakes. This patch allows for such enforcement

There already is a "strict" option, can't it be used here instead
of adding a new option?

Carl Eugen
William Martin Sept. 26, 2019, 11:31 p.m. UTC | #3
I don't think so. The existing 'strict' option is more general and touches
lots of things in encoding/decoding. The new option is very narrow in
scope. The existing 'strict' and the new 'rtmp_strict_paths` don't have
anything to do with each other - in fact it would be quite possible that a
user might want one off and the other on.

On Wed, Sep 25, 2019 at 3:14 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> Am Mi., 25. Sept. 2019 um 21:04 Uhr schrieb William Martin
> <unique.will.martin@gmail.com>:
> >
> > From: Will Martin <will.martin@verizondigitalmedia.com>
> >
> > Motivation: When running multiple rtmp ingest on the same machine on the
> same port, users may want to explicitly forbid mismatched rtmp streams from
> successfully completing handshakes. This patch allows for such enforcement
>
> There already is a "strict" option, can't it be used here instead
> of adding a new option?
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
William Martin Sept. 27, 2019, 5:37 p.m. UTC | #4
Hi Reino - good point. Though, that was an existing log message that I did
not modify. Should the old typo be addressed in a separate patch, or should
I update it here?

On Wed, Sep 25, 2019 at 2:36 PM Reino Wijnsma <rwijnsma@xs4all.nl> wrote:

> On 2019-09-25T20:57:08+0200, William Martin <unique.will.martin@gmail.com>
> wrote:
> > +            av_log(s, AV_LOG_ERROR, "App field don't match up: %s <->
> %s. "
> Although I'm not a native English speaker, since "field" isn't plural, I'm
> pretty confident "don't" should be "doesn't".
>
> -- Reino
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Reino Wijnsma Sept. 28, 2019, 9:55 a.m. UTC | #5
On 2019-09-27T19:37:40+0200, William Martin <unique.will.martin@gmail.com> wrote:
> On Wed, Sep 25, 2019 at 2:36 PM Reino Wijnsma <rwijnsma@xs4all.nl> wrote:
>> On 2019-09-25T20:57:08+0200, William Martin <unique.will.martin@gmail.com>
>> wrote:
>>> +            av_log(s, AV_LOG_ERROR, "App field don't match up: %s <-> %s. "
>> Although I'm not a native English speaker, since "field" isn't plural, I'm pretty confident "don't" should be "doesn't".
>>
>> -- Reino
>
> Hi Reino - good point. Though, that was an existing log message that I did
> not modify. Should the old typo be addressed in a separate patch, or should
> I update it here?
>
I believe that's good practise here, yes. However, I'm not a FFmpeg coder, so someone else would to correct me otherwise.

Please don't top-post on this mailinglist!

-- Reino
Michael Niedermayer Jan. 1, 2020, 3:38 p.m. UTC | #6
On Wed, Sep 25, 2019 at 11:57:08AM -0700, William Martin wrote:
> From: Will Martin <will.martin@verizondigitalmedia.com>
> 
> Motivation: When running multiple rtmp ingest on the same machine on the same port, users may want to explicitly forbid mismatched rtmp streams from successfully completing handshakes. This patch allows for such enforcement
> Signed-off-by: Will Martin <will.martin@verizondigitalmedia.com>
> ---
>  libavformat/librtmp.c   |  2 ++
>  libavformat/rtmpproto.c | 24 ++++++++++++++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
> index 43013e46e0..00b49666fd 100644
> --- a/libavformat/librtmp.c
> +++ b/libavformat/librtmp.c
> @@ -52,6 +52,7 @@ typedef struct LibRTMPContext {
>      int live;
>      char *temp_filename;
>      int buffer_size;
> +    bool strict_paths;
>  } LibRTMPContext;
>  
>  static void rtmp_log(int level, const char *fmt, va_list args)
> @@ -333,6 +334,7 @@ static const AVOption options[] = {
>      {"rtmp_swfurl", "URL of the SWF player. By default no value will be sent", OFFSET(swfurl), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC|ENC},
>      {"rtmp_swfverify", "URL to player swf file, compute hash/size automatically. (unimplemented)", OFFSET(swfverify), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC},
>      {"rtmp_tcurl", "URL of the target stream. Defaults to proto://host[:port]/app.", OFFSET(tcurl), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC|ENC},
> +    {"rtmp_strict_paths", "Error instead of warn for mismatch on stream or application path in url", OFFSET(strict_paths), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, DEC},
>  #if CONFIG_NETWORK
>      {"rtmp_buffer_size", "set buffer size in bytes", OFFSET(buffer_size), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, DEC|ENC },
>  #endif
> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
> index b741e421af..dded3b6028 100644
> --- a/libavformat/rtmpproto.c
> +++ b/libavformat/rtmpproto.c
> @@ -129,6 +129,7 @@ typedef struct RTMPContext {
>      char          auth_params[500];
>      int           do_reconnect;
>      int           auth_tried;
> +    int           strict_paths;               ///< If true, enforce strict string matching on rtmp stream and application
>  } RTMPContext;
>  
>  #define PLAYER_KEY_OPEN_PART_LEN 30   ///< length of partial key used for first client digest signing
> @@ -477,9 +478,16 @@ static int read_connect(URLContext *s, RTMPContext *rt)
>                                   "app", tmpstr, sizeof(tmpstr));
>      if (ret)
>          av_log(s, AV_LOG_WARNING, "App field not found in connect\n");
> -    if (!ret && strcmp(tmpstr, rt->app))
> -        av_log(s, AV_LOG_WARNING, "App field don't match up: %s <-> %s\n",
> +    if (!ret && strcmp(tmpstr, rt->app)) {
> +        if (rt->strict_paths) {
> +            av_log(s, AV_LOG_ERROR, "App field don't match up: %s <-> %s. "
> +               "Exiting since rtmp_strict_paths provided\n", tmpstr, rt->app);
> +            return AVERROR(EIO);
> +        } else {
> +            av_log(s, AV_LOG_WARNING, "App field don't match up: %s <-> %s\n",
>                 tmpstr, rt->app);
> +        }
> +    }
>      ff_rtmp_packet_destroy(&pkt);
>  
>      // Send Window Acknowledgement Size (as defined in specification)
> @@ -1946,9 +1954,16 @@ static int send_invoke_response(URLContext *s, RTMPPacket *pkt)
>                  pchar = s->filename;
>              }
>              pchar++;
> -            if (strcmp(pchar, filename))
> -                av_log(s, AV_LOG_WARNING, "Unexpected stream %s, expecting"
> +            if (strcmp(pchar, filename)) {
> +                if (rt->strict_paths) {
> +                    av_log(s, AV_LOG_ERROR, "Unexpected stream %s, expecting %s. "
> +                        "Exiting since rtmp_strict_paths provided.\n", filename, pchar);
> +                    return AVERROR(EIO);
> +                } else {
> +                    av_log(s, AV_LOG_WARNING, "Unexpected stream %s, expecting"
>                         " %s\n", filename, pchar);
> +                }
> +            }
>          }
>          rt->state = STATE_RECEIVING;
>      }
> @@ -3112,6 +3127,7 @@ static const AVOption rtmp_options[] = {
>      {"rtmp_listen", "Listen for incoming rtmp connections", OFFSET(listen), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC, "rtmp_listen" },
>      {"listen",      "Listen for incoming rtmp connections", OFFSET(listen), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC, "rtmp_listen" },
>      {"timeout", "Maximum timeout (in seconds) to wait for incoming connections. -1 is infinite. Implies -rtmp_listen 1",  OFFSET(listen_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC, "rtmp_listen" },
> +    {"rtmp_strict_paths", "Error instead of warn for mismatch on stream or application path in url", OFFSET(strict_paths), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, DEC},
>      { NULL },
>  };

These should be also documented in doc/protocols.texi

Thanks

[...]
diff mbox

Patch

diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
index 43013e46e0..00b49666fd 100644
--- a/libavformat/librtmp.c
+++ b/libavformat/librtmp.c
@@ -52,6 +52,7 @@  typedef struct LibRTMPContext {
     int live;
     char *temp_filename;
     int buffer_size;
+    bool strict_paths;
 } LibRTMPContext;
 
 static void rtmp_log(int level, const char *fmt, va_list args)
@@ -333,6 +334,7 @@  static const AVOption options[] = {
     {"rtmp_swfurl", "URL of the SWF player. By default no value will be sent", OFFSET(swfurl), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC|ENC},
     {"rtmp_swfverify", "URL to player swf file, compute hash/size automatically. (unimplemented)", OFFSET(swfverify), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC},
     {"rtmp_tcurl", "URL of the target stream. Defaults to proto://host[:port]/app.", OFFSET(tcurl), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC|ENC},
+    {"rtmp_strict_paths", "Error instead of warn for mismatch on stream or application path in url", OFFSET(strict_paths), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, DEC},
 #if CONFIG_NETWORK
     {"rtmp_buffer_size", "set buffer size in bytes", OFFSET(buffer_size), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, DEC|ENC },
 #endif
diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index b741e421af..dded3b6028 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -129,6 +129,7 @@  typedef struct RTMPContext {
     char          auth_params[500];
     int           do_reconnect;
     int           auth_tried;
+    int           strict_paths;               ///< If true, enforce strict string matching on rtmp stream and application
 } RTMPContext;
 
 #define PLAYER_KEY_OPEN_PART_LEN 30   ///< length of partial key used for first client digest signing
@@ -477,9 +478,16 @@  static int read_connect(URLContext *s, RTMPContext *rt)
                                  "app", tmpstr, sizeof(tmpstr));
     if (ret)
         av_log(s, AV_LOG_WARNING, "App field not found in connect\n");
-    if (!ret && strcmp(tmpstr, rt->app))
-        av_log(s, AV_LOG_WARNING, "App field don't match up: %s <-> %s\n",
+    if (!ret && strcmp(tmpstr, rt->app)) {
+        if (rt->strict_paths) {
+            av_log(s, AV_LOG_ERROR, "App field don't match up: %s <-> %s. "
+               "Exiting since rtmp_strict_paths provided\n", tmpstr, rt->app);
+            return AVERROR(EIO);
+        } else {
+            av_log(s, AV_LOG_WARNING, "App field don't match up: %s <-> %s\n",
                tmpstr, rt->app);
+        }
+    }
     ff_rtmp_packet_destroy(&pkt);
 
     // Send Window Acknowledgement Size (as defined in specification)
@@ -1946,9 +1954,16 @@  static int send_invoke_response(URLContext *s, RTMPPacket *pkt)
                 pchar = s->filename;
             }
             pchar++;
-            if (strcmp(pchar, filename))
-                av_log(s, AV_LOG_WARNING, "Unexpected stream %s, expecting"
+            if (strcmp(pchar, filename)) {
+                if (rt->strict_paths) {
+                    av_log(s, AV_LOG_ERROR, "Unexpected stream %s, expecting %s. "
+                        "Exiting since rtmp_strict_paths provided.\n", filename, pchar);
+                    return AVERROR(EIO);
+                } else {
+                    av_log(s, AV_LOG_WARNING, "Unexpected stream %s, expecting"
                        " %s\n", filename, pchar);
+                }
+            }
         }
         rt->state = STATE_RECEIVING;
     }
@@ -3112,6 +3127,7 @@  static const AVOption rtmp_options[] = {
     {"rtmp_listen", "Listen for incoming rtmp connections", OFFSET(listen), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC, "rtmp_listen" },
     {"listen",      "Listen for incoming rtmp connections", OFFSET(listen), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC, "rtmp_listen" },
     {"timeout", "Maximum timeout (in seconds) to wait for incoming connections. -1 is infinite. Implies -rtmp_listen 1",  OFFSET(listen_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC, "rtmp_listen" },
+    {"rtmp_strict_paths", "Error instead of warn for mismatch on stream or application path in url", OFFSET(strict_paths), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, DEC},
     { NULL },
 };