[FFmpeg-devel] [PATCH 2/3] avformat: add protocol_whitelist

[Andreas Cadhalpun]

On 28.01.2016 23:49, Michael Niedermayer wrote:
> From: Michael Niedermayer <michael at niedermayer.cc>
> 
> TODO: Docs
> TODO: version bump
> 
> Note to maintainers: update tools
> 
> Note to maintainers: set a default whitelist for your protocol
> If that makes no sense then consider to set "none" and thus require the user to specify a white-list
> for sub-protocols to be opened
> 
> Note, testing and checking for missing changes is needed
> 
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavformat/avformat.h      |    7 +++++
>  libavformat/avio.c          |   66 ++++++++++++++++++++++++++++++++++++++++---
>  libavformat/avio.h          |    5 ++++
>  libavformat/avio_internal.h |    4 +++
>  libavformat/aviobuf.c       |   25 +++++++++++++---
>  libavformat/options_table.h |    1 +
>  libavformat/url.h           |    6 ++++
>  libavformat/utils.c         |   22 ++++++++++++---
>  8 files changed, 124 insertions(+), 12 deletions(-)

This looks good to me. I just have two minor points, see below.

>  int av_format_get_probe_score(const AVFormatContext *s);
> diff --git a/libavformat/avio.c b/libavformat/avio.c
> index 96b18fd..c5c6bb1 100644
> --- a/libavformat/avio.c
> +++ b/libavformat/avio.c
[...]
> @@ -201,12 +207,42 @@ fail:
>  
>  int ffurl_connect(URLContext *uc, AVDictionary **options)
>  {
> -    int err =
> +    int err;
> +    AVDictionary *tmp_opts = NULL;
> +    AVDictionaryEntry *e;
> +
> +    if (!options)
> +        options = &tmp_opts;
> +
> +    // Check that URLContext was initialized correctly and lists are matching if set
> +    av_assert0(!(e=av_dict_get(*options, "protocol_whitelist", NULL, 0)) ||
> +               !strcmp(uc->protocol_whitelist, e->value));
> +
> +    if (uc->protocol_whitelist && av_match_list(uc->prot->name, uc->protocol_whitelist, ',') <= 0) {
> +        av_log(uc, AV_LOG_ERROR, "Protocol not on whitelist \'%s\'!\n", uc->protocol_whitelist);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    if (!uc->protocol_whitelist && uc->prot->default_whitelist) {
> +        uc->protocol_whitelist = av_strdup(uc->prot->default_whitelist);

I'd find a log message here quite useful for debugging, e.g.:
        av_log(uc, AV_LOG_DEBUG, "Setting default whitelist '%s'\n", uc->prot->default_whitelist);

> diff --git a/libavformat/url.h b/libavformat/url.h
> index 391e3bc..eb950f3 100644
> --- a/libavformat/url.h
> +++ b/libavformat/url.h
> @@ -47,6 +47,7 @@ typedef struct URLContext {
>      int is_connected;
>      AVIOInterruptCB interrupt_callback;
>      int64_t rw_timeout;         /**< maximum time to wait for (network) read/write operation completion, in mcs */
> +    const char *protocol_whitelist;
>  } URLContext;
>  
>  typedef struct URLProtocol {
> @@ -94,6 +95,7 @@ typedef struct URLProtocol {
>      int (*url_close_dir)(URLContext *h);
>      int (*url_delete)(URLContext *h);
>      int (*url_move)(URLContext *h_src, URLContext *h_dst);
> +    char *default_whitelist;

This misses a 'const' and thus produces lots of warnings:
warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

Best regards,
Andreas