| 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 :/ | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | warning | Make failed |
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".
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".
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". > >
蔡昊凝 (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,
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,
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".
蔡昊凝 (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.
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 --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