| Message ID | 20200727105629.5510-1-josef@pex.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat/url: fix logic for removing ".." path components | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Josef Zlomek <josef@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@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@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
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@gmail.com> wrote: > Josef Zlomek <josef@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@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@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@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 >
I also noticed that the 3rd part did not make it to patchwork. Maybe it is because it modifies the same file. I will squash it with the first part. Josef On Mon, Jul 27, 2020 at 1:36 PM Steven Liu <lingjiujianke@gmail.com> wrote: > Josef Zlomek <josef@pex.com> 于2020年7月27日周一 下午6:57写道： > > > > Signed-off-by: Josef Zlomek <josef@pex.com> > > --- > > libavformat/url.c | 13 +++++-------- > > 1 file changed, 5 insertions(+), 8 deletions(-) > > > > diff --git a/libavformat/url.c b/libavformat/url.c > > index ccaa28a1ed..28d12fd3de 100644 > > --- a/libavformat/url.c > > +++ b/libavformat/url.c > > @@ -186,14 +186,11 @@ void ff_make_absolute_url(char *buf, int size, > const char *base, > > > > root = p = buf; > > /* Get the path root of the url which start by "://" */ > > - if (p && strstr(p, "://")) { > > - sep = strstr(p, "://"); > > - if (sep) { > > - sep += 3; > > - root = strchr(sep, '/'); > > - if (!root) > > - return; > > - } > > + if (p && (sep = strstr(p, "://"))) { > > + sep += 3; > > + root = strchr(sep, '/'); > > + if (!root) > > + return; > > } > > > > /* Remove the file name from the base url */ > > -- > > 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-request@ffmpeg.org with subject "unsubscribe". > > this patch cannot get from: > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1869 > > > Thanks > Steven >
Zlomek, Josef <josef@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@gmail.com> wrote: >> >> Josef Zlomek <josef@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@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-request@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
If your code is correct then good. I also resubmit improved version. Josef On Mon, Jul 27, 2020 at 2:05 PM Steven Liu <lingjiujianke@gmail.com> wrote: > Zlomek, Josef <josef@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@gmail.com> > wrote: > >> > >> Josef Zlomek <josef@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@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-request@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 >
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);
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@pex.com> --- libavformat/url.c | 70 +++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 39 deletions(-)