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

Stefano Sabatini stefasab at gmail.com
Fri Feb 22 19:44:18 CET 2013


On date Thursday 2013-02-14 01:38:46 +0530, Senthilnathan Maadasamy encoded:
> On Tue, Feb 12, 2013 at 5:12 AM, Stefano Sabatini <stefasab at gmail.com>wrote:
> 
> >
> > > + * 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.
> 
> 
> I have made the suggested changes in the patch below:
> 
> Support for special characters in URL
> 
> Signed-off-by: Senthilnathan M <senthilnathan.maadasamy at gmail.com>
> ---
>  libavformat/utils.c |   54
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 10d2449..a068884 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -3754,6 +3754,40 @@ 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);
>  }
> 
> +/**
> + * Percent encode part of an URL string according to RFC 3986.
> + *
> + * @param dst destination buffer where the percent-encoded string is
> written

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

percent-encode

> + * @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 in bytes of the dst buffer
> + * @return complete length of the percent-encoded string
> + */
> +static size_t ff_url_percent_encode(char *dst, const char *src,
> +                                    const char *allowed, size_t dst_size)

note: since this is *static*, the ff_ is not even required, you can
name it "percent_encode_url()" for example

> +{
> +    char c;
> +    int enc_len = 0;
> +
> +    while (c = *src) {
> +        if (isalnum(c) || strchr("-._~%", c)
> +            || (allowed && strchr(allowed, c))) {

> +            if (enc_len+1 < dst_size) dst[enc_len] = c;
> +            enc_len++;


> +        } else {
> +            if (enc_len+3 < dst_size) snprintf(&dst[enc_len], 4, "%%%02x",
> c);
> +            enc_len += 3;
> +        }
> +        src++;
> +    }
> +

> +    if (enc_len < dst_size) dst[enc_len] = '\0';

This should always terminate the buffer, even when the buffer is not
long enough (but see comment below).

> +    enc_len++;
> +    return enc_len;
> +}
> +
>  void av_url_split(char *proto, int proto_size,
>                    char *authorization, int authorization_size,
>                    char *hostname, int hostname_size,
> @@ -3762,6 +3796,9 @@ 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];
> +    size_t enc_size;
> 
>      if (port_ptr)               *port_ptr = -1;
>      if (proto_size > 0)         proto[0] = 0;
> @@ -3817,6 +3854,23 @@ void av_url_split(char *proto, int proto_size,
>              av_strlcpy(hostname, p,
>                         FFMIN(ls + 1 - p, hostname_size));
>      }
> +

> +    enc_size = ff_url_percent_encode(hostname_enc, hostname, NULL,
> +                                        hostname_size);
> +    if (enc_size <= hostname_size) {
> +        av_strlcpy(hostname, hostname_enc, hostname_size);
> +    } else {
> +        av_log(NULL, AV_LOG_ERROR,
> +            "Skipping hostname percent-encoding since buffer is too small\n");
> +    }
> +
> +    enc_size = ff_url_percent_encode(path_enc, path, "/?", path_size);
> +    if (enc_size <= path_size) {
> +        av_strlcpy(path, path_enc, path_size);
> +    } else {
> +        av_log(NULL, AV_LOG_ERROR,
> +            "Skipping path percent-encoding since buffer is too small\n");
> +    }

I'm sorry to swing back and forth, since this is basically a custom
function (no use outside this file), it might be better to implement
your first approach, in order to avoid duplication and keep it simple
(that is you do in-place encoding).

Also the warning may be a bit more descriptive, for example:

       av_log(NULL, AV_LOG_ERROR,
              "Skipping percent-encoding for string '%s' since buffer is too small\n", );

Then as I wrote av_url_split() suffers major problems (no way to
signal errors, e.g. in case of buffer truncation), but fixing it is
beyond the scope of this patch I guess.

BTW please send a git-format patch, so I don't have to fight with
patching tools for applying the patch.
-- 
FFmpeg = Fundamentalist & Fabulous Mastodontic Patchable Erroneous Gymnast


More information about the ffmpeg-devel mailing list