| Message ID | 20200728041058.14557-1-josef@pex.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v3,1/2] 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 |
On Tue, 28 Jul 2020 06:10:57 +0200 "Josef Zlomek" <josef@pex.com> wrote: > > Now, the function trim_double_dot_url splits the path by "/" into > components, and processes the components one ny one: Typo: ny should be by, I'm assuming?
You are right, it is a typo. On Tue, Jul 28, 2020 at 12:02 PM Zane van Iperen <zane@zanevaniperen.com> wrote: > On Tue, 28 Jul 2020 06:10:57 +0200 > "Josef Zlomek" <josef@pex.com> wrote: > > > > > Now, the function trim_double_dot_url splits the path by "/" into > > components, and processes the components one ny one: > > Typo: ny should be by, I'm assuming? > > _______________________________________________ > 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".
Josef Zlomek <josef@pex.com> 于2020年7月28日周二 下午12:11写道： > > 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 output buffer is removed > - if the component is not empty, it is added to the output buffer > No temporary buffers are used anymore. > > Also the path containing no '/' after '://' is returned as it is. > > The duplicate logic was removed from ff_make_absolute_url. > > Signed-off-by: Josef Zlomek <josef@pex.com> > --- > libavformat/url.c | 101 +++++++++++++++++++++++----------------------- > 1 file changed, 50 insertions(+), 51 deletions(-) > > diff --git a/libavformat/url.c b/libavformat/url.c > index 20463a6674..46a08e88e3 100644 > --- a/libavformat/url.c > +++ b/libavformat/url.c > @@ -82,41 +82,62 @@ 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; > + char *dst; > + int dst_len = 0; > + const char *sep; > + const char *next; > + int last_is_dir; > > /* Get the path root of the url which start by "://" */ > if (p && (sep = strstr(p, "://"))) { > sep += 3; > root = strchr(sep, '/'); > - if (!root) > + if (!root) { > + av_strlcpy(buf, rel, size); > return; > + } > } > + if (*root == '/') > + ++root; > > - /* 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'; > - } > - > - 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. */ > + /* Copy the root to the output buffer */ > av_strlcpy(buf, rel, root - rel + 1); > + size -= root - rel; > + dst = &buf[root - rel]; > + > + /* Split the path by "/" and remove ".." and its corresponding directory. */ > + last_is_dir = 1; > + for (p = root; *p; p = next) { > + next = strchr(p, '/'); > + if (!next) { > + next = p + strlen(p); > + last_is_dir = 0; > + } > > - if (!av_stristart(tmp_path, "/", NULL) && root != rel) > - av_strlcat(buf, "/", size); > + if (next - p == 2 && !strncmp(p, "..", 2)) { > + /* remove the last directory from dst */ > + while (dst_len > 0 && dst[--dst_len] != '/') > + ; > + dst[dst_len] = '\0'; > + last_is_dir = 1; > + } else if (next > p) { > + int len_including_zero; > + /* copy the current path component to dst (including '/') */ > + if (dst_len) { > + av_strlcpy(dst + dst_len, "/", size - dst_len); > + ++dst_len; > + } > + len_including_zero = FFMIN(size - dst_len, next - p + 1); > + av_strlcpy(dst + dst_len, p, len_including_zero); > + dst_len += len_including_zero - 1; > + } > > - av_strlcat(buf, tmp_path, size); > + /* skip "/" */ > + while (*next == '/') > + ++next; > + } > + if (last_is_dir && dst_len) > + av_strlcpy(dst + dst_len, "/", size - dst_len); > } > > void ff_make_absolute_url(char *buf, int size, const char *base, > @@ -175,14 +196,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 */ > @@ -194,26 +212,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); As Marton said, tmp_path should remove too. > 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".
see version 3 On Tue, Jul 28, 2020 at 12:37 PM Steven Liu <lingjiujianke@gmail.com> wrote: > Josef Zlomek <josef@pex.com> 于2020年7月28日周二 下午12:11写道： > > > > 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 output buffer is > removed > > - if the component is not empty, it is added to the output buffer > > No temporary buffers are used anymore. > > > > Also the path containing no '/' after '://' is returned as it is. > > > > The duplicate logic was removed from ff_make_absolute_url. > > > > Signed-off-by: Josef Zlomek <josef@pex.com> > > --- > > libavformat/url.c | 101 +++++++++++++++++++++++----------------------- > > 1 file changed, 50 insertions(+), 51 deletions(-) > > > > diff --git a/libavformat/url.c b/libavformat/url.c > > index 20463a6674..46a08e88e3 100644 > > --- a/libavformat/url.c > > +++ b/libavformat/url.c > > @@ -82,41 +82,62 @@ 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; > > + char *dst; > > + int dst_len = 0; > > + const char *sep; > > + const char *next; > > + int last_is_dir; > > > > /* Get the path root of the url which start by "://" */ > > if (p && (sep = strstr(p, "://"))) { > > sep += 3; > > root = strchr(sep, '/'); > > - if (!root) > > + if (!root) { > > + av_strlcpy(buf, rel, size); > > return; > > + } > > } > > + if (*root == '/') > > + ++root; > > > > - /* 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'; > > - } > > - > > - 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. */ > > + /* Copy the root to the output buffer */ > > av_strlcpy(buf, rel, root - rel + 1); > > + size -= root - rel; > > + dst = &buf[root - rel]; > > + > > + /* Split the path by "/" and remove ".." and its corresponding > directory. */ > > + last_is_dir = 1; > > + for (p = root; *p; p = next) { > > + next = strchr(p, '/'); > > + if (!next) { > > + next = p + strlen(p); > > + last_is_dir = 0; > > + } > > > > - if (!av_stristart(tmp_path, "/", NULL) && root != rel) > > - av_strlcat(buf, "/", size); > > + if (next - p == 2 && !strncmp(p, "..", 2)) { > > + /* remove the last directory from dst */ > > + while (dst_len > 0 && dst[--dst_len] != '/') > > + ; > > + dst[dst_len] = '\0'; > > + last_is_dir = 1; > > + } else if (next > p) { > > + int len_including_zero; > > + /* copy the current path component to dst (including '/') */ > > + if (dst_len) { > > + av_strlcpy(dst + dst_len, "/", size - dst_len); > > + ++dst_len; > > + } > > + len_including_zero = FFMIN(size - dst_len, next - p + 1); > > + av_strlcpy(dst + dst_len, p, len_including_zero); > > + dst_len += len_including_zero - 1; > > + } > > > > - av_strlcat(buf, tmp_path, size); > > + /* skip "/" */ > > + while (*next == '/') > > + ++next; > > + } > > + if (last_is_dir && dst_len) > > + av_strlcpy(dst + dst_len, "/", size - dst_len); > > } > > > > void ff_make_absolute_url(char *buf, int size, const char *base, > > @@ -175,14 +196,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 */ > > @@ -194,26 +212,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); > As Marton said, tmp_path should remove too. > > > 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". > _______________________________________________ > 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".
Zlomek, Josef <josef@pex.com> 于2020年7月28日周二 下午6:58写道： > > see version 3 Isn't this version? or maybe your mean is version 4? > > On Tue, Jul 28, 2020 at 12:37 PM Steven Liu <lingjiujianke@gmail.com> wrote: > > > Josef Zlomek <josef@pex.com> 于2020年7月28日周二 下午12:11写道： > > > > > > 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 output buffer is > > removed > > > - if the component is not empty, it is added to the output buffer > > > No temporary buffers are used anymore. > > > > > > Also the path containing no '/' after '://' is returned as it is. > > > > > > The duplicate logic was removed from ff_make_absolute_url. > > > > > > Signed-off-by: Josef Zlomek <josef@pex.com> > > > --- > > > libavformat/url.c | 101 +++++++++++++++++++++++----------------------- > > > 1 file changed, 50 insertions(+), 51 deletions(-) > > > > > > diff --git a/libavformat/url.c b/libavformat/url.c > > > index 20463a6674..46a08e88e3 100644 > > > --- a/libavformat/url.c > > > +++ b/libavformat/url.c > > > @@ -82,41 +82,62 @@ 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; > > > + char *dst; > > > + int dst_len = 0; > > > + const char *sep; > > > + const char *next; > > > + int last_is_dir; > > > > > > /* Get the path root of the url which start by "://" */ > > > if (p && (sep = strstr(p, "://"))) { > > > sep += 3; > > > root = strchr(sep, '/'); > > > - if (!root) > > > + if (!root) { > > > + av_strlcpy(buf, rel, size); > > > return; > > > + } > > > } > > > + if (*root == '/') > > > + ++root; > > > > > > - /* 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'; > > > - } > > > - > > > - 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. */ > > > + /* Copy the root to the output buffer */ > > > av_strlcpy(buf, rel, root - rel + 1); > > > + size -= root - rel; > > > + dst = &buf[root - rel]; > > > + > > > + /* Split the path by "/" and remove ".." and its corresponding > > directory. */ > > > + last_is_dir = 1; > > > + for (p = root; *p; p = next) { > > > + next = strchr(p, '/'); > > > + if (!next) { > > > + next = p + strlen(p); > > > + last_is_dir = 0; > > > + } > > > > > > - if (!av_stristart(tmp_path, "/", NULL) && root != rel) > > > - av_strlcat(buf, "/", size); > > > + if (next - p == 2 && !strncmp(p, "..", 2)) { > > > + /* remove the last directory from dst */ > > > + while (dst_len > 0 && dst[--dst_len] != '/') > > > + ; > > > + dst[dst_len] = '\0'; > > > + last_is_dir = 1; > > > + } else if (next > p) { > > > + int len_including_zero; > > > + /* copy the current path component to dst (including '/') */ > > > + if (dst_len) { > > > + av_strlcpy(dst + dst_len, "/", size - dst_len); > > > + ++dst_len; > > > + } > > > + len_including_zero = FFMIN(size - dst_len, next - p + 1); > > > + av_strlcpy(dst + dst_len, p, len_including_zero); > > > + dst_len += len_including_zero - 1; > > > + } > > > > > > - av_strlcat(buf, tmp_path, size); > > > + /* skip "/" */ > > > + while (*next == '/') > > > + ++next; > > > + } > > > + if (last_is_dir && dst_len) > > > + av_strlcpy(dst + dst_len, "/", size - dst_len); > > > } > > > > > > void ff_make_absolute_url(char *buf, int size, const char *base, > > > @@ -175,14 +196,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 */ > > > @@ -194,26 +212,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); > > As Marton said, tmp_path should remove too. > > > > > 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". > > _______________________________________________ > > 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". > > > > -- > 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-request@ffmpeg.org with subject "unsubscribe".
I have removed tmp_path from function trim_double_dot_url I did not change the function ff_make_absolute_url much, as I wanted to just fix handling of "..". Josef On Tue, Jul 28, 2020 at 12:59 PM Steven Liu <lingjiujianke@gmail.com> wrote: > Zlomek, Josef <josef@pex.com> 于2020年7月28日周二 下午6:58写道： > > > > see version 3 > Isn't this version? or maybe your mean is version 4? > > > > On Tue, Jul 28, 2020 at 12:37 PM Steven Liu <lingjiujianke@gmail.com> > wrote: > > > > > Josef Zlomek <josef@pex.com> 于2020年7月28日周二 下午12:11写道： > > > > > > > > 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 output buffer > is > > > removed > > > > - if the component is not empty, it is added to the output buffer > > > > No temporary buffers are used anymore. > > > > > > > > Also the path containing no '/' after '://' is returned as it is. > > > > > > > > The duplicate logic was removed from ff_make_absolute_url. > > > > > > > > Signed-off-by: Josef Zlomek <josef@pex.com> > > > > --- > > > > libavformat/url.c | 101 > +++++++++++++++++++++++----------------------- > > > > 1 file changed, 50 insertions(+), 51 deletions(-) > > > > > > > > diff --git a/libavformat/url.c b/libavformat/url.c > > > > index 20463a6674..46a08e88e3 100644 > > > > --- a/libavformat/url.c > > > > +++ b/libavformat/url.c > > > > @@ -82,41 +82,62 @@ 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; > > > > + char *dst; > > > > + int dst_len = 0; > > > > + const char *sep; > > > > + const char *next; > > > > + int last_is_dir; > > > > > > > > /* Get the path root of the url which start by "://" */ > > > > if (p && (sep = strstr(p, "://"))) { > > > > sep += 3; > > > > root = strchr(sep, '/'); > > > > - if (!root) > > > > + if (!root) { > > > > + av_strlcpy(buf, rel, size); > > > > return; > > > > + } > > > > } > > > > + if (*root == '/') > > > > + ++root; > > > > > > > > - /* 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'; > > > > - } > > > > - > > > > - 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. */ > > > > + /* Copy the root to the output buffer */ > > > > av_strlcpy(buf, rel, root - rel + 1); > > > > + size -= root - rel; > > > > + dst = &buf[root - rel]; > > > > + > > > > + /* Split the path by "/" and remove ".." and its corresponding > > > directory. */ > > > > + last_is_dir = 1; > > > > + for (p = root; *p; p = next) { > > > > + next = strchr(p, '/'); > > > > + if (!next) { > > > > + next = p + strlen(p); > > > > + last_is_dir = 0; > > > > + } > > > > > > > > - if (!av_stristart(tmp_path, "/", NULL) && root != rel) > > > > - av_strlcat(buf, "/", size); > > > > + if (next - p == 2 && !strncmp(p, "..", 2)) { > > > > + /* remove the last directory from dst */ > > > > + while (dst_len > 0 && dst[--dst_len] != '/') > > > > + ; > > > > + dst[dst_len] = '\0'; > > > > + last_is_dir = 1; > > > > + } else if (next > p) { > > > > + int len_including_zero; > > > > + /* copy the current path component to dst (including > '/') */ > > > > + if (dst_len) { > > > > + av_strlcpy(dst + dst_len, "/", size - dst_len); > > > > + ++dst_len; > > > > + } > > > > + len_including_zero = FFMIN(size - dst_len, next - p + > 1); > > > > + av_strlcpy(dst + dst_len, p, len_including_zero); > > > > + dst_len += len_including_zero - 1; > > > > + } > > > > > > > > - av_strlcat(buf, tmp_path, size); > > > > + /* skip "/" */ > > > > + while (*next == '/') > > > > + ++next; > > > > + } > > > > + if (last_is_dir && dst_len) > > > > + av_strlcpy(dst + dst_len, "/", size - dst_len); > > > > } > > > > > > > > void ff_make_absolute_url(char *buf, int size, const char *base, > > > > @@ -175,14 +196,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 */ > > > > @@ -194,26 +212,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); > > > As Marton said, tmp_path should remove too. > > > > > > > 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". > > > _______________________________________________ > > > 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". > > > > > > > > -- > > 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-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".
Josef Zlomek (12020-07-28): > 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 output buffer is removed > - if the component is not empty, it is added to the output buffer > No temporary buffers are used anymore. > > Also the path containing no '/' after '://' is returned as it is. > > The duplicate logic was removed from ff_make_absolute_url. I am afraid ff_make_absolute_url() as it is is broken beyond repair. For example: http://server/menu redirect?url=http://otherserver/target => redirect?url=http://otherserver/target while the result should have been http://server/redirect?url=http://otherserver/target I will have a shot at rewriting it cleanly, with proper parsing of URL components. Regards,
On Wed, Jul 29, 2020 at 12:01 PM Nicolas George <george@nsup.org> wrote: > I am afraid ff_make_absolute_url() as it is is broken beyond repair. For > example: > > http://server/menu redirect?url=http://otherserver/target => > redirect?url=http://otherserver/target > > while the result should have been > > http://server/redirect?url=http://otherserver/target > > I will have a shot at rewriting it cleanly, with proper parsing of URL > components. > I also noticed that there are many more bugs in ff_make_absolute_url() I just fixed one of them. I am looking forward for your complete fix. Best regards, Josef
Zlomek, Josef (12020-07-29): > I also noticed that there are many more bugs in ff_make_absolute_url() > I just fixed one of them. > I am looking forward for your complete fix. I think the key to a working version is to properly parse URLs first. Then we can parse the base and relative part, and build the absolute URL from the components, simplifying the path component, and only the path component, at the same time. Here is my preliminary code for parsing, in case somebody wants to look at it. Next step is to use it for rewriting ff_make_absolute_url(). Possibly, use it for av_url_split() too. Regards,
On Wed, 29 Jul 2020, Nicolas George wrote: > Zlomek, Josef (12020-07-29): >> I also noticed that there are many more bugs in ff_make_absolute_url() >> I just fixed one of them. >> I am looking forward for your complete fix. > > I think the key to a working version is to properly parse URLs first. > Then we can parse the base and relative part, and build the absolute URL > from the components, simplifying the path component, and only the path > component, at the same time. > > Here is my preliminary code for parsing, in case somebody wants to look > at it. Thanks for working on this. I agree that proper (as RFC compliant as it can be) URL parsing is needed here. Probably we should clearly document differences from RFC compliant parsing, if we cannot do it entirely RFC compliantly... > diff --git a/libavformat/url.c b/libavformat/url.c > index 20463a6674..7612a2ee6e 100644 > --- a/libavformat/url.c > +++ b/libavformat/url.c > @@ -78,6 +78,75 @@ int ff_url_join(char *str, int size, const char *proto, > return strlen(str); > } > > +static const char *find_delim(const char *delim, const char *cur, const char *end) > +{ > + while (cur < end && !strchr(delim, *cur)) > + cur++; > + return cur; > +} > + > +int ff_url_decompose(URLComponents *uc, const char *url, const char *end) > +{ > + const char *cur, *aend, *p; > + > + if (!end) > + end = url + strlen(url); > + cur = uc->url = url; > + > + /* scheme */ > + uc->scheme = cur; > + p = find_delim(":/", cur, end); /* lavf "schemes" can contain options */ > + if (*p == ':') > + cur = p + 1; > + > + /* authority */ > + uc->authority = cur; > + if (end - cur >= 2 && cur[0] == '/' && cur[1] == '/') { > + cur += 2; > + aend = find_delim("/", cur, end); ? and # can also separate authority from the rest. > + > + /* userinfo */ > + uc->userinfo = cur; > + p = find_delim("@", cur, aend); > + if (*p == '@') > + cur = p + 1; > + > + /* host */ > + uc->host = cur; > + if (*cur == '[') { /* hello IPv6, thanks for using colons! */ > + p = find_delim("]", cur, aend); > + if (*p != ']') > + return AVERROR(EINVAL); > + if (p + 1 < aend && p[1] != ':') > + return AVERROR(EINVAL); > + cur = p + 1; > + } else { > + cur = find_delim(":", cur, aend); > + } > + > + /* port */ > + uc->port = cur; > + cur = aend; > + } else { > + uc->userinfo = uc->host = uc->port = cur; > + } > + > + /* path */ > + uc->path = cur; > + cur = find_delim("?#", cur, end); > + > + /* query */ > + uc->query = cur; > + if (*cur == '?') > + cur = find_delim("#", cur, end); > + > + /* fragment */ > + uc->fragment = cur; > + > + uc->end = end; > + return 0; > +} > + > static void trim_double_dot_url(char *buf, const char *rel, int size) > { > const char *p = rel; > diff --git a/libavformat/url.h b/libavformat/url.h > index de0d30aca0..99d453b378 100644 > --- a/libavformat/url.h > +++ b/libavformat/url.h > @@ -344,4 +344,41 @@ const AVClass *ff_urlcontext_child_class_iterate(void **iter); > const URLProtocol **ffurl_get_protocols(const char *whitelist, > const char *blacklist); > > +typedef struct URLComponents { > + const char *url; /**< whole URL, for reference */ > + const char *scheme; /**< possibly including lavf-specific options */ > + const char *authority; /**< "//" if it is a real URL */ > + const char *userinfo; /**< including final '@' if present */ > + const char *host; > + const char *port; /**< including initial ':' if present */ > + const char *path; > + const char *query; /**< including initial '?' if present */ > + const char *fragment; /**< including initial '#' if present */ > + const char *end; > +} URLComponents; > + > +#define url_component_end_scheme authority > +#define url_component_end_authority userinfo > +#define url_component_end_userinfo host > +#define url_component_end_host port > +#define url_component_end_port path > +#define url_component_end_path query > +#define url_component_end_query fragment > +#define url_component_end_fragment end I am not sure about this approach, the known characters at the end or at the start will make further operations a bit harder. I'd just simply add another field for each URL component to signal the end, e.g. url url_end scheme scheme_end ... Regards, Marton
Marton Balint (12020-07-29): > Thanks for working on this. I agree that proper (as RFC compliant as it can > be) URL parsing is needed here. Probably we should clearly document > differences from RFC compliant parsing, if we cannot do it entirely RFC > compliantly... Indeed. Unfortunately, that may cause some trouble, but not for right now. > ? and # can also separate authority from the rest. Thanks, fixed. > I am not sure about this approach, the known characters at the end or at the > start will make further operations a bit harder. I'd just simply add another > field for each URL component to signal the end, e.g. I considered this, but it was not actually convenient. I have something that works, but it needs polishing and more testing. But you can have a peek. Regards,
diff --git a/libavformat/url.c b/libavformat/url.c index 20463a6674..46a08e88e3 100644 --- a/libavformat/url.c +++ b/libavformat/url.c @@ -82,41 +82,62 @@ 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; + char *dst; + int dst_len = 0; + const char *sep; + const char *next; + int last_is_dir; /* Get the path root of the url which start by "://" */ if (p && (sep = strstr(p, "://"))) { sep += 3; root = strchr(sep, '/'); - if (!root) + if (!root) { + av_strlcpy(buf, rel, size); return; + } } + if (*root == '/') + ++root; - /* 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'; - } - - 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. */ + /* Copy the root to the output buffer */ av_strlcpy(buf, rel, root - rel + 1); + size -= root - rel; + dst = &buf[root - rel]; + + /* Split the path by "/" and remove ".." and its corresponding directory. */ + last_is_dir = 1; + for (p = root; *p; p = next) { + next = strchr(p, '/'); + if (!next) { + next = p + strlen(p); + last_is_dir = 0; + } - if (!av_stristart(tmp_path, "/", NULL) && root != rel) - av_strlcat(buf, "/", size); + if (next - p == 2 && !strncmp(p, "..", 2)) { + /* remove the last directory from dst */ + while (dst_len > 0 && dst[--dst_len] != '/') + ; + dst[dst_len] = '\0'; + last_is_dir = 1; + } else if (next > p) { + int len_including_zero; + /* copy the current path component to dst (including '/') */ + if (dst_len) { + av_strlcpy(dst + dst_len, "/", size - dst_len); + ++dst_len; + } + len_including_zero = FFMIN(size - dst_len, next - p + 1); + av_strlcpy(dst + dst_len, p, len_including_zero); + dst_len += len_including_zero - 1; + } - av_strlcat(buf, tmp_path, size); + /* skip "/" */ + while (*next == '/') + ++next; + } + if (last_is_dir && dst_len) + av_strlcpy(dst + dst_len, "/", size - dst_len); } void ff_make_absolute_url(char *buf, int size, const char *base, @@ -175,14 +196,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 */ @@ -194,26 +212,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 output buffer is removed - if the component is not empty, it is added to the output buffer No temporary buffers are used anymore. Also the path containing no '/' after '://' is returned as it is. The duplicate logic was removed from ff_make_absolute_url. Signed-off-by: Josef Zlomek <josef@pex.com> --- libavformat/url.c | 101 +++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 51 deletions(-)