diff mbox series

[FFmpeg-devel] avformat/avio: use av_match_name to match protocol lists

Message ID 20220420203328.67317-1-leo.izen@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/avio: use av_match_name to match protocol lists | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Leo Izen April 20, 2022, 8:33 p.m. UTC
This swaps av_match_list to av_match_name inside ffurl_connect,
which allows ALL to be treated as a valid value on the whitelist
or blacklist in accordance with the behavior described in the
documentation.
---
 libavformat/avio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Paul B Mahol April 21, 2022, 10:54 a.m. UTC | #1
On Wed, Apr 20, 2022 at 10:34 PM Leo Izen <leo.izen@gmail.com> wrote:

> This swaps av_match_list to av_match_name inside ffurl_connect,
> which allows ALL to be treated as a valid value on the whitelist
> or blacklist in accordance with the behavior described in the
> documentation.
> ---
>  libavformat/avio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/avio.c b/libavformat/avio.c
> index 4846bbd8c6..d252bec2f3 100644
> --- a/libavformat/avio.c
> +++ b/libavformat/avio.c
> @@ -178,12 +178,12 @@ int ffurl_connect(URLContext *uc, AVDictionary
> **options)
>      av_assert0(!(e=av_dict_get(*options, "protocol_blacklist", NULL, 0))
> ||
>                 (uc->protocol_blacklist && !strcmp(uc->protocol_blacklist,
> e->value)));
>
> -    if (uc->protocol_whitelist && av_match_list(uc->prot->name,
> uc->protocol_whitelist, ',') <= 0) {
> +    if (uc->protocol_whitelist && av_match_name(uc->prot->name,
> uc->protocol_whitelist) <= 0) {
>          av_log(uc, AV_LOG_ERROR, "Protocol '%s' not on whitelist
> '%s'!\n", uc->prot->name, uc->protocol_whitelist);
>          return AVERROR(EINVAL);
>      }
>
> -    if (uc->protocol_blacklist && av_match_list(uc->prot->name,
> uc->protocol_blacklist, ',') > 0) {
> +    if (uc->protocol_blacklist && av_match_name(uc->prot->name,
> uc->protocol_blacklist) > 0) {
>          av_log(uc, AV_LOG_ERROR, "Protocol '%s' on blacklist '%s'!\n",
> uc->prot->name, uc->protocol_blacklist);
>          return AVERROR(EINVAL);
>      }
>

So this still works if multiple items are provided ?
Leo Izen April 21, 2022, 2:16 p.m. UTC | #2
On 4/21/22 06:54, Paul B Mahol wrote:
>
> So this still works if multiple items are provided ?
>
It does. Running something like this:

$ ./ffmpeg_g -protocol_whitelist http,file -i test.png

successfully reads the file.

- Leo Izen (thebombzen)
Leo Izen April 28, 2022, 12:43 p.m. UTC | #3
On 4/20/22 16:33, Leo Izen wrote:
> This swaps av_match_list to av_match_name inside ffurl_connect,
> which allows ALL to be treated as a valid value on the whitelist
> or blacklist in accordance with the behavior described in the
> documentation.
> ---
>   libavformat/avio.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
This has been on the ML for a week, can someone take a look at it? 
Thanks in advance.

Leo Izen (thebombzen)
Michael Niedermayer April 28, 2022, 8:28 p.m. UTC | #4
On Thu, Apr 28, 2022 at 08:43:21AM -0400, Leo Izen wrote:
> On 4/20/22 16:33, Leo Izen wrote:
> > This swaps av_match_list to av_match_name inside ffurl_connect,
> > which allows ALL to be treated as a valid value on the whitelist
> > or blacklist in accordance with the behavior described in the
> > documentation.
> > ---
> >   libavformat/avio.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> This has been on the ML for a week, can someone take a look at it? Thanks in
> advance.

protocol whitelist documents ALL
protocol blacklist is undocumented
codec whitelist does not document ALL
format whitelist does not document ALL

There may be a very valid reason but i cannot remember it

also adding entries with - to the blacklist disables them. Iam not sure
if this is great. One could assume that adding an entry to the backlist
would increase but never decrease what is blocked

thx

[...]
Leo Izen April 29, 2022, 1:23 a.m. UTC | #5
On 4/28/22 16:28, Michael Niedermayer wrote:
> On Thu, Apr 28, 2022 at 08:43:21AM -0400, Leo Izen wrote:
>> On 4/20/22 16:33, Leo Izen wrote:
>>> This swaps av_match_list to av_match_name inside ffurl_connect,
>>> which allows ALL to be treated as a valid value on the whitelist
>>> or blacklist in accordance with the behavior described in the
>>> documentation.
>>> ---
>>>    libavformat/avio.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
> protocol whitelist documents ALL
> protocol blacklist is undocumented
> codec whitelist does not document ALL
> format whitelist does not document ALL
>
This patch only affects the protocol list, and I could change it so it 
only affects the whitelist, not the blacklist. Thoughts?

-Leo Izen (thebombzen)
diff mbox series

Patch

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 4846bbd8c6..d252bec2f3 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -178,12 +178,12 @@  int ffurl_connect(URLContext *uc, AVDictionary **options)
     av_assert0(!(e=av_dict_get(*options, "protocol_blacklist", NULL, 0)) ||
                (uc->protocol_blacklist && !strcmp(uc->protocol_blacklist, e->value)));
 
-    if (uc->protocol_whitelist && av_match_list(uc->prot->name, uc->protocol_whitelist, ',') <= 0) {
+    if (uc->protocol_whitelist && av_match_name(uc->prot->name, uc->protocol_whitelist) <= 0) {
         av_log(uc, AV_LOG_ERROR, "Protocol '%s' not on whitelist '%s'!\n", uc->prot->name, uc->protocol_whitelist);
         return AVERROR(EINVAL);
     }
 
-    if (uc->protocol_blacklist && av_match_list(uc->prot->name, uc->protocol_blacklist, ',') > 0) {
+    if (uc->protocol_blacklist && av_match_name(uc->prot->name, uc->protocol_blacklist) > 0) {
         av_log(uc, AV_LOG_ERROR, "Protocol '%s' on blacklist '%s'!\n", uc->prot->name, uc->protocol_blacklist);
         return AVERROR(EINVAL);
     }