If your code is correct then good.
I also resubmit improved version.
Josef
On Mon, Jul 27, 2020 at 2:05 PM Steven Liu <lingjiujia...@gmail.com> wrote:
> Zlomek, Josef <jo...@pex.com> 于2020年7月27日周一 下午7:45写道：
> >
> > Hi Steven,
> >
> > ".." is always a directory.
> >
> > your tests (diff starting with -) are not correct.
> > My results (lines starting with +) are correct.
> >
> > Josef
> >
> > On Mon, Jul 27, 2020 at 1:36 PM Steven Liu <lingjiujia...@gmail.com>
> wrote:
> >>
> >> Josef Zlomek <jo...@pex.com> 于2020年7月27日周一 下午6:57写道：
> >> >
> >> > Fixes: 8814
> >> >
> >> > The logic for removing ".." path components and their corresponding
> >> > upper directories was reworked.
> >> >
> >> > Now, the function trim_double_dot_url splits the path by "/" into
> >> > components, and processes the components one ny one:
> >> > - if the component is "..", the last path component in tmp_path is
> removed
> >> > - if the component is not empty, it is added to tmp_path
> >> >
> >> > The duplicate logic was removed from ff_make_absolute_url.
> >> >
> >> > Signed-off-by: Josef Zlomek <jo...@pex.com>
> >> > ---
> >> > libavformat/url.c | 70
> +++++++++++++++++++++--------------------------
> >> > 1 file changed, 31 insertions(+), 39 deletions(-)
> >> >
> >> > diff --git a/libavformat/url.c b/libavformat/url.c
> >> > index 20463a6674..ccaa28a1ed 100644
> >> > --- a/libavformat/url.c
> >> > +++ b/libavformat/url.c
> >> > @@ -83,8 +83,9 @@ static void trim_double_dot_url(char *buf, const
> char *rel, int size)
> >> > const char *p = rel;
> >> > const char *root = rel;
> >> > char tmp_path[MAX_URL_SIZE] = {0, };
> >> > - char *sep;
> >> > - char *node;
> >> > + int tmp_len = 0;
> >> > + const char *sep;
> >> > + const char *next;
> >> >
> >> > /* Get the path root of the url which start by "://" */
> >> > if (p && (sep = strstr(p, "://"))) {
> >> > @@ -93,29 +94,39 @@ static void trim_double_dot_url(char *buf, const
> char *rel, int size)
> >> > if (!root)
> >> > return;
> >> > }
> >> > + if (*root == '/')
> >> > + ++root;
> >> > +
> >> > + /* Split the path by "/" and remove ".." and its corresponding
> directory. */
> >> > + for (p = root; *p; p = next) {
> >> > + next = strchr(p, '/');
> >> > + if (!next)
> >> > + next = p + strlen(p);
> >> > +
> >> > + if (next - p == 2 && !strncmp(p, "..", 2)) {
> >> > + /* remove the last directory from tmp_path */
> >> > + while (tmp_len > 0 && tmp_path[--tmp_len] != '/')
> >> > + ;
> >> > + tmp_path[tmp_len] = '\0';
> >> > + } else if (next > p) {
> >> > + /* copy the current path component to tmp_path
> (including '/') */
> >> > + if (tmp_len) {
> >> > + av_strlcpy(tmp_path + tmp_len, "/", sizeof(tmp_path)
> - tmp_len);
> >> > + ++tmp_len;
> >> > + }
> >> > + av_strlcpy(tmp_path + tmp_len, p, FFMIN(sizeof(tmp_path)
> - tmp_len,
> >> > + next - p + 1));
> >> > + tmp_len += next - p;
> >> > + tmp_path[tmp_len] = '\0';
> >> > + }
> And I think only one loop is ok than this way.
> I will resubmit a new patch, maybe that is simpler than yours.
> >> >
> >> > - /* set new current position if the root node is changed */
> >> > - p = root;
> >> > - while (p && (node = strstr(p, ".."))) {
> >> > - av_strlcat(tmp_path, p, node - p + strlen(tmp_path));
> >> > - p = node + 3;
> >> > - sep = strrchr(tmp_path, '/');
> >> > - if (sep)
> >> > - sep[0] = '\0';
> >> > - else
> >> > - tmp_path[0] = '\0';
> >> > + /* skip "/" */
> >> > + while (*next == '/')
> >> > + ++next;
> >> > }
> >> >
> >> > - if (!av_stristart(p, "/", NULL) && root != rel)
> >> > - av_strlcat(tmp_path, "/", size);
> >> > -
> >> > - av_strlcat(tmp_path, p, size);
> >> > /* start set buf after temp path process. */
> >> > av_strlcpy(buf, rel, root - rel + 1);
> >> > -
> >> > - if (!av_stristart(tmp_path, "/", NULL) && root != rel)
> >> > - av_strlcat(buf, "/", size);
> >> > -
> >> > av_strlcat(buf, tmp_path, size);
> >> > }
> >> >
> >> > @@ -194,26 +205,7 @@ void ff_make_absolute_url(char *buf, int size,
> const char *base,
> >> > sep[1] = '\0';
> >> > else
> >> > buf[0] = '\0';
> >> > - while (av_strstart(rel, "..", NULL) && sep) {
> >> > - /* Remove the path delimiter at the end */
> >> > - if (sep > root) {
> >> > - sep[0] = '\0';
> >> > - sep = strrchr(buf, '/');
> >> > - }
> >> >
> >> > - /* If the next directory name to pop off is "..", break here
> */
> >> > - if (!strcmp(sep ? &sep[1] : buf, "..")) {
> >> > - /* Readd the slash we just removed */
> >> > - av_strlcat(buf, "/", size);
> >> > - break;
> >> > - }
> >> > - /* Cut off the directory name */
> >> > - if (sep)
> >> > - sep[1] = '\0';
> >> > - else
> >> > - buf[0] = '\0';
> >> > - rel += 3;
> >> > - }
> >> > av_strlcat(buf, rel, size);
> >> > trim_double_dot_url(tmp_path, buf, size);
> >> > memset(buf, 0, size);
> >> > --
> >> > 2.17.1
> >> >
> >> > _______________________________________________
> >> > ffmpeg-devel mailing list
> >> > ffmpeg-devel@ffmpeg.org
> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >
> >> > To unsubscribe, visit link above, or email
> >> > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> >>
> >> maybe the last node is a file, not directory
> >>
> >> (base) liuqi05:clang liuqi$ make fate-url
> >> CC libavformat/tests/url.o
> >> LD libavformat/tests/url
> >> TEST url
> >> --- src/tests/ref/fate/url 2020-07-27 19:30:58.000000000 +0800
> >> +++ tests/data/fate/url 2020-07-27 19:34:25.000000000 +0800
> >> @@ -15,9 +15,9 @@
> >> http://server/foo/bar //other/url
> >> => http://other/url
> >> http://server/foo/bar
> >> ../../../../../other/url => http://server/other/url
> >> http://server/foo/bar
> >> a/b/../c/d/../e../..f/.../../other/url/test..mp3 =>
> >> http://server/foo/a/c/e../..f/other/url/test..mp3
> >> - http://server/foo/bar
> >> a/b/../c/d/../e../..f/.../../other/url/.. =>
> >> http://server/foo/a/c/e../..f/other/url/..
> >> - http://server/foo/bar
> >> b/../c/d/../e../..f/.../../other/url/.. =>
> >> http://server/foo/c/e../..f/other/url/..
> >> - http://server/foo/bar
> >> b/../c/d/../e../..f/.../..other/url/.. =>
> >> http://server/foo/c/e../..f/.../..other/url/..
> >> + http://server/foo/bar
> >> a/b/../c/d/../e../..f/.../../other/url/.. =>
> >> http://server/foo/a/c/e../..f/other
> >> + http://server/foo/bar
> >> b/../c/d/../e../..f/.../../other/url/.. =>
> >> http://server/foo/c/e../..f/other
> >> + http://server/foo/bar
> >> b/../c/d/../e../..f/.../..other/url/.. =>
> >> http://server/foo/c/e../..f/.../..other
> >> http://server/foo/bar
> >> a/b/../c/d/../e../..f/.../other/url/test..mp3 =>
> >> http://server/foo/a/c/e../..f/.../other/url/test..mp3
> >> http://server/foo/bar
> >> /a/b/../c/d/../e../..f/.../other/url/test..mp3 =>
> >> http://server/a/c/e../..f/.../other/url/test..mp3
> >> http://server/foo/bar
> >> /../../../../../other/url => http://server/other/url
> >> Test url failed. Look at tests/data/fate/url.err for details.
> >> make: *** [fate-url] Error 1
> >
> >
> >
> > --
> > Josef Zlomek
>
--
Josef Zlomek
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
