Skip to content

fix: preserve mirror URL path when rewriting requests#2183

Open
mohitdebian wants to merge 7 commits intoconda:mainfrom
mohitdebian:fix-mirror-channel-1283
Open

fix: preserve mirror URL path when rewriting requests#2183
mohitdebian wants to merge 7 commits intoconda:mainfrom
mohitdebian:fix-mirror-channel-1283

Conversation

@mohitdebian
Copy link

@mohitdebian mohitdebian commented Mar 6, 2026

Description

Fix mirror URL rewriting when the mirror URL contains a path component.

Previously the mirror middleware used Url::join() when constructing mirrored request URLs.
Because Url::join() follows RFC3986 semantics, the last path segment of the base URL can be
treated as a file when the base URL does not end with a trailing slash.

This caused mirror URLs such as https://my-custom-url/channel to lose their path during rewriting.

Example

Upstream request

https://prefix.dev/conda-forge/linux-64/k9s-0.40.5.conda

Mirror configuration

[mirrors]
"https://my-custom-url/channel" = ["https://prefix.dev/conda-forge"]

Incorrect result

https://my-custom-url/linux-64/k9s-0.40.5.conda

Expected result

https://my-custom-url/channel/linux-64/k9s-0.40.5.conda

Fixes #1283

How Has This Been Tested?

  • Ran networking tests locally:
  cargo test -p rattler_networking
  • Verified that all CI checks pass across supported platforms.

AI Disclosure

  • This PR contains AI-generated content.
  • I have tested any AI-generated content in my PR.
  • I take responsibility for any AI-generated content in my PR.

Tools: N/A

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added sufficient tests to cover my changes.

@mohitdebian
Copy link
Author

This PR should address the mirror path rewriting issue described in #1283.

The change replaces Url::join() with explicit path concatenation so the mirror base path is preserved when rewriting requests (e.g. https://my-custom-url/channel/...).

  • All rattler_networking tests pass locally.
  • CI checks are green across platforms.

Please let me know if any adjustments are needed

@pavelzw
Copy link
Member

pavelzw commented Mar 8, 2026

Please fill in the pr template

@mohitdebian
Copy link
Author

I've updated the pr description to follow the template. Let me know if anything else needs adjustment

@pavelzw
Copy link
Member

pavelzw commented Mar 10, 2026

### AI Disclosure
<!--- Remove this section if your PR does not contain AI-generated content. --->
- [ ] This PR contains AI-generated content.
- [ ] I have tested any AI-generated content in my PR.
- [ ] I take responsibility for any AI-generated content in my PR.
<!--- If you used AI to generate code, please specify the tool used and the prompt below. --->
Tools: {e.g., Claude, Codex, GitHub Copilot, ChatGPT, etc.}

@pavelzw
Copy link
Member

pavelzw commented Mar 10, 2026

sorry for being unclear earlier, i updated our pr template to reflect this better: #2204

Replace Url::join() with manual path concatenation in the mirror
middleware. Url::join() follows RFC 3986 and replaces the last path
segment of the base URL instead of appending when no trailing slash
is present.

This caused mirror URLs with path components (e.g.
https://my-custom-url/channel) to lose their path during rewriting.

Fixes conda#1283
Replace `Url::join()` with explicit path concatenation in the mirror
middleware. `Url::join()` follows RFC3986 relative resolution which
replaces the last path segment when the base URL has no trailing slash.

This caused mirror URLs with path components (e.g.
https://my-custom-url/channel) to lose their path during rewriting.

The new implementation preserves the mirror base path and safely
appends the remaining request path.

Adds a regression test verifying correct path rewriting.
@mohitdebian mohitdebian force-pushed the fix-mirror-channel-1283 branch from 6d3b35b to be4e008 Compare March 11, 2026 13:50
features = ["gcs", "s3"]

[dependencies]
rattler_conda_types = { workspace = true }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mm sucks a little bit that we have to add this dependency just to get to that type. Maybe we should not do it and use your previous fix instead. Sorry for the noise.

Copy link
Author

Choose a reason for hiding this comment

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

Mm sucks a little bit that we have to add this dependency just to get to that type. Maybe we should not do it and use your previous fix instead. Sorry for the noise.

no worries about the noise i agree that adding a dependency just for that type might not be ideal if you'd prefer the previous implementation i can revert the change and update the pr

@baszalmstra baszalmstra changed the title fix(networking): preserve mirror URL path when rewriting requests (#1283) fix: preserve mirror URL path when rewriting requests Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mirror middleware issues

3 participants