[PATCH] bitbake: git fetcher: use urllib quote ...

[PATCH] bitbake: git fetcher: use urllib quote ...

[CESTONARO Thilo]

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

to use the path url-compatible. This needs to happen before the shell quotation happens.

Without this commit, spaces in the clone URL will be used as " " and not as "%20" which will fail.
This commit changes the " " in the URL to "%20" when it is a http or https url.

Signed-off-by: Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>
---
 lib/bb/fetch2/git.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 578edc59..dc7f848d 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -66,6 +66,7 @@ import re
 import shlex
 import subprocess
 import tempfile
+import urllib
 import bb
 import bb.progress
 from contextlib import contextmanager
@@ -697,7 +698,12 @@ class Git(FetchMethod):
             username = ud.user + '@'
         else:
             username = ""
-        return "%s://%s%s%s" % (ud.proto, username, ud.host, ud.path)
+
+        path = ud.path
+        if ud.proto in [ 'http', 'https' ]:
+            path = urllib.parse.quote(ud.path)
+
+        return "%s://%s%s%s" % (ud.proto, username, ud.host, path)

     def _revision_key(self, ud, d, name):
         """
--
2.37.2


[-- Attachment #2: Type: text/html, Size: 3561 bytes --]

Re: [bitbake-devel] [PATCH] bitbake: git fetcher: use urllib quote ...

[Michael Opdenacker]

Hi Thilo,


Thanks for the patch!


On 22.03.23 at 09:37, Thilo C. via lists.openembedded.org wrote:
> to use the path url-compatible. This needs to happen before the shell 
> quotation happens.
>
> Without this commit, spaces in the clone URL will be used as " " and 
> not as "%20" which will fail.
> This commit changes the " " in the URL to "%20" when it is a http or 
> https url.
>
> Signed-off-by: Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>


Oops, are you sure it applies to the latest "master" branch? I tried and 
it didn't, though I don't see why...


Another issue with your patch is the commit author name, which will be 
"Thilo C. via lists.openembedded.org 
<thilo.cestonaro=thalesgroup.com@lists.openembedded.org>" instead of 
"Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>".


See 
https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded#Fixing_your_From_identity 
for details and a solution.


Don't hesitate to try to send the patch again to me privately before 
resending to the mailing list...


Thanks again
Cheers
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

AW: [bitbake-devel] [PATCH] bitbake: git fetcher: use urllib quote ...

[CESTONARO Thilo]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

Hi Michael

Sorry for the late answer.
I'm currently trying to be able to directly send the patches via Git but our IT does not allow it.

So here is another try with Outlook. And hopefully it does not change anything, like last time. As I think it was Outlooks fault that the patch did not apply.

This time I gzipped it. Hopefully this is ok for you!

Cheers
Thilo

________________________________________
Von: Michael Opdenacker <michael.opdenacker@bootlin.com>
Gesendet: Mittwoch, 22. März 2023 11:40
An: CESTONARO Thilo
Cc: bitbake-devel@lists.openembedded.org
Betreff: Re: [bitbake-devel] [PATCH] bitbake: git fetcher: use urllib quote ...

Hi Thilo,


Thanks for the patch!


On 22.03.23 at 09:37, Thilo C. via lists.openembedded.org wrote:
> to use the path url-compatible. This needs to happen before the shell
> quotation happens.
>
> Without this commit, spaces in the clone URL will be used as " " and
> not as "%20" which will fail.
> This commit changes the " " in the URL to "%20" when it is a http or
> https url.
>
> Signed-off-by: Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>


Oops, are you sure it applies to the latest "master" branch? I tried and
it didn't, though I don't see why...


Another issue with your patch is the commit author name, which will be
"Thilo C. via lists.openembedded.org
<thilo.cestonaro=thalesgroup.com@lists.openembedded.org>" instead of
"Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>".


See
https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded#Fixing_your_From_identity
for details and a solution.


Don't hesitate to try to send the patch again to me privately before
resending to the mailing list...


Thanks again
Cheers
Michael.

--
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


[-- Attachment #2: 0001-bitbake-git-fetcher-use-urllib-quote.patch.gz --]
[-- Type: application/gzip, Size: 778 bytes --]

Re: [bitbake-devel] [PATCH] bitbake: git fetcher: use urllib quote ...

[Alexandre Belloni]

Hello,

This doesn't apply on master, can you rebase?

Thanks

On 22/03/2023 08:37:15+0000, Thilo C. via lists.openembedded.org wrote:
> to use the path url-compatible. This needs to happen before the shell quotation happens.
> 
> Without this commit, spaces in the clone URL will be used as " " and not as "%20" which will fail.
> This commit changes the " " in the URL to "%20" when it is a http or https url.
> 
> Signed-off-by: Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>
> ---
>  lib/bb/fetch2/git.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> index 578edc59..dc7f848d 100644
> --- a/lib/bb/fetch2/git.py
> +++ b/lib/bb/fetch2/git.py
> @@ -66,6 +66,7 @@ import re
>  import shlex
>  import subprocess
>  import tempfile
> +import urllib
>  import bb
>  import bb.progress
>  from contextlib import contextmanager
> @@ -697,7 +698,12 @@ class Git(FetchMethod):
>              username = ud.user + '@'
>          else:
>              username = ""
> -        return "%s://%s%s%s" % (ud.proto, username, ud.host, ud.path)
> +
> +        path = ud.path
> +        if ud.proto in [ 'http', 'https' ]:
> +            path = urllib.parse.quote(ud.path)
> +
> +        return "%s://%s%s%s" % (ud.proto, username, ud.host, path)
> 
>      def _revision_key(self, ud, d, name):
>          """
> --
> 2.37.2
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14612): https://lists.openembedded.org/g/bitbake-devel/message/14612
> Mute This Topic: https://lists.openembedded.org/mt/97773918/3617179
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [bitbake-devel] [PATCH] bitbake: git fetcher: use urllib quote ...

[Richard Purdie]

On Wed, 2023-03-22 at 08:37 +0000, Thilo C. via lists.openembedded.org
wrote:
> to use the path url-compatible. This needs to happen before the shell
> quotation happens.
> 
> Without this commit, spaces in the clone URL will be used as " " and
> not as "%20" which will fail.
> This commit changes the " " in the URL to "%20" when it is a http or
> https url.
> 
> Signed-off-by: Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>
> ---
>  lib/bb/fetch2/git.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> index 578edc59..dc7f848d 100644
> --- a/lib/bb/fetch2/git.py
> +++ b/lib/bb/fetch2/git.py
> @@ -66,6 +66,7 @@ import re
>  import shlex
>  import subprocess
>  import tempfile
> +import urllib
>  import bb
>  import bb.progress
>  from contextlib import contextmanager
> @@ -697,7 +698,12 @@ class Git(FetchMethod):
>              username = ud.user + '@'
>          else:
>              username = ""
> -        return "%s://%s%s%s" % (ud.proto, username, ud.host,
> ud.path)
> +
> +        path = ud.path
> +        if ud.proto in [ 'http', 'https' ]:
> +            path = urllib.parse.quote(ud.path)
> +
> +        return "%s://%s%s%s" % (ud.proto, username, ud.host, path)
>  
>      def _revision_key(self, ud, d, name):
>          """

I'd like to ensure that we add test coverage to "bitbake-selftest" to
cover this situation (see lib/bb/tests/fetch.py).

We tend not to accept fixes like this unless we have test coverage to
prevent future regressions.

Cheers,

Richard

[PATCH v2] bitbake: git fetcher: use urllib quote ...

[CESTONARO Thilo]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

Hi Richard!

> I'd like to ensure that we add test coverage to "bitbake-selftest" to
> cover this situation (see lib/bb/tests/fetch.py).
> 
> We tend not to accept fixes like this unless we have test coverage to
> prevent future regressions.

Next version of my patch. I tried to have a as simple as possible test which exactly does show what goes wrong without my patch.

Thank you!

Cheers,
Thilo




[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-bitbake-git-fetcher-use-urllib-quote.patch --]
[-- Type: text/x-patch; name="0001-bitbake-git-fetcher-use-urllib-quote.patch", Size: 2286 bytes --]

From 1bc898b5e1abd98d2932dce053dc82b3f3d6b3f8 Mon Sep 17 00:00:00 2001
From: Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>
Date: Tue, 7 Feb 2023 13:44:02 +0100
Subject: [PATCH] bitbake: git fetcher: use urllib quote ...

to use the path url-compatible. This needs to happen before the shell quotation happens.

Without this commit, spaces in the clone URL will be used as " " and not as "%20" which will fail.
This commit changes the " " in the URL to "%20" when it is a http or https url.

Signed-off-by: Thilo Cestonaro <thilo.cestonaro@thalesgroup.com>
---
 lib/bb/fetch2/git.py  |  8 +++++++-
 lib/bb/tests/fetch.py | 13 +++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 2a3c06fe..28729241 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -67,6 +67,7 @@ import re
 import shlex
 import subprocess
 import tempfile
+import urllib
 import bb
 import bb.progress
 from contextlib import contextmanager
@@ -698,7 +699,12 @@ class Git(FetchMethod):
             username = ud.user + '@'
         else:
             username = ""
-        return "%s://%s%s%s" % (ud.proto, username, ud.host, ud.path)
+
+        path = ud.path
+        if ud.proto in [ 'http', 'https' ]:
+            path = urllib.parse.quote(ud.path)
+
+        return "%s://%s%s%s" % (ud.proto, username, ud.host, path)
 
     def _revision_key(self, ud, d, name):
         """
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 6ef0836f..db798f0d 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -1697,6 +1697,19 @@ class GitShallowTest(FetcherTest):
         ud = fetcher.ud[uri]
         return fetcher, ud
 
+    def assert_uri_with_spaces(self):
+        class FetchDataFake():
+            proto = "https"
+            user = ""
+            host = "example.org"
+            path = "/git/url with spaces/imaginary.git"
+
+        m = bb.fetch2.git.Git(self.d)
+        ud = FetchDataFake()
+        urlgenerated = m._get_repo_url(ud)
+
+        self.assertEqual("https://example.org/git/url%20with%20spaces/imaginary.git", urlgenerated)
+
     def fetch_and_unpack(self, uri=None):
         fetcher, ud = self.fetch(uri)
         fetcher.unpack(self.d.getVar('WORKDIR'))
-- 
2.39.2

Re: [bitbake-devel] [PATCH v2] bitbake: git fetcher: use urllib quote ...

[Michael Opdenacker]

Hi Thilo,

On 10.05.23 at 15:10, Thilo C. via lists.openembedded.org wrote:
> Hi Richard!
>
>> I'd like to ensure that we add test coverage to "bitbake-selftest" to
>> cover this situation (see lib/bb/tests/fetch.py).
>>
>> We tend not to accept fixes like this unless we have test coverage to
>> prevent future regressions.
> Next version of my patch. I tried to have a as simple as possible test which exactly does show what goes wrong without my patch.


Thank you very much for the patch, but please don't send it as an 
attachment. Otherwise, people cannot comment or ask questions about 
specific lines.

You should basically send your patches through "git send-email". See 
https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded

Thanks in advance :)
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com