Skip to content

DO NOT MERGE YET encode + as %2B in S3 paths #5034

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 2 commits into from

Conversation

carols10cents
Copy link
Member

🚨 🚨 🚨 🚨 We need to do migration of some files on S3 before this can be merged!!! 🚨 🚨 🚨 🚨

Fixes #4891, I think.

To test this:

  • First, on the master branch with S3 configured, publish a crate with some build metadata (that is, containing a +)
  • Observe the behavior reported in Encoded URLs with a + sign fail #4891, that the S3 URL is treating the + more like a space: requests to the S3 URL containing %2B (encoded +) will return 403 when it should return 200.
  • Manually do the mass migration I think we need to script to change all + into %2B, by downloading and re-uploading the S3 file using %2B instead of + (or using the aws CLI). Don't use the S3 "copy" web interface, that seems to double encode.
  • Observe requests to the S3 URL containing %2B now return 200.
  • Switch to this PR's branch.
  • Create a new local project that depends on the migrated crate containing build metadata on the registry now running this branch, to simulate us having deployed it. Verify Cargo can find and download that crate version.
  • Remove the old S3 file containing + and confirm Cargo can still find and download that crate version (clear Cargo's download cache first to be sure), to simulate cleaning up after this branch is deployed
  • Publish a new version containing build metadata and observe that when this branch does the uploading, the + gets encoded as %2B in the S3 path
  • Verify Cargo can find and download that crate version too.

@bors
Copy link
Contributor

bors commented Nov 25, 2022

☔ The latest upstream changes (presumably #5535) made this pull request unmergeable. Please resolve the merge conflicts.

@Turbo87
Copy link
Member

Turbo87 commented Jun 21, 2023

this has been superseded by #6649 and some other upcoming PRs :)

@Turbo87 Turbo87 closed this Jun 21, 2023
@Turbo87 Turbo87 deleted the plus-encoding branch September 21, 2023 09:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encoded URLs with a + sign fail
3 participants