[Regression] pulp fails downloading packages with special symbols from Amazon Linux repositories

[fao89]

Author: micuz (micuz)

Redmine Issue: 9529, https://pulp.plan.io/issues/9529

---

Fresh installed Pulp 3.16.0.
Probably regression of issue 8875.

Steps to reproduce:

1. pulp rpm repository create --name cerved-amzn2-core
...
1. pulp rpm remote create --name cerved-amzn2-core --url http://amazonlinux.eu-central-1.amazonaws.com/2/core/latest/x86_64/mirror.list
...
1. pulp rpm repository sync --name cerved-amzn2-core --remote cerved-amzn2-core
Started background task /pulp/api/v3/tasks/633605bb-8c53-4e75-9a8a-595c9718524a/
........Error: Task /pulp/api/v3/tasks/633605bb-8c53-4e75-9a8a-595c9718524a/ failed: '403, message='Forbidden', url=URL('http://amazonlinux.eu-central-1.amazonaws.com/blobstore/c29be7cc532fd89e7565ec6e981c8f441f1184259db1eb492aad42e7a1a1c2af/uuid-c++-1.6.2-26.amzn2.0.1.x86_64.rpm')'

[fao89]

From: micuz (micuz)
Date: 2021-10-22T14:17:24Z

---

fix for 9464 could have regressed this but I'm not sure

[fao89]

From: @ggainey (ggainey)
Date: 2021-10-22T14:40:03Z

---

Regression caused by the fix for #9464. IRC discussion:

mimmus
hi, I'm taking up syncing an Amazon Linux repo again, after ggainey_ kindly produced a patch to overcome a bug (8875)
mimmus
patch seemed decisive but now I'm getting again:
$ pulp --no-verify-ssl --username admin rpm repository sync --name cerved-amzn2-core --remote cerved-amzn2-core
password:
Started background task /pulp/api/v3/tasks/b86fb6f5-8e36-4c73-9d3f-a8852099e2e3/
..................Error: Task /pulp/api/v3/tasks/b86fb6f5-8e36-4c73-9d3f-a8852099e2e3/ failed: '403, message='Forbidden', url=URL('http://amazonlinux.us-east-1.amazonaws.com/blobstore/c29be7cc532fd89e7565ec6e981c8f441f1184259db1eb492aad42e7a1a1c2af/uuid-c++-1.6.2-26.amzn2.0.1.x86_64.rpm')'
mimmus
regression?
ggainey
mimmus: hullo - folk are off today, but I happened to notice this - yes, that feels like a regression. Sigh. Open an issue, refer to 8875, add in the remote's URL, I'll look at Monday
ggainey
(esp since I know which bug we fixed, that likely introduced the regression, and I'm the one that 'fixed' it...)
mimmus
:-) thank you!
ggainey
altho how the fix for 9464 could have regressed this, I am not sure
ggainey
sigh
mimmus
just before using Pulp in real world 😰
mimmus
https://pulp.plan.io/issues/9529
ggainey
thank you
ggainey
I can see what's going on - urlunparse undoes the quote() work, because It Knows Better
ggainey
argh
ggainey
mimmus: if I give you a patch in the next 5 min, are you game to test it?
mimmus
yes!
ggainey
what version of pulp_rpm are you on?
mimmus
how can I check?
mimmus
it's not a pkg, I installed by pulp_installer
mimmus
3.16.0 ?
ggainey
http :/pulp/api/v3/status/ will tell you the versions of all installed plugins
ggainey
(or "pulp status" if you have pulp-cli installed)
mimmus
 "versions": [
    {
      "component": "core",
      "version": "3.16.0"
    },
    {
      "component": "rpm",
      "version": "3.16.0"
    }
  ],
ggainey
yup, cool
ggainey
ok, let me test this before I inflict it on you
mimmus
"inflict it on you" 🤣
ggainey
only fair, I inflicted the regression on you
ggainey
and it works - here's a diff : https://paste.centos.org/view/97b1259d
ggainey
unparse(), it turns out, "helpfully" undoes the quote() output back into ++ . When fixing the bnasic-auth problem, I didn't run the amazon test, and it isn't part of the auto-tests because syncing amazon-core takes too much time/space for CI runs. (not an excuse, btw, just explaining to myself how we/I missed this regression)
ggainey
fun!
ggainey
I'll add to that issue, one sec

This diff fixes the problem:

(pulp3) (3.16) ~/github/Pulp3/pulp_rpm $ git diff
diff --git a/pulp_rpm/app/downloaders.py b/pulp_rpm/app/downloaders.py
index 67bcb948..bd5a102b 100644
--- a/pulp_rpm/app/downloaders.py
+++ b/pulp_rpm/app/downloaders.py
@@ -69,8 +69,8 @@ class RpmDownloader(HttpDownloader):
             # Let's make them happy - while not urlencoding **anything else**
             parsed = urlparse(self.url)
             new_path = quote(unquote(parsed.path), safe=":/")
-            parsed._replace(path=new_path)
-            new_url = urlunparse(parsed)
+            from urllib.parse import urljoin
+            new_url = urljoin(self.url, new_path)
 
         if self.sles_auth_token:
             auth_param = f"?{self.sles_auth_token}"
(pulp3) (3.16) ~/github/Pulp3/pulp_rpm $ 

[fao89]

From: pulpbot (pulpbot)
Date: 2021-10-25T16:32:09Z

---

PR: #2160

[fao89]

From: @ggainey (ggainey)
Date: 2021-10-26T13:11:09Z

---

Applied in changeset commit:d903ec9a8d473f57f9fab28410c4088806c4eb0c.

[dralley]

#2319 (comment)