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

[Jun Li]

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