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

2020-07-29 Thread Nicolas George
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,

-- 
  Nicolas George
>From d6c429e879ffe3cdce0af6d22854dbbcce6c8222 Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Thu, 30 Jul 2020 00:02:10 +0200
Subject: [PATCH] WIP lavf/url: rewrite ff_make_absolute_url().

Signed-off-by: Nicolas George 
---
 libavformat/url.c | 223 +++---
 libavformat/url.h |   4 +-
 2 files changed, 96 insertions(+), 131 deletions(-)

diff --git a/libavformat/url.c b/libavformat/url.c
index 26aaab4019..fa265e90ea 100644
--- a/libavformat/url.c
+++ b/libavformat/url.c
@@ -27,6 +27,7 @@
 #if CONFIG_NETWORK
 #include "network.h"
 #endif
+#include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
 
 /**
@@ -152,146 +153,110 @@ int ff_url_decompose(URLComponents *uc, const char *url, const char *end)
 return 0;
 }
 
-static void trim_double_dot_url(char *buf, const char *rel, int size)
+static int append_path(char *root, char *out_end, char **rout,
+   const char *in, const char *in_end)
 {
-const char *p = rel;
-const char *root = rel;
-char tmp_path[MAX_URL_SIZE] = {0, };
-char *sep;
-char *node;
-
-/* Get the path root of the url which start by "://" */
-if (p && (sep = strstr(p, "://"))) {
-sep += 3;
-root = strchr(sep, '/');
-if (!root)
-return;
-}
-
-/* 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';
+char *out = *rout;
+const char *d, *next;
+
+if (in < in_end && *in == '/')
+in++; /* already taken care of */
+while (in < in_end) {
+d = find_delim("/", in, in_end);
+next = d + (d < in_end && *d == '/');
+if (d - in == 1 && in[0] == '.') {
+/* skip */
+} else if (out > root && /* "../" at the very beginning really means "../" */
+   d - in == 2 && in[0] == '.' && in[1] == '.') {
+av_assert1(out[-1] == '/');
+if (out - root > 1)
+while (out > root && (--out)[-1] != '/');
+} else {
+if (out_end - out < next - in)
+return AVERROR(ENOMEM);
+memcpy(out, in, next - in);
+out += next - in;
+}
+in = 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);
+*rout = out;
+return 0;
 }
 
-void ff_make_absolute_url(char *buf, int size, const char *base,
+int ff_make_absolute_url(char *buf, int size, const char *base,
   const char *rel)
 {
-char *sep, *path_query;
-char *root, *p;
-char tmp_path[MAX_URL_SIZE];
-
-memset(tmp_path, 0, sizeof(tmp_path));
-/* Absolute path, relative to the current server */
-if (base && strstr(base, "://") && rel[0] == '/') {
-if (base != buf)
-av_strlcpy(buf, base, size);
-sep = strstr(buf, "://");
-if (sep) {
-/* Take scheme from base url */
-if (rel[1] == '/') {
-sep[1] = '\0';
-} else {
-/* Take scheme and host from base url */
-sep += 3;
-sep = strchr(sep, '/');
-if (sep)
-*sep = '\0';
-}
+URLComponents ub, uc;
+char *out, *out_end, *path;
+const char *keep;
+int ret;
+
+//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 *p

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

2020-07-29 Thread Marton Balint



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_userinfohost
+#define url_component_end_hostport
+#define url_component_end_portpath
+#define url_component_end_pathquery
+#define url_component_end_query   fragment
+#define url_component_end_fragmentend


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
___
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".

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

2020-07-29 Thread Nicolas George
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,

-- 
  Nicolas George
From 6e874f86d3ab3c8b69a12d118cce0a10333848dc Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Wed, 29 Jul 2020 14:39:20 +0200
Subject: [PATCH] lavf/url: add ff_url_decompose().

Signed-off-by: Nicolas George 
---
 libavformat/tests/url.c | 33 
 libavformat/url.c   | 69 +
 libavformat/url.h   | 37 ++
 tests/ref/fate/url  | 45 +++
 4 files changed, 184 insertions(+)

diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
index 1d961a1b43..1b69b4cea8 100644
--- a/libavformat/tests/url.c
+++ b/libavformat/tests/url.c
@@ -21,6 +21,31 @@
 #include "libavformat/url.h"
 #include "libavformat/avformat.h"
 
+static void test_decompose(const char *url)
+{
+URLComponents uc;
+int len, ret;
+
+printf("%s =>\n", url);
+ret = ff_url_decompose(&uc, url, NULL);
+if (ret < 0) {
+printf("  error: %s\n", av_err2str(ret));
+return;
+}
+#define PRINT_COMPONENT(comp) \
+len = uc.url_component_end_##comp - uc.comp; \
+if (len) printf("  "#comp": %.*s\n", len, uc.comp);
+PRINT_COMPONENT(scheme);
+PRINT_COMPONENT(authority);
+PRINT_COMPONENT(userinfo);
+PRINT_COMPONENT(host);
+PRINT_COMPONENT(port);
+PRINT_COMPONENT(path);
+PRINT_COMPONENT(query);
+PRINT_COMPONENT(fragment);
+printf("\n");
+}
+
 static void test(const char *base, const char *rel)
 {
 char buf[200], buf2[200];
@@ -51,6 +76,14 @@ static void test2(const char *url)
 
 int main(void)
 {
+printf("Testing ff_url_decompose:\n\n");
+test_decompose("http://user:pass@ffmpeg:8080/dir/file?query#fragment";);
+test_decompose("http://ffmpeg/dir/file";);
+test_decompose("file:///dev/null");
+test_decompose("file:/dev/null");
+test_decompose("http://[::1]/dev/null";);
+test_decompose("http://[::1]:8080/dev/null";);
+test_decompose("//ffmpeg/dev/null");
 printf("Testing ff_make_absolute_url:\n");
 test(NULL, "baz");
 test("/foo/bar", "baz");
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);
+
+/* 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,
 

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

2020-07-29 Thread Zlomek, Josef
On Wed, Jul 29, 2020 at 12:01 PM Nicolas George  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
___
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".

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

2020-07-29 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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".

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

2020-07-28 Thread Zlomek, Josef
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  wrote:

> Zlomek, Josef  于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 
> wrote:
> >
> > > Josef Zlomek  于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 
> > > > ---
> > > >  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,
> > 

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

2020-07-28 Thread Steven Liu
Zlomek, Josef  于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  wrote:
>
> > Josef Zlomek  于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 
> > > ---
> > >  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, '/');
> > > +   

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

2020-07-28 Thread Zlomek, Josef
see version 3

On Tue, Jul 28, 2020 at 12:37 PM Steven Liu  wrote:

> Josef Zlomek  于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 
> > ---
> >  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 a

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

2020-07-28 Thread Steven Liu
Josef Zlomek  于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 
> ---
>  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, "/

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

2020-07-28 Thread Zlomek, Josef
You are right, it is a typo.

On Tue, Jul 28, 2020 at 12:02 PM Zane van Iperen 
wrote:

> On Tue, 28 Jul 2020 06:10:57 +0200
> "Josef Zlomek"  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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-07-28 Thread Zane van Iperen
On Tue, 28 Jul 2020 06:10:57 +0200
"Josef Zlomek"  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-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-07-27 Thread Josef Zlomek
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 
---
 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);
 memset(buf, 0, size);
-- 
2.17.1

__