[Buildroot] [PATCH 0/1] download url broken if url include '+'

[Buildroot] [PATCH 0/1] download url broken if url include '+'

[jemy.zhang at gmail.com]

From: "Jemy Zhang" <jemy.zhang@gmail.com>

While downloading pacakges, the `SITE_METHOD` will be prefixed to the download url of the main package, but the extra download url would not be. If the `SITE URL` accidentally includes '+', url link before '+' would be parsed as `SITE METHOD` by dl-wrapper, and the url broken.

Take `android-tools` package for example:
```
ANDROID_TOOLS_SITE = https://launchpad.net/ubuntu/+archive/primary/+files
ANDROID_TOOLS_EXTRA_DOWNLOADS = android-tools_$(ANDROID_TOOLS_VERSION)-3ubuntu41.debian.tar.gz
```

while build the android-tools package, `ALL_DOWNLOADS` will be expended as:

```
for p in https+https://launchpad.net/ubuntu/+archive/primary/+files/android-tools_4.2.2+git20130218.orig.tar.xz   https://launchpad.ne
t/ubuntu/+archive/primary/+files/android-tools_4.2.2+git20130218-3ubuntu41.debian.tar.gz; do \
        if test ! -e /mnt/fileroot/jun.zhang/buildroot-git/buildroot/dl/android-tools/`basename $p` ; then \
                echo ">>> android-tools 4.2.2+git20130218 Downloading" ; \
                break ; \
        fi ; \
done
```

Notice that there's no 'https+' prefixed to the url, `SITE_METHOD` missing.

and while downloading package, url corrupted:

>--2019-11-05 14:10:29--  http://archive/primary/+files/android-tools_4.2.2+git20130218-3ubuntu41.debian.tar.gz


Jemy Zhang (1):
  fix corrupted download url of extra packages

 package/pkg-generic.mk | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.22.0

[Buildroot] [PATCH 1/1] fix corrupted download url of extra packages

[jemy.zhang at gmail.com]

From: "Jemy Zhang" <jemy.zhang@gmail.com>

If the download url of extra packages contains '+', the contents before
'+' will be parsed as SITE_METHOD, the original download url corrupted.

check with android-tools package.

Signed-off-by: Jemy Zhang <jemy.zhang@gmail.com>
---
 package/pkg-generic.mk | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 893faba22a..bb0ca939a3 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -523,8 +523,9 @@ endif
 $(2)_ALL_DOWNLOADS = \
 	$$(if $$($(2)_SOURCE),$$($(2)_SITE_METHOD)+$$($(2)_SITE)/$$($(2)_SOURCE)) \
 	$$(foreach p,$$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
-		$$(if $$(findstring ://,$$(p)),$$(p),\
-			$$($(2)_SITE)/$$(p)))
+		$$(call getschemeplusuri,$$(call qstrip,\
+		  $$(if $$(findstring ://,$$(p)),$$(p),\
+			  $$($(2)_SITE)/$$(p))),urlencode))
 
 ifndef $(2)_SITE
  ifdef $(3)_SITE
-- 
2.22.0

[Buildroot] [PATCH 1/1] fix corrupted download url of extra packages

[Yann E. MORIN]

Jemy, All,

On 2019-11-05 17:04 +0800, jemy.zhang at gmail.com spake thusly:
> From: "Jemy Zhang" <jemy.zhang@gmail.com>
> 
> If the download url of extra packages contains '+', the contents before
> '+' will be parsed as SITE_METHOD, the original download url corrupted.
> 
> check with android-tools package.

Indeed this is broken. Thank you very much for noticing the issue.

> Signed-off-by: Jemy Zhang <jemy.zhang@gmail.com>
> ---
>  package/pkg-generic.mk | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 893faba22a..bb0ca939a3 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -523,8 +523,9 @@ endif
>  $(2)_ALL_DOWNLOADS = \
>  	$$(if $$($(2)_SOURCE),$$($(2)_SITE_METHOD)+$$($(2)_SITE)/$$($(2)_SOURCE)) \
>  	$$(foreach p,$$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
> -		$$(if $$(findstring ://,$$(p)),$$(p),\
> -			$$($(2)_SITE)/$$(p)))
> +		$$(call getschemeplusuri,$$(call qstrip,\
> +		  $$(if $$(findstring ://,$$(p)),$$(p),\
> +			  $$($(2)_SITE)/$$(p))),urlencode))

I am not certain that we want to unconditionally enforce urlencode on
all additional downloads...

Instead, I think we should just replicate the site method for extra
downloads that use the same site as the main download.

I've sent a patch that does just that:
    https://patchwork.ozlabs.org/patch/1192073/

Can you see if that fixes it for you (it did for me, but if you have an
alternate test-case, that'd be nice to have feedback on it too).

Thanks again for the report! :-)

Regards,
Yann E. MORIN.

>  ifndef $(2)_SITE
>   ifdef $(3)_SITE
> -- 
> 2.22.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] fix corrupted download url of extra packages

[jemy zhang]

Correct, if the extra download url is relative to the main, only
SITE_METHOD prefix is needed. Thanks for your reviews.

Yann E. MORIN <yann.morin.1998@free.fr> ? 2019?11?9??? ??1:28???

> Jemy, All,
>
> On 2019-11-05 17:04 +0800, jemy.zhang at gmail.com spake thusly:
> > From: "Jemy Zhang" <jemy.zhang@gmail.com>
> >
> > If the download url of extra packages contains '+', the contents before
> > '+' will be parsed as SITE_METHOD, the original download url corrupted.
> >
> > check with android-tools package.
>
> Indeed this is broken. Thank you very much for noticing the issue.
>
> > Signed-off-by: Jemy Zhang <jemy.zhang@gmail.com>
> > ---
> >  package/pkg-generic.mk | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 893faba22a..bb0ca939a3 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -523,8 +523,9 @@ endif
> >  $(2)_ALL_DOWNLOADS = \
> >       $$(if
> $$($(2)_SOURCE),$$($(2)_SITE_METHOD)+$$($(2)_SITE)/$$($(2)_SOURCE)) \
> >       $$(foreach p,$$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
> > -             $$(if $$(findstring ://,$$(p)),$$(p),\
> > -                     $$($(2)_SITE)/$$(p)))
> > +             $$(call getschemeplusuri,$$(call qstrip,\
> > +               $$(if $$(findstring ://,$$(p)),$$(p),\
> > +                       $$($(2)_SITE)/$$(p))),urlencode))
>
> I am not certain that we want to unconditionally enforce urlencode on
> all additional downloads...
>
> Instead, I think we should just replicate the site method for extra
> downloads that use the same site as the main download.
>
> I've sent a patch that does just that:
>     https://patchwork.ozlabs.org/patch/1192073/
>
> Can you see if that fixes it for you (it did for me, but if you have an
> alternate test-case, that'd be nice to have feedback on it too).
>
> Thanks again for the report! :-)
>
> Regards,
> Yann E. MORIN.
>
> >  ifndef $(2)_SITE
> >   ifdef $(3)_SITE
> > --
> > 2.22.0
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot at busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
>
> --
>
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
>      |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is
> no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
>  conspiracy.  |
>
> '------------------------------^-------^------------------^--------------------'
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20191109/5535bf9a/attachment-0001.html>