| 1 |
On 5/31/20 12:21 PM, Mike Gilbert wrote: |
| 2 |
> On Sun, May 31, 2020 at 2:01 PM Zac Medico <zmedico@g.o> wrote: |
| 3 |
>> |
| 4 |
>> On 5/31/20 6:17 AM, Mike Gilbert wrote: |
| 5 |
>>> Unquote the URL basename when fetching from upstream. |
| 6 |
>>> Quote the filename when fetching from mirrors. |
| 7 |
>>> |
| 8 |
>>> Bug: https://bugs.gentoo.org/719810 |
| 9 |
>>> Signed-off-by: Mike Gilbert <floppym@g.o> |
| 10 |
>>> --- |
| 11 |
>>> lib/portage/dbapi/porttree.py | 7 ++++++- |
| 12 |
>>> lib/portage/package/ebuild/fetch.py | 9 +++++++-- |
| 13 |
>>> 2 files changed, 13 insertions(+), 3 deletions(-) |
| 14 |
>>> |
| 15 |
>>> diff --git a/lib/portage/dbapi/porttree.py b/lib/portage/dbapi/porttree.py |
| 16 |
>>> index 08af17bcd..984263039 100644 |
| 17 |
>>> --- a/lib/portage/dbapi/porttree.py |
| 18 |
>>> +++ b/lib/portage/dbapi/porttree.py |
| 19 |
>>> @@ -55,6 +55,11 @@ try: |
| 20 |
>>> except ImportError: |
| 21 |
>>> from urlparse import urlparse |
| 22 |
>>> |
| 23 |
>>> +try: |
| 24 |
>>> + from urllib.parse import unquote as urlunquote |
| 25 |
>>> +except ImportError: |
| 26 |
>>> + from urllib import unquote as urlunquote |
| 27 |
>>> + |
| 28 |
>>> if sys.hexversion >= 0x3000000: |
| 29 |
>>> # pylint: disable=W0622 |
| 30 |
>>> basestring = str |
| 31 |
>>> @@ -1547,7 +1552,7 @@ def _parse_uri_map(cpv, metadata, use=None): |
| 32 |
>>> myuris.pop() |
| 33 |
>>> distfile = myuris.pop() |
| 34 |
>>> else: |
| 35 |
>>> - distfile = os.path.basename(uri) |
| 36 |
>>> + distfile = urlunquote(os.path.basename(uri)) |
| 37 |
>>> if not distfile: |
| 38 |
>>> raise portage.exception.InvalidDependString( |
| 39 |
>>> ("getFetchMap(): '%s' SRC_URI has no file " + \ |
| 40 |
>>> diff --git a/lib/portage/package/ebuild/fetch.py b/lib/portage/package/ebuild/fetch.py |
| 41 |
>>> index 28e7caf53..47c3ad28f 100644 |
| 42 |
>>> --- a/lib/portage/package/ebuild/fetch.py |
| 43 |
>>> +++ b/lib/portage/package/ebuild/fetch.py |
| 44 |
>>> @@ -26,6 +26,11 @@ try: |
| 45 |
>>> except ImportError: |
| 46 |
>>> from urlparse import urlparse |
| 47 |
>>> |
| 48 |
>>> +try: |
| 49 |
>>> + from urllib.parse import quote as urlquote |
| 50 |
>>> +except ImportError: |
| 51 |
>>> + from urllib import quote as urlquote |
| 52 |
>>> + |
| 53 |
>>> import portage |
| 54 |
>>> portage.proxy.lazyimport.lazyimport(globals(), |
| 55 |
>>> 'portage.package.ebuild.config:check_config_instance,config', |
| 56 |
>>> @@ -351,7 +356,7 @@ _size_suffix_map = { |
| 57 |
>>> |
| 58 |
>>> class FlatLayout(object): |
| 59 |
>>> def get_path(self, filename): |
| 60 |
>>> - return filename |
| 61 |
>>> + return urlquote(filename) |
| 62 |
>>> |
| 63 |
>>> def get_filenames(self, distdir): |
| 64 |
>>> for dirpath, dirnames, filenames in os.walk(distdir, |
| 65 |
>>> @@ -382,7 +387,7 @@ class FilenameHashLayout(object): |
| 66 |
>>> c = c // 4 |
| 67 |
>>> ret += fnhash[:c] + '/' |
| 68 |
>>> fnhash = fnhash[c:] |
| 69 |
>>> - return ret + filename |
| 70 |
>>> + return ret + urlquote(filename) |
| 71 |
>>> |
| 72 |
>>> def get_filenames(self, distdir): |
| 73 |
>>> pattern = '' |
| 74 |
>>> |
| 75 |
>> |
| 76 |
>> We've also got these other basename calls in fetch.py: |
| 77 |
>> |
| 78 |
>>> diff --git a/lib/portage/package/ebuild/fetch.py b/lib/portage/package/ebuild/fetch.py |
| 79 |
>>> index 28e7caf53..56b375d58 100644 |
| 80 |
>>> --- a/lib/portage/package/ebuild/fetch.py |
| 81 |
>>> +++ b/lib/portage/package/ebuild/fetch.py |
| 82 |
>>> @@ -730,9 +730,9 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0, |
| 83 |
>>> else: |
| 84 |
>>> for myuri in myuris: |
| 85 |
>>> if urlparse(myuri).scheme: |
| 86 |
>>> - file_uri_tuples.append((os.path.basename(myuri), myuri)) |
| 87 |
>>> + file_uri_tuples.append((urlunquote(os.path.basename(myuri)), myuri)) |
| 88 |
>>> else: |
| 89 |
>>> - file_uri_tuples.append((os.path.basename(myuri), None)) |
| 90 |
>>> + file_uri_tuples.append((urlunquote(os.path.basename(myuri)), None)) |
| 91 |
>> |
| 92 |
>> The case with no scheme is not a URI, so we need to decide whether |
| 93 |
>> or not to unquote, and we should make _parse_uri_map behavior |
| 94 |
>> consistent for this case. |
| 95 |
> |
| 96 |
> Backing up, I think unquoting the basename is really a separate issue |
| 97 |
> from quoting the filename in Layout.get_path(). The latter fixes a |
| 98 |
> known bug when fetching from mirrors, whereas the former seems like a |
| 99 |
> cosmetic issue. I don't think we should mix these two up into the same |
| 100 |
> commit. |
| 101 |
> |
| 102 |
> Could I get your approval to push my original patch (Escape |
| 103 |
> percent-signs in filename when fetching from mirrors)? |
| 104 |
|
| 105 |
Separate patches are fine, but both patches really should be merged at |
| 106 |
the same time since the quoting and unquoting are inherently coupled. |
| 107 |
-- |
| 108 |
Thanks, |
| 109 |
Zac |
Archives