diff mbox series

[FFmpeg-devel] libavformat/utils.c: Fixed URL parsing

Message ID 20200212181230.24531-1-gautamramk@gmail.com
State New
Headers show
Series [FFmpeg-devel] libavformat/utils.c: Fixed URL parsing
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Gautam Ramakrishnan Feb. 12, 2020, 6:12 p.m. UTC
From: Gautam Ramakrishnan <gautamramk@gmail.com>

This commit fixes bug #8466 wherein URLs with query string
not preceeded by the '/' symbol in a URL was failing to get
parsed. The av_url_split() function now checks if the path
starts with a '/' and prepends it if it does not.
---
 libavformat/utils.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Paul B Mahol Feb. 12, 2020, 6:22 p.m. UTC | #1
On 2/12/20, gautamramk@gmail.com <gautamramk@gmail.com> wrote:
> From: Gautam Ramakrishnan <gautamramk@gmail.com>
>
> This commit fixes bug #8466 wherein URLs with query string
> not preceeded by the '/' symbol in a URL was failing to get
> parsed. The av_url_split() function now checks if the path
> starts with a '/' and prepends it if it does not.
> ---
>  libavformat/utils.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 81ea239a66..a14d4ef90d 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4819,6 +4819,13 @@ void av_url_split(char *proto, int proto_size,
>          av_strlcpy(path, ls, path_size);
>      else
>          ls = &p[strlen(p)];  // XXX
> +    /* if path does not start with a /, prepend a / */
> +    if (path[0] != '/') {
> +        char temp[1024];

What makes 1024 so special?

> +        av_strlcpy(temp, path, path_size);
> +        path[0] = '/';
> +        av_strlcpy(path + 1, temp, path_size - 1);
> +    }
>
>      /* the rest is hostname, use that to parse auth/port */
>      if (ls != p) {
> --
> 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".
Marton Balint Feb. 12, 2020, 7:01 p.m. UTC | #2
On Wed, 12 Feb 2020, gautamramk@gmail.com wrote:

> From: Gautam Ramakrishnan <gautamramk@gmail.com>
>
> This commit fixes bug #8466 wherein URLs with query string
> not preceeded by the '/' symbol in a URL was failing to get
> parsed. The av_url_split() function now checks if the path
> starts with a '/' and prepends it if it does not.
> ---
> libavformat/utils.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 81ea239a66..a14d4ef90d 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4819,6 +4819,13 @@ void av_url_split(char *proto, int proto_size,
>         av_strlcpy(path, ls, path_size);
>     else
>         ls = &p[strlen(p)];  // XXX
> +    /* if path does not start with a /, prepend a / */
> +    if (path[0] != '/') {
> +        char temp[1024];
> +        av_strlcpy(temp, path, path_size);
> +        path[0] = '/';
> +        av_strlcpy(path + 1, temp, path_size - 1);
> +    }

I made a patch which fixes this in the HTTP protocol, as modifying the 
empty path to / is a HTTP requirement, other protocols might not need 
this.

http://mplayerhq.hu/pipermail/ffmpeg-devel/2020-February/256902.html

Regards,
Marton
Gautam Ramakrishnan Feb. 13, 2020, 2:31 a.m. UTC | #3
On Thu, Feb 13, 2020 at 12:31 AM Marton Balint <cus@passwd.hu> wrote:
>
>
>
> On Wed, 12 Feb 2020, gautamramk@gmail.com wrote:
>
> > From: Gautam Ramakrishnan <gautamramk@gmail.com>
> >
> > This commit fixes bug #8466 wherein URLs with query string
> > not preceeded by the '/' symbol in a URL was failing to get
> > parsed. The av_url_split() function now checks if the path
> > starts with a '/' and prepends it if it does not.
> > ---
> > libavformat/utils.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 81ea239a66..a14d4ef90d 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -4819,6 +4819,13 @@ void av_url_split(char *proto, int proto_size,
> >         av_strlcpy(path, ls, path_size);
> >     else
> >         ls = &p[strlen(p)];  // XXX
> > +    /* if path does not start with a /, prepend a / */
> > +    if (path[0] != '/') {
> > +        char temp[1024];
> > +        av_strlcpy(temp, path, path_size);
> > +        path[0] = '/';
> > +        av_strlcpy(path + 1, temp, path_size - 1);
> > +    }
>
> I made a patch which fixes this in the HTTP protocol, as modifying the
> empty path to / is a HTTP requirement, other protocols might not need
> this.
>
Oh I just found this on patchwork. I think my patch becomes redundant
then. I guess the patch has not been merged yet?
> http://mplayerhq.hu/pipermail/ffmpeg-devel/2020-February/256902.html
>
> Regards,
> Marton
Marton Balint Feb. 15, 2020, 6:33 p.m. UTC | #4
On Thu, 13 Feb 2020, Gautam Ramakrishnan wrote:

> On Thu, Feb 13, 2020 at 12:31 AM Marton Balint <cus@passwd.hu> wrote:
>>
>>
>>
>> On Wed, 12 Feb 2020, gautamramk@gmail.com wrote:
>>
>> > From: Gautam Ramakrishnan <gautamramk@gmail.com>
>> >
>> > This commit fixes bug #8466 wherein URLs with query string
>> > not preceeded by the '/' symbol in a URL was failing to get
>> > parsed. The av_url_split() function now checks if the path
>> > starts with a '/' and prepends it if it does not.
>> > ---
>> > libavformat/utils.c | 7 +++++++
>> > 1 file changed, 7 insertions(+)
>> >
>> > diff --git a/libavformat/utils.c b/libavformat/utils.c
>> > index 81ea239a66..a14d4ef90d 100644
>> > --- a/libavformat/utils.c
>> > +++ b/libavformat/utils.c
>> > @@ -4819,6 +4819,13 @@ void av_url_split(char *proto, int proto_size,
>> >         av_strlcpy(path, ls, path_size);
>> >     else
>> >         ls = &p[strlen(p)];  // XXX
>> > +    /* if path does not start with a /, prepend a / */
>> > +    if (path[0] != '/') {
>> > +        char temp[1024];
>> > +        av_strlcpy(temp, path, path_size);
>> > +        path[0] = '/';
>> > +        av_strlcpy(path + 1, temp, path_size - 1);
>> > +    }
>>
>> I made a patch which fixes this in the HTTP protocol, as modifying the
>> empty path to / is a HTTP requirement, other protocols might not need
>> this.
>>
> Oh I just found this on patchwork. I think my patch becomes redundant
> then. I guess the patch has not been merged yet?
>> http://mplayerhq.hu/pipermail/ffmpeg-devel/2020-February/256902.html

The patch is now applied.

Regards,
Marton
diff mbox series

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 81ea239a66..a14d4ef90d 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -4819,6 +4819,13 @@  void av_url_split(char *proto, int proto_size,
         av_strlcpy(path, ls, path_size);
     else
         ls = &p[strlen(p)];  // XXX
+    /* if path does not start with a /, prepend a / */
+    if (path[0] != '/') {
+        char temp[1024];
+        av_strlcpy(temp, path, path_size);
+        path[0] = '/';
+        av_strlcpy(path + 1, temp, path_size - 1);
+    }
 
     /* the rest is hostname, use that to parse auth/port */
     if (ls != p) {