Project

Profile

Help

Issue #8670

URL escaping in yarl alters escaped characters

Added by dannysauer 3 months ago. Updated 26 days ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
-
Category:
-
Sprint/Milestone:
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
Master
Platform Release:
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Sprint:
Sprint 98
Quarter:

Description

After signing the URL when using the S3 backend (or presumably Azure), the call to generate the HTTP redirect re-parses the URL returned, sometimes resulting in characters being encoded as HTML entities or previously-encoded entities being decoded. While S3 is ok with ignoring an altered query string, CloudFront expects the entire URL to match what was signed exactly. This wasn't an issue before I patched django-storages to actually include the parameters passed in to the artifact_file.storage.url() call, but once that patch was applied, query strings were sent back with the URL for content-disposition.

It appears that providing a Yarl URL object to the raise HTTPFound() prevents re-parsing, resulting in the query string being left alone. If the object is created using URL() with encoded=True, that bypasses the reparsing in Yarl, and everyone's much happier. It's also verified to not break regular S3 operation.

Associated revisions

Revision 6066e5b1 View on GitHub
Added by dannysauer about 1 month ago

Stabilize URLs before generating redirects

The aio URL object mangles query strings when they're passed in. Specifically, it unescapes http entities which are escaped but which don't need escaped. This is normally ok, but in the case of signed URLs, changing anything about the URL invalidates the signature. By pre-creating a yarl.URL object before raising the redirect exception, the parsing doesn't happen in aiohttp. Passing in the "encoding=True" stops the reparsing when the object is created.

fixes #8670 https://pulp.plan.io/issues/8670

History

#1 Updated by pulpbot 3 months ago

  • Status changed from NEW to POST

#2 Updated by fao89 3 months ago

  • Triaged changed from No to Yes
  • Sprint set to Sprint 95

#3 Updated by rchan 3 months ago

  • Sprint changed from Sprint 95 to Sprint 96

#4 Updated by rchan 2 months ago

  • Sprint changed from Sprint 96 to Sprint 97

#5 Updated by rchan about 2 months ago

  • Sprint changed from Sprint 97 to Sprint 98

#6 Updated by dannysauer about 1 month ago

  • Status changed from POST to MODIFIED

#7 Updated by pulpbot 26 days ago

  • Sprint/Milestone set to 3.14.0

#8 Updated by pulpbot 26 days ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE

Please register to edit this issue

Also available in: Atom PDF