[FFmpeg-devel] [PATCH] Handling special characters in a URL.

[Stefano Sabatini]

On date Sunday 2013-02-10 23:17:39 +0530, Senthilnathan Maadasamy encoded:
> Hi Stefano,
>                  The attached patch has all the changes suggested by you.
>  Let me know if any more changes are needed.
> 
> Thanks,
> Senthil

> From 027d90015e68d9ec4df8f16d85950241ff0548d8 Mon Sep 17 00:00:00 2001
> From: Senthilnathan M <senthilnathan.maadasamy at gmail.com>
> Date: Sun, 10 Feb 2013 23:08:52 +0530
> Subject: [PATCH] Support for special characters in URL
> 
> Signed-off-by: Senthilnathan M <senthilnathan.maadasamy at gmail.com>
> ---
>  libavformat/avformat.h |   13 +++++++++++++
>  libavformat/utils.c    |   38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 03fca57..ff6c689 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1970,6 +1970,19 @@ int av_index_search_timestamp(AVStream *st, int64_t timestamp, int flags);
>  int av_add_index_entry(AVStream *st, int64_t pos, int64_t timestamp,
>                         int size, int distance, int flags);
>  
> +/**
> + * Percent encodes parts of a URL string based on RFC 3986.

Nit:
Percent encode part of an URL string according to RFC 3986.

The following corrections are subtleties related to internal/global
consistency/semantics.

> + *
> + * @param dst buffer for the percent-encoded URL component

destination buffer where the percent-encoded string is written

> + * @param src portion of a URL (e.g. protocol, hostname, path) which needs to
> + *          be percent-encoded

@param src portion of an URL (e.g. protocol, hostname, path) to percent-encoded

> + * @param allowed string containing the extra-permissible characters which must
> + *          not be encoded. It may be NULL if there is no such character.

@param allowed string containing the allowed characters which must not
be encoded. It may be NULL if there are no such characters.

> + * @param dst_size size of the dst buffer

size in bytes of the dst buffer

> + * @return 0 if OK, -1 on error (if length of percent encoded string is greater
> + *          than dst_size)

I suggest to adopt the snprintf() return style, and return a size_t
containing the *complete* lenght of the encoded string, so you can
allocate a buffer with the correct size on a second iteration.

> + */
> +int ff_url_percent_encode(char *dst, const char *src, const char *allowed, size_t dst_size);

BTW just noted, since this is not a public function, you can move it
to libavformat/utils.c, and declare it
static int ff_url_percent_encode(...)
{
...
}

>  
>  /**
>   * Split a URL string into components.
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 10d2449..2ceecb8 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -3754,6 +3754,28 @@ void av_pkt_dump_log2(void *avcl, int level, AVPacket *pkt, int dump_payload,
>      pkt_dump_internal(avcl, NULL, level, pkt, dump_payload, st->time_base);
>  }
>  
> +int ff_url_percent_encode(char *dst, const char *src, const char *allowed, size_t dst_size)

> +{
> +    char c;
> +    int enc_len = 0;
> +
> +    while (c = *src) {
> +        if (isalnum(c) || strchr("-._~%", c)
> +            || (allowed && strchr(allowed, c))) {
> +            if (enc_len+1 >= dst_size) return -1;
> +            dst[enc_len] = c;
> +            enc_len++;
> +        } else {
> +            if (enc_len+3 >= dst_size) return -1;
> +            snprintf(&dst[enc_len], 4, "%%%02x", c);
> +            enc_len+=3;
> +        }
> +        src++;
> +    }
> +    dst[enc_len] = '\0';
> +    return 0;
> +}
> +
>  void av_url_split(char *proto, int proto_size,
>                    char *authorization, int authorization_size,
>                    char *hostname, int hostname_size,
> @@ -3762,6 +3784,8 @@ void av_url_split(char *proto, int proto_size,
>                    const char *url)
>  {
>      const char *p, *ls, *ls2, *at, *at2, *col, *brk;
> +    char hostname_enc[MAX_URL_SIZE];
> +    char path_enc[MAX_URL_SIZE];
>  
>      if (port_ptr)               *port_ptr = -1;
>      if (proto_size > 0)         proto[0] = 0;
> @@ -3817,6 +3841,20 @@ void av_url_split(char *proto, int proto_size,
>              av_strlcpy(hostname, p,
>                         FFMIN(ls + 1 - p, hostname_size));
>      }
> +
> +    if (ff_url_percent_encode(hostname_enc, hostname, NULL,
> +                              hostname_size) < 0) {
> +        av_log(NULL, AV_LOG_ERROR,
> +            "Skipping hostname percent-encoding since buffer is too small\n");
> +    } else {
> +        av_strlcpy(hostname, hostname_enc, hostname_size);
> +    }
> +    if (ff_url_percent_encode(path_enc, path, "/?", path_size) < 0) {
> +        av_log(NULL, AV_LOG_ERROR,
> +            "Skipping path percent-encoding since buffer is too small\n");
> +    } else {
> +        av_strlcpy(path, path_enc, path_size);
> +    }

I'm not happy about the function inability to signal an encoding error
at the programmatic level, but this is unrelated and I guess logging
an error is better than nothing.

Ideally we should add a new av_url_split variant with proper error
handling.
-- 
FFmpeg = Fantastic and Funny Muttering Portable Evangelical Gangster