diff mbox series

[FFmpeg-devel,v1] lavf/url: fix rel path’s query string contains :/

Message ID 20201015140644.95563-1-caihaoning83@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v1] lavf/url: fix rel path’s query string contains :/
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make warning Make failed

Commit Message

蔡昊凝 Oct. 15, 2020, 2:06 p.m. UTC
From: "ruiquan.crq" <caihaoning83@gmail.com>

Signed-off-by: ruiquan.crq <caihaoning83@gmail.com>
---
 libavformat/tests/url.c |  1 +
 libavformat/url.c       | 10 +++++++---
 tests/ref/fate/url      |  4 ++++
 3 files changed, 12 insertions(+), 3 deletions(-)

Comments

Marton Balint Oct. 15, 2020, 7:15 p.m. UTC | #1
On Thu, 15 Oct 2020, caihaoning83@gmail.com wrote:

> From: "ruiquan.crq" <caihaoning83@gmail.com>
>
> Signed-off-by: ruiquan.crq <caihaoning83@gmail.com>
> ---
> libavformat/tests/url.c |  1 +
> libavformat/url.c       | 10 +++++++---
> tests/ref/fate/url      |  4 ++++
> 3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
> index 2440ae08bc..c294795fa2 100644
> --- a/libavformat/tests/url.c
> +++ b/libavformat/tests/url.c
> @@ -90,6 +90,7 @@ int main(void)
>     test_decompose("http://[::1]/dev/null");
>     test_decompose("http://[::1]:8080/dev/null");
>     test_decompose("//ffmpeg/dev/null");
> +    test_decompose("test?url=http://server/path");
>
>     printf("Testing ff_make_absolute_url:\n");
>     test(NULL, "baz");
> diff --git a/libavformat/url.c b/libavformat/url.c
> index 3c858f0257..2f7aa37e78 100644
> --- a/libavformat/url.c
> +++ b/libavformat/url.c
> @@ -88,7 +88,7 @@ static const char *find_delim(const char *delim, const char *cur, const char *en
> 
> int ff_url_decompose(URLComponents *uc, const char *url, const char *end)
> {
> -    const char *cur, *aend, *p;
> +    const char *cur, *aend, *p, *tq;
>
>     av_assert0(url);
>     if (!end)
> @@ -98,8 +98,12 @@ int ff_url_decompose(URLComponents *uc, const char *url, const char *end)
>     /* scheme */
>     uc->scheme = cur;
>     p = find_delim(":/", cur, end); /* lavf "schemes" can contain options */

Why not simply add ? and # to the list of delimiters instead?

Nevertheless that would disallow ? and # in lavf specific scheme options. 
Is it an acceptable tradeoff?

Thanks,
Marton

> -    if (*p == ':')
> -        cur = p + 1;
> +    if (*p == ':') {
> +        tq = find_delim("?", cur, end);
> +        if (*tq != '?' || p < tq) { /* not contain "?", or ":/" before "?" */
> +            cur = p + 1;
> +        }
> +    }
>
>     /* authority */
>     uc->authority = cur;
> diff --git a/tests/ref/fate/url b/tests/ref/fate/url
> index 7e6395c47b..a9db0251f1 100644
> --- a/tests/ref/fate/url
> +++ b/tests/ref/fate/url
> @@ -43,6 +43,10 @@ http://[::1]:8080/dev/null =>
>   host: ffmpeg
>   path: /dev/null
> 
> +test?url=http://server/path =>
> +  path: test
> +  query: ?url=http://server/path
> +
> Testing ff_make_absolute_url:
>                                             (null) baz                  => baz
>                                           /foo/bar baz                  => /foo/baz
> -- 
> 2.24.1 (Apple Git-126)
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
蔡昊凝 Oct. 16, 2020, 3:24 a.m. UTC | #2
Thanks for your reply.

Scheme can't contain ?.

RFC3986 definition of Scheme (https://tools.ietf.org/html/rfc3986#section-3.1)

scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )

Delimiters is lavf "schemes" can contain options, and ? should not be
part of schemes. So it is not suitable to add ? to the list of
delimiters,

although it can reduce one search.

Thans

Alex Cai


Marton Balint <cus@passwd.hu> 于2020年10月16日周五 上午3:15写道:

>
>
> On Thu, 15 Oct 2020, caihaoning83@gmail.com wrote:
>
> > From: "ruiquan.crq" <caihaoning83@gmail.com>
> >
> > Signed-off-by: ruiquan.crq <caihaoning83@gmail.com>
> > ---
> > libavformat/tests/url.c |  1 +
> > libavformat/url.c       | 10 +++++++---
> > tests/ref/fate/url      |  4 ++++
> > 3 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
> > index 2440ae08bc..c294795fa2 100644
> > --- a/libavformat/tests/url.c
> > +++ b/libavformat/tests/url.c
> > @@ -90,6 +90,7 @@ int main(void)
> >     test_decompose("http://[::1]/dev/null");
> >     test_decompose("http://[::1]:8080/dev/null");
> >     test_decompose("//ffmpeg/dev/null");
> > +    test_decompose("test?url=http://server/path");
> >
> >     printf("Testing ff_make_absolute_url:\n");
> >     test(NULL, "baz");
> > diff --git a/libavformat/url.c b/libavformat/url.c
> > index 3c858f0257..2f7aa37e78 100644
> > --- a/libavformat/url.c
> > +++ b/libavformat/url.c
> > @@ -88,7 +88,7 @@ static const char *find_delim(const char *delim, const
> char *cur, const char *en
> >
> > int ff_url_decompose(URLComponents *uc, const char *url, const char *end)
> > {
> > -    const char *cur, *aend, *p;
> > +    const char *cur, *aend, *p, *tq;
> >
> >     av_assert0(url);
> >     if (!end)
> > @@ -98,8 +98,12 @@ int ff_url_decompose(URLComponents *uc, const char
> *url, const char *end)
> >     /* scheme */
> >     uc->scheme = cur;
> >     p = find_delim(":/", cur, end); /* lavf "schemes" can contain
> options */
>
> Why not simply add ? and # to the list of delimiters instead?
>
> Nevertheless that would disallow ? and # in lavf specific scheme options.
> Is it an acceptable tradeoff?
>
> Thanks,
> Marton
>
> > -    if (*p == ':')
> > -        cur = p + 1;
> > +    if (*p == ':') {
> > +        tq = find_delim("?", cur, end);
> > +        if (*tq != '?' || p < tq) { /* not contain "?", or ":/" before
> "?" */
> > +            cur = p + 1;
> > +        }
> > +    }
> >
> >     /* authority */
> >     uc->authority = cur;
> > diff --git a/tests/ref/fate/url b/tests/ref/fate/url
> > index 7e6395c47b..a9db0251f1 100644
> > --- a/tests/ref/fate/url
> > +++ b/tests/ref/fate/url
> > @@ -43,6 +43,10 @@ http://[::1]:8080/dev/null =>
> >   host: ffmpeg
> >   path: /dev/null
> >
> > +test?url=http://server/path =>
> > +  path: test
> > +  query: ?url=http://server/path
> > +
> > Testing ff_make_absolute_url:
> >                                             (null) baz
> => baz
> >                                           /foo/bar baz
> => /foo/baz
> > --
> > 2.24.1 (Apple Git-126)
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
蔡昊凝 Oct. 16, 2020, 6:48 a.m. UTC | #3
Simply add ? to the list of delimiters,and add comment.

Signed-off-by: ruiquan.crq <caihaoning83@gmail.com>
---
    libavformat/tests/url.c | 1 +
    libavformat/url.c       | 2 +-
    tests/ref/fate/url      | 4 ++++
    3 files changed, 6 insertions(+), 1 deletion(-)

   diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
   index 2440ae08bc..c294795fa2 100644
   --- a/libavformat/tests/url.c
   +++ b/libavformat/tests/url.c
   @@ -90,6 +90,7 @@ int main(void)
        test_decompose("http://[::1]/dev/null");
        test_decompose("http://[::1]:8080/dev/null");
        test_decompose("//ffmpeg/dev/null");
   +    test_decompose("test?url=http://server/path");

        printf("Testing ff_make_absolute_url:\n");
        test(NULL, "baz");
   diff --git a/libavformat/url.c b/libavformat/url.c
   index 3c858f0257..da5950723e 100644
   --- a/libavformat/url.c
   +++ b/libavformat/url.c
   @@ -97,7 +97,7 @@ int ff_url_decompose(URLComponents *uc, const
char *url, const char *end)

        /* scheme */
        uc->scheme = cur;
   -    p = find_delim(":/", cur, end); /* lavf "schemes" can contain options */
   +    p = find_delim(":/?", cur, end); /* lavf "schemes" can contain
options, or "schemes" can't contains characters['?']*/
        if (*p == ':')
            cur = p + 1;

   diff --git a/tests/ref/fate/url b/tests/ref/fate/url
   index 7e6395c47b..a9db0251f1 100644
   --- a/tests/ref/fate/url
   +++ b/tests/ref/fate/url
   @@ -43,6 +43,10 @@ http://[::1]:8080/dev/null =>
      host: ffmpeg
      path: /dev/null

   +test?url=http://server/path =>
   +  path: test
   +  query: ?url=http://server/path
   +
    Testing ff_make_absolute_url:
                                                (null) baz
     => baz
                                              /foo/bar baz
     => /foo/baz
   --
2.24.1 (Apple Git-126)


蔡昊凝 <caihaoning83@gmail.com> 于2020年10月16日周五 上午11:24写道:

> Thanks for your reply.
>
> Scheme can't contain ?.
>
> RFC3986 definition of Scheme (https://tools.ietf.org/html/rfc3986#section-3.1)
>
> scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
>
> Delimiters is lavf "schemes" can contain options, and ? should not be part of schemes. So it is not suitable to add ? to the list of delimiters,
>
> although it can reduce one search.
>
> Thans
>
> Alex Cai
>
>
> Marton Balint <cus@passwd.hu> 于2020年10月16日周五 上午3:15写道:
>
>>
>>
>> On Thu, 15 Oct 2020, caihaoning83@gmail.com wrote:
>>
>> > From: "ruiquan.crq" <caihaoning83@gmail.com>
>> >
>> > Signed-off-by: ruiquan.crq <caihaoning83@gmail.com>
>> > ---
>> > libavformat/tests/url.c |  1 +
>> > libavformat/url.c       | 10 +++++++---
>> > tests/ref/fate/url      |  4 ++++
>> > 3 files changed, 12 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
>> > index 2440ae08bc..c294795fa2 100644
>> > --- a/libavformat/tests/url.c
>> > +++ b/libavformat/tests/url.c
>> > @@ -90,6 +90,7 @@ int main(void)
>> >     test_decompose("http://[::1]/dev/null");
>> >     test_decompose("http://[::1]:8080/dev/null");
>> >     test_decompose("//ffmpeg/dev/null");
>> > +    test_decompose("test?url=http://server/path");
>> >
>> >     printf("Testing ff_make_absolute_url:\n");
>> >     test(NULL, "baz");
>> > diff --git a/libavformat/url.c b/libavformat/url.c
>> > index 3c858f0257..2f7aa37e78 100644
>> > --- a/libavformat/url.c
>> > +++ b/libavformat/url.c
>> > @@ -88,7 +88,7 @@ static const char *find_delim(const char *delim,
>> const char *cur, const char *en
>> >
>> > int ff_url_decompose(URLComponents *uc, const char *url, const char
>> *end)
>> > {
>> > -    const char *cur, *aend, *p;
>> > +    const char *cur, *aend, *p, *tq;
>> >
>> >     av_assert0(url);
>> >     if (!end)
>> > @@ -98,8 +98,12 @@ int ff_url_decompose(URLComponents *uc, const char
>> *url, const char *end)
>> >     /* scheme */
>> >     uc->scheme = cur;
>> >     p = find_delim(":/", cur, end); /* lavf "schemes" can contain
>> options */
>>
>> Why not simply add ? and # to the list of delimiters instead?
>>
>> Nevertheless that would disallow ? and # in lavf specific scheme options.
>> Is it an acceptable tradeoff?
>>
>> Thanks,
>> Marton
>>
>> > -    if (*p == ':')
>> > -        cur = p + 1;
>> > +    if (*p == ':') {
>> > +        tq = find_delim("?", cur, end);
>> > +        if (*tq != '?' || p < tq) { /* not contain "?", or ":/" before
>> "?" */
>> > +            cur = p + 1;
>> > +        }
>> > +    }
>> >
>> >     /* authority */
>> >     uc->authority = cur;
>> > diff --git a/tests/ref/fate/url b/tests/ref/fate/url
>> > index 7e6395c47b..a9db0251f1 100644
>> > --- a/tests/ref/fate/url
>> > +++ b/tests/ref/fate/url
>> > @@ -43,6 +43,10 @@ http://[::1]:8080/dev/null =>
>> >   host: ffmpeg
>> >   path: /dev/null
>> >
>> > +test?url=http://server/path =>
>> > +  path: test
>> > +  query: ?url=http://server/path
>> > +
>> > Testing ff_make_absolute_url:
>> >                                             (null) baz
>> => baz
>> >                                           /foo/bar baz
>> => /foo/baz
>> > --
>> > 2.24.1 (Apple Git-126)
>> >
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>> > To unsubscribe, visit link above, or email
>> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
>
Nicolas George Oct. 16, 2020, 8:13 a.m. UTC | #4
蔡昊凝 (12020-10-16):
> Scheme can't contain ?.

Scheme in standard URLs cannot contain ?, but these are not standard
URLs, and the protocol part can contain ?.

> Marton Balint <cus@passwd.hu> 于2020年10月16日周五 上午3:15写道:

Please remember that top-posting is not allowed here; if you don't know
what it means look it up.

Regards,
Nicolas George Oct. 16, 2020, 8:13 a.m. UTC | #5
Marton Balint (12020-10-15):
> Why not simply add ? and # to the list of delimiters instead?
> 
> Nevertheless that would disallow ? and # in lavf specific scheme options. Is
> it an acceptable tradeoff?

I think that would be an acceptable constraint, and I think it is a more
correct fix indeed.

Regards,
蔡昊凝 Oct. 16, 2020, 10:35 a.m. UTC | #6
Although not a standard URL, is it necessary that protocol can contain "?"

Regards,

Nicolas George <george@nsup.org> 于2020年10月16日周五 下午4:13写道:

> 蔡昊凝 (12020-10-16):
> > Scheme can't contain ?.
>
> Scheme in standard URLs cannot contain ?, but these are not standard
> URLs, and the protocol part can contain ?.
>
> > Marton Balint <cus@passwd.hu> 于2020年10月16日周五 上午3:15写道:
>
> Please remember that top-posting is not allowed here; if you don't know
> what it means look it up.
>
> Regards,
>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Nicolas George Oct. 16, 2020, 10:55 a.m. UTC | #7
蔡昊凝 (12020-10-16):
> Although not a standard URL, is it necessary that protocol can contain "?"
> 
> Regards,
> 
> Nicolas George <george@nsup.org> 于2020年10月16日周五 下午4:13写道:

Please remember that top-posting is not allowed here; if you don't know
what it means look it up.
蔡昊凝 Oct. 16, 2020, 12:21 p.m. UTC | #8
Excuse me, what does this mean for your last reply. I submitted the patch
for the first time.

Regards,

Nicolas George <george@nsup.org> 于2020年10月16日周五 下午6:55写道:

> 蔡昊凝 (12020-10-16):
> > Although not a standard URL, is it necessary that protocol can contain
> "?"
> >
> > Regards,
> >
> > Nicolas George <george@nsup.org> 于2020年10月16日周五 下午4:13写道:
>
> Please remember that top-posting is not allowed here; if you don't know
> what it means look it up.
>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
index 2440ae08bc..c294795fa2 100644
--- a/libavformat/tests/url.c
+++ b/libavformat/tests/url.c
@@ -90,6 +90,7 @@  int main(void)
     test_decompose("http://[::1]/dev/null");
     test_decompose("http://[::1]:8080/dev/null");
     test_decompose("//ffmpeg/dev/null");
+    test_decompose("test?url=http://server/path");
 
     printf("Testing ff_make_absolute_url:\n");
     test(NULL, "baz");
diff --git a/libavformat/url.c b/libavformat/url.c
index 3c858f0257..2f7aa37e78 100644
--- a/libavformat/url.c
+++ b/libavformat/url.c
@@ -88,7 +88,7 @@  static const char *find_delim(const char *delim, const char *cur, const char *en
 
 int ff_url_decompose(URLComponents *uc, const char *url, const char *end)
 {
-    const char *cur, *aend, *p;
+    const char *cur, *aend, *p, *tq;
 
     av_assert0(url);
     if (!end)
@@ -98,8 +98,12 @@  int ff_url_decompose(URLComponents *uc, const char *url, const char *end)
     /* scheme */
     uc->scheme = cur;
     p = find_delim(":/", cur, end); /* lavf "schemes" can contain options */
-    if (*p == ':')
-        cur = p + 1;
+    if (*p == ':') {
+        tq = find_delim("?", cur, end);
+        if (*tq != '?' || p < tq) { /* not contain "?", or ":/" before "?" */
+            cur = p + 1;
+        }
+    }
 
     /* authority */
     uc->authority = cur;
diff --git a/tests/ref/fate/url b/tests/ref/fate/url
index 7e6395c47b..a9db0251f1 100644
--- a/tests/ref/fate/url
+++ b/tests/ref/fate/url
@@ -43,6 +43,10 @@  http://[::1]:8080/dev/null =>
   host: ffmpeg
   path: /dev/null
 
+test?url=http://server/path =>
+  path: test
+  query: ?url=http://server/path
+
 Testing ff_make_absolute_url:
                                             (null) baz                  => baz
                                           /foo/bar baz                  => /foo/baz