-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Support redirects in uv publish
#17130
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
Support redirects in uv publish
#17130
Conversation
b4b34b2 to
9495e20
Compare
uv publishuv publish
konstin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good already!
We need one more addition for security: #17126 (comment)
crates/uv-publish/src/lib.rs
Outdated
| "The request was redirected, but redirects are not allowed when publishing, please use the canonical URL: `{0}`" | ||
| )] | ||
| RedirectError(Url), | ||
| #[error("Too many redirects: {0} (max number of redirects: {1})")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this error message, isn't the number of retry always the maximum number of redirects? i.e., can we say something like "Too many redirects, only {n_past_redirections} redirects are allowed"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, fixed :)
crates/uv-publish/src/lib.rs
Outdated
| Ok(response) => { | ||
| // When the user accidentally uses https://test.pypi.org/legacy (no slash) as publish URL, we | ||
| // get a redirect to https://test.pypi.org/legacy/ (the canonical index URL). | ||
| // In the above case we get 308, reqwest or `RedirectClientWithMiddleware` would try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a word like a "where" missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
crates/uv-publish/src/lib.rs
Outdated
| if let Some(location_header) = response.headers().get(LOCATION) { | ||
| let location = location_header.to_str().unwrap(); | ||
| current_registry = DisplaySafeUrl::parse(location).unwrap(); | ||
| warn!("Redirecting the request to: {}", current_registry); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| warn!("Redirecting the request to: {}", current_registry); | |
| debug!("Redirecting the request to: {}", current_registry); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
crates/uv-publish/src/lib.rs
Outdated
| )); | ||
| } | ||
| if let Some(location_header) = response.headers().get(LOCATION) { | ||
| let location = location_header.to_str().unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't unwrap here, we need to handle those errors gracefully. In uv production code, we need to avoid panics whenever possible; In this case, it's external input, so we have to assume it's in some way malformed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed, please take a look
crates/uv-publish/src/lib.rs
Outdated
| .mount(&mock_server) | ||
| .await; | ||
|
|
||
| let group = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: We don't need to have an inner block for the assignment, it can be let group = UploadDistribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
c94cf68 to
3be2223
Compare
|
@konstin, added a check that the new URL is in the same realm: ...
if Realm::from(¤t_registry) == Realm::from(registry) {
debug!("Redirecting the request to: {}", current_registry);
n_past_redirections += 1;
continue;
}
... |
3be2223 to
100be27
Compare
100be27 to
424ba0b
Compare
|
Thanks for fixing! I switched to explicit error types and added a test for infinite redirects. |
|
I also added a test for the realm switch, since we consider that a security feature. |
|
Thanks for merging, @konstin! |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.9.17` -> `0.9.18` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.9.18`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0918) [Compare Source](astral-sh/uv@0.9.17...0.9.18) Released on 2025-12-16. ##### Enhancements - Add value hints to command line arguments to improve shell completion accuracy ([#​17080](astral-sh/uv#17080)) - Improve error handling in `uv publish` ([#​17096](astral-sh/uv#17096)) - Improve rendering of multiline error messages ([#​17132](astral-sh/uv#17132)) - Support redirects in `uv publish` ([#​17130](astral-sh/uv#17130)) - Include Docker images with the alpine version, e.g., `python3.x-alpine3.23` ([#​17100](astral-sh/uv#17100)) ##### Configuration - Accept `--torch-backend` in `[tool.uv]` ([#​17116](astral-sh/uv#17116)) ##### Performance - Speed up `uv cache size` ([#​17015](astral-sh/uv#17015)) - Initialize S3 signer once ([#​17092](astral-sh/uv#17092)) ##### Bug fixes - Avoid panics due to reads on failed requests ([#​17098](astral-sh/uv#17098)) - Enforce latest-version in `@latest` requests ([#​17114](astral-sh/uv#17114)) - Explicitly set `EntryType` for file entries in tar ([#​17043](astral-sh/uv#17043)) - Ignore `pyproject.toml` index username in lockfile comparison ([#​16995](astral-sh/uv#16995)) - Relax error when using `uv add` with `UV_GIT_LFS` set ([#​17127](astral-sh/uv#17127)) - Support file locks on ExFAT on macOS ([#​17115](astral-sh/uv#17115)) - Change schema for `exclude-newer` into optional string ([#​17121](astral-sh/uv#17121)) ##### Documentation - Drop arm musl caveat from Docker documentation ([#​17111](astral-sh/uv#17111)) - Fix version reference in resolver example ([#​17085](astral-sh/uv#17085)) - Better documentation for `exclude-newer*` ([#​17079](astral-sh/uv#17079)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi41Ny4xIiwidXBkYXRlZEluVmVyIjoiNDIuNTcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
<!-- Thank you for contributing to uv! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary Follow redirects for `uv publish`. Related issue: astral-sh#17126. ## Test Plan <!-- How was it tested? --> Added a unit test to test the custom redirect logic. --------- Co-authored-by: konstin <konstin@mailbox.org>
Summary
Follow redirects for
uv publish. Fixes #17126.Test Plan
Added a unit test to test the custom redirect logic.