[FFmpeg-devel] [PATCH v4] avformat/http, icecast, rtsp: Add option to disable send-100-continue

Jun Li junli1026 at gmail.com
Sun Mar 31 07:43:03 EEST 2019


Thanks Michael for review.
I updated the iteration to address your feedback:
https://patchwork.ffmpeg.org/patch/12540/

Best Regards,
-Jun

On Sat, Mar 30, 2019 at 6:14 PM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Fri, Mar 29, 2019 at 03:59:21PM -0700, Jun Li wrote:
> > On Wed, Mar 27, 2019 at 11:27 PM Jun Li <junli1026 at gmail.com> wrote:
> >
> > >
> > >
> > > On Mon, Mar 25, 2019 at 2:42 PM Jun Li <junli1026 at gmail.com> wrote:
> > >
> > >>
> > >>
> > >> On Fri, Mar 22, 2019 at 4:12 PM Jun Li <junli1026 at gmail.com> wrote:
> > >>
> > >>> The current setting for send-100-continue option is either
> > >>> enabled if applicable or forced enabled, no option to force
> > >>> disable the header. This change is to expand the option setting
> > >>> to provide more flexibility, which is useful for rstp case.
> > >>> ---
> > >>>  libavformat/http.c    | 28 +++++++++++++++++-----------
> > >>>  libavformat/icecast.c |  2 +-
> > >>>  libavformat/rtsp.c    |  1 +
> > >>>  3 files changed, 19 insertions(+), 12 deletions(-)
> > >>>
> > >>> diff --git a/libavformat/http.c b/libavformat/http.c
> > >>> index 74d743850d..5a937994cf 100644
> > >>> --- a/libavformat/http.c
> > >>> +++ b/libavformat/http.c
> > >>> @@ -113,6 +113,7 @@ typedef struct HTTPContext {
> > >>>      uint8_t *inflate_buffer;
> > >>>  #endif /* CONFIG_ZLIB */
> > >>>      AVDictionary *chained_options;
> > >>> +    /* -1 = try to send if applicable, 0 = always disabled, 1 =
> always
> > >>> enabled */
> > >>>      int send_expect_100;
> > >>>      char *method;
> > >>>      int reconnect;
> > >>> @@ -155,7 +156,7 @@ static const AVOption options[] = {
> > >>>      { "auth_type", "HTTP authentication type",
> > >>> OFFSET(auth_state.auth_type), AV_OPT_TYPE_INT, { .i64 =
> HTTP_AUTH_NONE },
> > >>> HTTP_AUTH_NONE, HTTP_AUTH_BASIC, D | E, "auth_type"},
> > >>>      { "none", "No auth method set, autodetect", 0,
> AV_OPT_TYPE_CONST, {
> > >>> .i64 = HTTP_AUTH_NONE }, 0, 0, D | E, "auth_type"},
> > >>>      { "basic", "HTTP basic authentication", 0, AV_OPT_TYPE_CONST, {
> > >>> .i64 = HTTP_AUTH_BASIC }, 0, 0, D | E, "auth_type"},
> > >>> -    { "send_expect_100", "Force sending an Expect: 100-continue
> header
> > >>> for POST", OFFSET(send_expect_100), AV_OPT_TYPE_BOOL, { .i64 = 0 },
> 0, 1, E
> > >>> },
> > >>> +    { "send_expect_100", "Force sending an Expect: 100-continue
> header
> > >>> for POST", OFFSET(send_expect_100), AV_OPT_TYPE_BOOL, { .i64 = -1 },
> -1, 1,
> > >>> E },
> > >>>      { "location", "The actual location of the data received",
> > >>> OFFSET(location), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
> > >>>      { "offset", "initial byte offset", OFFSET(off),
> AV_OPT_TYPE_INT64,
> > >>> { .i64 = 0 }, 0, INT64_MAX, D },
> > >>>      { "end_offset", "try to limit the request to bytes preceding
> this
> > >>> offset", OFFSET(end_off), AV_OPT_TYPE_INT64, { .i64 = 0 }, 0,
> INT64_MAX, D
> > >>> },
> > >>> @@ -1179,16 +1180,21 @@ static int http_connect(URLContext *h, const
> > >>> char *path, const char *local_path,
> > >>>                                                  local_path, method);
> > >>>      proxyauthstr =
> ff_http_auth_create_response(&s->proxy_auth_state,
> > >>> proxyauth,
> > >>>                                                  local_path, method);
> > >>> -    if (post && !s->post_data) {
> > >>> -        send_expect_100 = s->send_expect_100;
> > >>> -        /* The user has supplied authentication but we don't know
> the
> > >>> auth type,
> > >>> -         * send Expect: 100-continue to get the 401 response
> including
> > >>> the
> > >>> -         * WWW-Authenticate header, or an 100 continue if no auth
> > >>> actually
> > >>> -         * is needed. */
> > >>> -        if (auth && *auth &&
> > >>> -            s->auth_state.auth_type == HTTP_AUTH_NONE &&
> > >>> -            s->http_code != 401)
> > >>> -            send_expect_100 = 1;
> > >>> +
> > >>> +     if (post && !s->post_data) {
> > >>> +        if (s->send_expect_100 != -1) {
> > >>> +            send_expect_100 = s->send_expect_100;
> > >>> +        } else {
> > >>> +            send_expect_100 = 0;
> > >>> +            /* The user has supplied authentication but we don't
> know
> > >>> the auth type,
> > >>> +             * send Expect: 100-continue to get the 401 response
> > >>> including the
> > >>> +             * WWW-Authenticate header, or an 100 continue if no
> auth
> > >>> actually
> > >>> +             * is needed. */
> > >>> +            if (auth && *auth &&
> > >>> +                s->auth_state.auth_type == HTTP_AUTH_NONE &&
> > >>> +                s->http_code != 401)
> > >>> +                send_expect_100 = 1;
> > >>> +        }
> > >>>      }
> > >>>
> > >>>  #if FF_API_HTTP_USER_AGENT
> > >>> diff --git a/libavformat/icecast.c b/libavformat/icecast.c
> > >>> index c93b06b553..d2198b78ec 100644
> > >>> --- a/libavformat/icecast.c
> > >>> +++ b/libavformat/icecast.c
> > >>> @@ -115,7 +115,7 @@ static int icecast_open(URLContext *h, const char
> > >>> *uri, int flags)
> > >>>      av_dict_set(&opt_dict, "auth_type", "basic", 0);
> > >>>      av_dict_set(&opt_dict, "headers", headers, 0);
> > >>>      av_dict_set(&opt_dict, "chunked_post", "0", 0);
> > >>> -    av_dict_set(&opt_dict, "send_expect_100", s->legacy_icecast ?
> "0" :
> > >>> "1", 0);
> > >>> +    av_dict_set(&opt_dict, "send_expect_100", s->legacy_icecast ?
> "-1"
> > >>> : "1", 0);
> > >>>      if (NOT_EMPTY(s->content_type))
> > >>>          av_dict_set(&opt_dict, "content_type", s->content_type, 0);
> > >>>      else
> > >>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > >>> index ae8811234a..6e67e411b9 100644
> > >>> --- a/libavformat/rtsp.c
> > >>> +++ b/libavformat/rtsp.c
> > >>> @@ -1793,6 +1793,7 @@ redirect:
> > >>>                   sessioncookie);
> > >>>          av_opt_set(rt->rtsp_hd_out->priv_data, "headers", headers,
> 0);
> > >>>          av_opt_set(rt->rtsp_hd_out->priv_data, "chunked_post", "0",
> 0);
> > >>> +        av_opt_set(rt->rtsp_hd_out->priv_data, "send_expect_100",
> "0",
> > >>> 0);
> > >>>
> > >>>          /* Initialize the authentication state for the POST session.
> > >>> The HTTP
> > >>>           * protocol implementation doesn't properly handle
> multi-pass
> > >>> --
> > >>> 2.17.1
> > >>>
> > >>
> > >>
> > >> Ping.
> > >> The current setting for send_100_expect in http.c is applicable only
> when
> > >> request is POST
> > >> and post body is empty. The code checks the post body as this:
> > >> * +     if (post && !s->post_data) { *
> > >>     ......
> > >>
> > >> This is absolutely correct. But rtsp tunnel in http is a special
> case. It
> > >> post the http header
> > >> without any post_data, and then write the tunneled data into the tcp
> file
> > >> descriptor --  that is
> > >> two steps and skips the s->post_data.
> > >>
> > >> So in the case, it exactly falls into the send_100_expect condition
> > >> although the real post data
> > >> will be written to fd later. It is also an issue when I test it with
> IP
> > >> cameras because they don't
> > >> support "Expect: 100-continue".
> > >>
> > >> I don't see a smart fix in http.c because whoever use http.c can skip
> the
> > >> http API and write
> > >> to the TCP fd directly. This kind of behavior will create a a valid
> post
> > >> operation but http.c
> > >> would never get any information of post_data. It kind of skips the
> http
> > >> layer and directly talk
> > >> to the tcp.
> > >>
> > >> So the patch here I proposed another path --- trust the user who is
> using
> > >> http.c. In this specific
> > >> case, it is rtsp.c who is using http.c, so it is rtsp's obligation to
> > >> configure the correct header since
> > >> it choose to skip http and write post data directly to the file
> > >> descriptor.
> > >>
> > >> This is how this patch comes out.
> > >>
> > >> Thanks
> > >> -Jun
> > >>
> > >
> > >
> > > Ping x 2
> > > Someone met the same issue with me, ticket 7297 --
> > > https://trac.ffmpeg.org/ticket/7297
> > >
> > > This patch is to fix the rtsp tunnel http issue. The root cause is the
> > > "Expect:100-continue" in the http header, it is applicable only when
> > > request is POST and post body is empty. The code checks the post body
> is as
> > > following in http.c:
> > > * +     if (post && !s->post_data) { *
> > >     ......
> > >
> > > This is correct in most cases but rtsp tunnel in http is special. It
> post
> > > the http header WITHOUT any post_data, and then write the post data
> into
> > > the tcp file descriptor directly.
> > >
> > > So in the case, it exactly falls into the send_100_expect condition
> > > although the real post data will be written to fd later. It is also an
> > > issue when I test it with IP cameras because they don't support
> "Expect:
> > > 100-continue".
> > >
> > > So this patch is for fixing the rtsp tunnel http issue.
> > >
> > > Thanks
> > > -Jun
> > >
> >
> >
> > Ping x 3.
> > Could someone take a look at this patch please ? This is for addressing
> > this ticket https://trac.ffmpeg.org/ticket/7297
>
> If this patch fixes ticket 7297 then the commit message should say so
>
> the option also may need documentation unless its private (which it is not
> currently) its listed in -h full
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> "You are 36 times more likely to die in a bathtub than at the hands of a
> terrorist. Also, you are 2.5 times more likely to become a president and
> 2 times more likely to become an astronaut, than to die in a terrorist
> attack." -- Thoughty2
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list