[FFmpeg-devel] [PATCH 1/3] avformat/url: fix logic for removing ".." path components
Steven Liu
lingjiujianke at gmail.com
Mon Jul 27 15:05:18 EEST 2020
Zlomek, Josef <josef at 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 <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';
>> > + }
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 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