Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use correct host in URL construction #17338

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

miketheman
Copy link
Member

From #17324

This catches us often in tasks, defaulting to localhost.

This catches us often in tasks, defaulting to `localhost`.

Signed-off-by: Mike Fiedler <[email protected]>
@miketheman miketheman requested a review from a team as a code owner December 30, 2024 18:46
@miketheman
Copy link
Member Author

miketheman commented Dec 30, 2024

I did some code spelunking to see if this was relatively easy to make "standard", and here's the path:

  • construct a request-like object when called via get_request via pyramid.scripting.prepare(registry=registry)
  • as we don't pass a request (how could we? it doesn't exist yet!), only a registry to prepare()
  • it in turn calls _make_request with a path of / and registry
  • inside webob.request.BaseRequest.blank, where a call to environ_from_url(path='/') will return http://localhost:80/ from this logic
    It doesn't look like there's an opportunity to inject a domain setting from the registry along that path.

I experimented with crafting a dummy request object by pulling the domain from the settings, but since we don't have a full URL handy, it doesn't work well.

diff --git a/warehouse/tasks.py b/warehouse/tasks.py
index 830906cb8..ce0db84a3 100644
--- a/warehouse/tasks.py
+++ b/warehouse/tasks.py
@@ -26,6 +26,7 @@ import transaction
 import venusian

 from kombu import Queue
+from pyramid.request import Request
 from pyramid.threadlocal import get_current_request
 from urllib3.util import parse_url

@@ -92,7 +93,8 @@ class WarehouseTask(celery.Task):
     def get_request(self):
         if not hasattr(self.request, "pyramid_env"):
             registry = self.app.pyramid_config.registry
-            env = pyramid.scripting.prepare(registry=registry)
+            _request = Request.blank("/", base_url=registry.settings.get("warehouse.domain"))
+            env = pyramid.scripting.prepare(request=_request, registry=registry)
             env["request"].tm = transaction.TransactionManager(explicit=True)
             env["request"].timings = {"new_request_start": time.time() * 1000}
             env["request"].remote_addr = "127.0.0.1"

with a WAREHOUSE_DOMAIN of pypi.org ends up as:

'request': <Request at 0xffff8bad6480 GET http://localhostpypi.org/>`

Going to proceed with these changes for now.

@miketheman miketheman merged commit 775dc3b into pypi:main Dec 30, 2024
20 checks passed
@miketheman miketheman deleted the miketheman/fix-domain branch December 30, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant