[FFmpeg-devel] [PATCH 1/3] avformat/url: fix logic for removing ".." path components

Zlomek, Josef josef at pex.com
Mon Jul 27 14:45:20 EEST 2020


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 <lingjiujianke at gmail.com> wrote:

> Josef Zlomek <josef at 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 <josef at 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';
> > +        }
> >
> > -    /* 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 at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at 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


More information about the ffmpeg-devel mailing list