Skip to content

Cannot compile url 2.5.4 with rustc 1.80.1 #1032

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

Closed
heaths opened this issue Mar 6, 2025 · 6 comments
Closed

Cannot compile url 2.5.4 with rustc 1.80.1 #1032

heaths opened this issue Mar 6, 2025 · 6 comments

Comments

@heaths
Copy link

heaths commented Mar 6, 2025

While this seems similar to #992, the resolution wouldn't be. We link to url v2.5.4 directly and if we turn off the default feature, we lose URL encoding. We probably don't need IDNA but I can't turn that off because it's not it's own feature.

What's actually breaking for us is litemap v0.7.4 and zerofrom v0.1.5:

error: rustc 1.80.1 is not supported by the following packages:
  [email protected] requires rustc 1.81
  [email protected] requires rustc 1.81
Either upgrade rustc or select compatible dependency versions with
`cargo update <name>@<current-ver> --precise <compatible-ver>`
where `<compatible-ver>` is the latest version supporting rustc 1.80.1

We're not ready to bump our MSRV yet - not for dependencies we likely don't need. Would it be possible to declare a separate feature so we can use default-features = false, features = ["percent-encoded","form-urlencoded"]?

Some files of note:

@Manishearth
Copy link
Member

Manishearth commented Mar 7, 2025

Could you downgrade your litemap and zerofrom dependencies? rust-url does not need the latest versions.

unicode-org/icu4x#6192 (comment) has ICU4X's position on MSRV bumps. I don't think rust-url needs a different position here: Do not rely on dependencies not making MSRV changes within the same version stream, rust-url's guarantee is that there is some combination of published crates that is compatible with it on its MSRV. rust-url isn't planning on guaranteeing that all versions in a dependency's major version stream stick to the MSRV: that's just not possible.

The new Cargo resolver also makes this problem moot. It's probably not available yet on your MSRV, but it means that if you need to pin things in your lockfile it'll only be temporary.

@Manishearth Manishearth closed this as not planned Won't fix, can't repro, duplicate, stale Mar 7, 2025
@Manishearth
Copy link
Member

Basically, in this PR: Azure/azure-sdk-for-rust#2294, just don't bump these dependencies. It is not a norm followed by the ecosystem that minor version bumps keep MSRV the same; if you care about MSRV then cargo update -p the crate and update it later. both ICU4X and rust-url have had to do this on occasion.

(ICU4X in particular just doesn't run cargo update; it bumps deps in Cargo.toml, if something new is needed it should be reflected in Cargo.toml)

@heaths
Copy link
Author

heaths commented Mar 7, 2025

I came to the same conclusion (before I saw this) and was just trying that now. :) What's odd is that our CI builds are passing - I just kicked off a manual run, but the PR builds are failing almost as if they aren't using the Cargo.lock file. I mean, we're not passing --locked and probably should, but something is different between the two runs I need to track down.

Still, IDNA pulls in a lot of dependencies that I don't think we need1. It could benefit our customers' build times.

Footnotes

  1. Azure services don't need it directly, but some services like Storage support custom domains so we may indeed need to support it. .NET's System.Uri does out of the box and I'm checking on other languages' IDNA support.

@Manishearth
Copy link
Member

I'm rather reluctant to make idna optional, disabling idna is a really easy way to get security issues since it affects how URL identity works.

(there are probably ways to make it such that "disable idna" leads to behavior that is more errorful instead of causing security issues, but it would need to be thought through quite thoroughly, looking at actual use cases. I don't think it's a trivial project)

@hsivonen
Copy link
Collaborator

hsivonen commented Mar 10, 2025

Still, IDNA pulls in a lot of dependencies that I don't think we need1. It could benefit our customers' build times.

Did you notice the link in the url and idna READMEs about 1) an option that trades away run-time performance and binary smallness for shorter build times and 2) an option to turn off proper IDNA processing?

there are probably ways to make it such that "disable idna" leads to behavior that is more errorful instead of causing security issues

I think the results of the "Turning off IDNA support" section of the above link are as good as they can logically be:

  • Non-ASCII input in the domain name becomes an error.
  • Punycode validity is still checked.
  • Punycode labels logically cannot be further validated for their decoded form meeting the spec's constraints when there's no back end providing Unicode Database -dependent operations.

@heaths
Copy link
Author

heaths commented Mar 10, 2025

Thanks. I didn't notice it in the way it more or less provides what I asked for. I didn't consider checking dependencies' READMEs. Seems odd taking a precise dependency on a particular version, but with a comment in our workspace's dependency I could hopefully mitigate future maintainers from updating it incidentally.

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

No branches or pull requests

3 participants