Skip to content

Conversation

@dijor0310
Copy link
Contributor

@dijor0310 dijor0310 commented Dec 14, 2025

Summary

Follow redirects for uv publish. Fixes #17126.

Test Plan

Added a unit test to test the custom redirect logic.

@dijor0310 dijor0310 force-pushed the diyor/add-redirect-uv-publish branch 2 times, most recently from b4b34b2 to 9495e20 Compare December 14, 2025 23:44
@konstin konstin changed the title add redirect logic to uv publish Support redirects in uv publish Dec 15, 2025
@konstin konstin added the enhancement New feature or improvement to existing functionality label Dec 15, 2025
Copy link
Member

@konstin konstin left a 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)

"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})")]
Copy link
Member

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"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, fixed :)

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
Copy link
Member

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
warn!("Redirecting the request to: {}", current_registry);
debug!("Redirecting the request to: {}", current_registry);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

));
}
if let Some(location_header) = response.headers().get(LOCATION) {
let location = location_header.to_str().unwrap();
Copy link
Member

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.

Copy link
Contributor Author

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

.mount(&mock_server)
.await;

let group = {
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@dijor0310 dijor0310 force-pushed the diyor/add-redirect-uv-publish branch 2 times, most recently from c94cf68 to 3be2223 Compare December 15, 2025 22:20
@dijor0310
Copy link
Contributor Author

@konstin, added a check that the new URL is in the same realm:

...
if Realm::from(&current_registry) == Realm::from(registry) {
    debug!("Redirecting the request to: {}", current_registry);
    n_past_redirections += 1;
    continue;
}
...

@dijor0310 dijor0310 force-pushed the diyor/add-redirect-uv-publish branch from 3be2223 to 100be27 Compare December 15, 2025 22:34
@dijor0310 dijor0310 force-pushed the diyor/add-redirect-uv-publish branch from 100be27 to 424ba0b Compare December 15, 2025 23:20
@konstin
Copy link
Member

konstin commented Dec 16, 2025

Thanks for fixing!

I switched to explicit error types and added a test for infinite redirects.

@konstin
Copy link
Member

konstin commented Dec 16, 2025

I also added a test for the realm switch, since we consider that a security feature.

@konstin konstin enabled auto-merge (squash) December 16, 2025 08:49
@konstin konstin merged commit b58f543 into astral-sh:main Dec 16, 2025
162 checks passed
@dijor0310
Copy link
Contributor Author

Thanks for merging, @konstin!

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Dec 18, 2025
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 ([#&#8203;17080](astral-sh/uv#17080))
- Improve error handling in `uv publish` ([#&#8203;17096](astral-sh/uv#17096))
- Improve rendering of multiline error messages ([#&#8203;17132](astral-sh/uv#17132))
- Support redirects in `uv publish` ([#&#8203;17130](astral-sh/uv#17130))
- Include Docker images with the alpine version, e.g., `python3.x-alpine3.23` ([#&#8203;17100](astral-sh/uv#17100))

##### Configuration

- Accept `--torch-backend` in `[tool.uv]` ([#&#8203;17116](astral-sh/uv#17116))

##### Performance

- Speed up `uv cache size` ([#&#8203;17015](astral-sh/uv#17015))
- Initialize S3 signer once ([#&#8203;17092](astral-sh/uv#17092))

##### Bug fixes

- Avoid panics due to reads on failed requests ([#&#8203;17098](astral-sh/uv#17098))
- Enforce latest-version in `@latest` requests ([#&#8203;17114](astral-sh/uv#17114))
- Explicitly set `EntryType` for file entries in tar ([#&#8203;17043](astral-sh/uv#17043))
- Ignore `pyproject.toml` index username in lockfile comparison ([#&#8203;16995](astral-sh/uv#16995))
- Relax error when using `uv add` with `UV_GIT_LFS` set ([#&#8203;17127](astral-sh/uv#17127))
- Support file locks on ExFAT on macOS ([#&#8203;17115](astral-sh/uv#17115))
- Change schema for `exclude-newer` into optional string ([#&#8203;17121](astral-sh/uv#17121))

##### Documentation

- Drop arm musl caveat from Docker documentation ([#&#8203;17111](astral-sh/uv#17111))
- Fix version reference in resolver example ([#&#8203;17085](astral-sh/uv#17085))
- Better documentation for `exclude-newer*` ([#&#8203;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-->
keestux pushed a commit to keestux/uv that referenced this pull request Dec 18, 2025
<!--
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or improvement to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

uv publish fails on 308 Permanent Redirect

2 participants