[FFmpeg-devel] [PATCH] Add support for digest auth in the http and rtsp protocols

[Ronald S. Bultje]

Hi,

On Thu, Mar 25, 2010 at 5:08 PM, Martin Storsj? <martin at martin.st> wrote:
> On Thu, 25 Mar 2010, Ronald S. Bultje wrote:
>> On Thu, Mar 25, 2010 at 4:09 AM, Martin Storsj? <martin at martin.st> wrote:
>> [..]
>> > +static int find_method_and_url(char *buf, const char **method, const char **url)
>> [..]
>>
>> As said on IRC, this is a little ugly, we're basically creating a line
>> from elements and then splitting again.
>>
>> You can say no then I'll do it ;-), but we need to split this function
>> a little so it takes (instead of just "cmd") a "method", "uri" and
>> "cmd" (which is original cmd - method/uri), and then have
>> ff_rtsp_send_cmd_with_content_async() construct the correct first
>> line. Might not be ideal, but will prevent the having to split it
>> again, which I personally consider ugly.
>
> Ok, tested this approach now.
>
> This requires a bit of modifications to all users of the rtsp_send_cmd
> functions, but IMO it actually cleans some things up a little. And thanks
> to this, the later patches become much more acceptable.
[..]
> @@ -1002,10 +1003,13 @@ void ff_rtsp_send_cmd_with_content_async(AVFormatContext *s,
>      char buf[4096], buf1[1024];
>
>      rt->seq++;
> -    av_strlcpy(buf, cmd, sizeof(buf));
> +    snprintf(buf, sizeof(buf), "%s %s RTSP/1.0\r\n", method, url);
> +    if (headers)
> +        av_strlcat(buf, headers, sizeof(buf));
>      snprintf(buf1, sizeof(buf1), "CSeq: %d\r\n", rt->seq);
>      av_strlcat(buf, buf1, sizeof(buf));
> -    if (rt->session_id[0] != '\0' && !strstr(cmd, "\nIf-Match:")) {
> +    if (rt->session_id[0] != '\0' && (!headers ||
> +        !strstr(headers, "\nIf-Match:"))) {
>          snprintf(buf1, sizeof(buf1), "Session: %s\r\n", rt->session_id);
>          av_strlcat(buf, buf1, sizeof(buf));
>      }

Completely irrelevant here, but can you change this (afterwards) to
use av_strlcatf() instead of this buf1[] mess?

> @@ -1365,9 +1366,7 @@ static int rtsp_setup_output_streams(AVFormatContext *s, const char *addr)
>
>      /* Announce the stream */
>      snprintf(cmd, sizeof(cmd),
> -             "ANNOUNCE %s RTSP/1.0\r\n"
> -             "Content-Type: application/sdp\r\n",
> -             rt->control_uri);
> +             "Content-Type: application/sdp\r\n");
>      sdp = av_mallocz(8192);
>      if (sdp == NULL)
>          return AVERROR(ENOMEM);
> @@ -1392,7 +1391,8 @@ static int rtsp_setup_output_streams(AVFormatContext *s, const char *addr)
>          return AVERROR_INVALIDDATA;
>      }
>      av_log(s, AV_LOG_INFO, "SDP:\n%s\n", sdp);
> -    ff_rtsp_send_cmd_with_content(s, cmd, reply, NULL, sdp, strlen(sdp));
> +    ff_rtsp_send_cmd_with_content(s, "ANNOUNCE", rt->control_uri, cmd,
> +                                  reply, NULL, sdp, strlen(sdp));
>      av_free(sdp);
>      if (reply->status_code != RTSP_STATUS_OK)
>          return AVERROR_INVALIDDATA;

cmd is unneeded and can be removed.

Rest for this patch OK, feel free to commit. Patch #2 is also OK.

For #3:

> @@ -871,6 +872,12 @@ void ff_rtsp_parse_line(RTSPMessageHeader *reply, const char *buf)
>      } else if (av_stristart(p, "Location:", &p)) {
>          skip_spaces(&p);
>          av_strlcpy(reply->location, p , sizeof(reply->location));
> +    } else if (av_stristart(p, "WWW-Authenticate:", &p)) {
> +        skip_spaces(&p);
> +        ff_http_auth_handle_header(auth_state, "WWW-Authenticate", p);
> +    } else if (av_stristart(p, "Authentication-Info:", &p)) {
> +        skip_spaces(&p);
> +        ff_http_auth_handle_header(auth_state, "Authentication-Info", p);
>      }
>  }

This needs an if (auth_state), else it'll crash for ffserver. Can be
committed with that modification.

Patch #4 is OK then.

Ronald