[FFmpeg-devel] [PATCH]RTSP Basic Authentication

[Benoit Fouet]

Hi Ronald,

Ronald S. Bultje wrote :
> Hi,
>
> On Sat, Feb 28, 2009 at 12:34 PM, Ronald S. Bultje<rsbultje at gmail.com> wrote:
>   
>> On Mon, Feb 16, 2009 at 1:40 PM, Philip Coombes
>> <philip.coombes at zoneminder.com> wrote:
>>     
>>> Thanks Ronald. I just wanted to make sure that no-one was waiting for me
>>> to do anything.
>>>       
>> I was just checkign to apply this, and you could do something,
>> actually. So, my impression is that this patch adds the auth-line in
>> every RTSP command, is that true? If so, is that necessary? Isn't it
>> sufficient to add it just to one command?
>>
>> Secondly, the auth takes 256 bytes, although it will almost always be
>> absent, and the 256 might then be too small depending on password. For
>> completeness, could you please dynamically allocate the auth string in
>> RTSPState (size = ceil(strlen(auth)*6/8.)+1 or something)?
>>
>> Lastly, if you're going to resend the patch anyway, could you remove
>> all whitespace around if ( <- this one -> condition <- and this one
>> ->) statements, change auth[128] into auth[1024] just like all other
>> strings and put the long av_strlcatf() on just one or two lines
>> instead of 6 like now?
>>     
>
> Well this didn't happen so I did it myself, see attached. If Luca
> doesn't mind I'll apply in 3 days.
>
> Ronald
>   
> ------------------------------------------------------------------------
>
> Index: ffmpeg-svn/libavformat/rtsp.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rtsp.c	2009-07-27 10:00:23.000000000 -0400
> +++ ffmpeg-svn/libavformat/rtsp.c	2009-07-27 16:30:04.000000000 -0400
> @@ -22,6 +22,7 @@
>  /* needed by inet_aton() */
>  #define _SVID_SOURCE
>  
> +#include "libavutil/base64.h"
>  #include "libavutil/avstring.h"
>  #include "libavutil/intreadwrite.h"
>  #include "avformat.h"
> @@ -855,6 +856,10 @@
>          snprintf(buf1, sizeof(buf1), "Session: %s\r\n", rt->session_id);
>          av_strlcat(buf, buf1, sizeof(buf));
>      }
> +    if (rt->auth_b64 != NULL)
>   

you can remove the '!= NULL'

> +        av_strlcatf(buf, sizeof(buf),
> +                   "Authorization: Basic %s\r\n",
> +                   rt->auth_b64);
>      av_strlcat(buf, "\r\n", sizeof(buf));
>  
>      dprintf(s, "Sending:\n%s--\n", buf);
> @@ -899,6 +904,7 @@
>          av_close_input_stream (rt->asf_ctx);
>          rt->asf_ctx = NULL;
>      }
> +    av_freep(&rt->auth_b64);
>  }
>  
>  static int
> @@ -1159,7 +1165,7 @@
>                              AVFormatParameters *ap)
>  {
>      RTSPState *rt = s->priv_data;
> -    char host[1024], path[1024], tcpname[1024], cmd[2048], *option_list, *option;
> +    char auth[128], host[1024], path[1024], tcpname[1024], cmd[2048], *option_list, *option;
>   

it may just be me, but I would find it easier to read if auth definition
was at the end of the line.

>      URLContext *rtsp_hd;
>      int port, ret, err;
>      RTSPMessageHeader reply1, *reply = &reply1;
> @@ -1168,8 +1174,16 @@
>      char real_challenge[64];
>  
>      /* extract hostname and port */
> -    url_split(NULL, 0, NULL, 0,
> +    url_split(NULL, 0, auth, sizeof(auth),
>                host, sizeof(host), &port, path, sizeof(path), s->filename);
> +    if (auth[0] != '\0') {
>   

'if (auth[0])' or even 'if (*auth)'
that would be consistent with how it is initialized in url_split()

> +        int auth_len = strlen(auth), b64_len = ((auth_len + 2) / 3) * 4 + 1;
> +
> +        if (!(rt->auth_b64 = av_malloc(b64_len)))
>   

this is never freed

> +            return AVERROR(ENOMEM);
> +        if (!av_base64_encode(rt->auth_b64, b64_len, auth, strlen(auth)))
>   

s/strlen(auth)/auth_len/

Ben