reuse url encoding from url crate, don't use separate percent-encoding

[klensy]

Reuse encoding from url, don't use separate percent-encoding.

[rustbot]

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

[klensy]

There is similar one use with percent-encoding

cargo/crates/crates-io/lib.rs

Lines 347 to 353 in ddfe87f

	pub fn search(&mut self, query: &str, limit: u32) -> Result<(Vec<Crate>, u32)> {
	let formatted_query = percent_encode(query.as_bytes(), NON_ALPHANUMERIC);
	let body = self.req(
	&format!("/crates?q={}&per_page={}", formatted_query, limit),
	None,
	Auth::Unauthorized,
	)?;

and url

cargo/crates/crates-io/lib.rs

Lines 509 to 513 in ddfe87f

	pub fn is_url_crates_io(url: &str) -> bool {
	Url::parse(url)
	.map(|u| u.host_str() == Some("crates.io"))
	.unwrap_or(false)
	}

but i don't have good (and simple) idea what to do.

[arlosi]

Thanks! The example in the crates-io is more difficult since it's a relative URL, and the Url crate can't handle that. The other example doesn't use URL encoding at all, so I think it's fine as-is.

@bors r+

[bors]

📌 Commit 37d429c has been approved by arlosi

It is now in the queue for this repository.

[bors]

⌛ Testing commit 37d429c with merge b6c7fbe...

[bors]

☀️ Test successful - checks-actions
Approved by: arlosi
Pushing b6c7fbe to master...

[Eh2406]

Are there tests for the encoding of +? I don't want to break existing use cases re: rust-lang/crates.io#4891

[klensy]

Are there tests for the encoding of +? I don't want to break existing use cases re: rust-lang/crates.io#4891

No, i don't see tests for that.

cargo search "hello+f" --limit 101
cargo run search "hello+f" --limit 101

both: (go to https://crates.io/search?q=hello%2Bf to see more)

but here the diff (For that usecase, new behavior for is actually more accurate?)

 cargo search "hello f" --limit 101
(go to https://crates.io/search?q=hello%20f to see more)

cargo run search "hello f" --limit 101
(go to https://crates.io/search?q=hello+f to see more)

[arlosi]

crates.io can handle both encodings of (space) in the query string (%20 and and +). + must always be encoded as %2B

I think there's a copy-paste error in your diff. Searching for "hello f" produces %20 on existing Cargo.

 cargo search "hello f" --limit 101
(go to https://crates.io/search?q=hello%20f to see more)

[Eh2406]

crates.io can handle both encodings of (space) in the query string (%20 and and +).

Yes.

+ must always be encoded as %2B

I think you have it backwards. https://static.crates.io/crates/libgit2-sys/libgit2-sys-0.12.25+1.3.0.crate works but https://static.crates.io/crates/libgit2-sys/libgit2-sys-0.12.25%2B1.3.0.crate dose not. This is a bug in crate.io, or really a bug in s3, but we can not start hitting it.

[Eh2406]

I did not notice that this was just for search. Please ignore my rant.

[arlosi]

This is a bug in crate.io, or really a bug in s3, but we can not start hitting it.

I think it's a bug in Cargo & crates.io. We should be encoding the + in the URL. However, fixing it would be a significant breaking change. The actual files in S3 are storing the + as a .

[Turbo87]

I think it's a bug in Cargo & crates.io. We should be encoding the + in the URL. However, fixing it would be a significant breaking change.

FWIW I looked into the situation today. I agree that it would be a breaking change for cargo, but I don't agree that it is a bug in cargo.

the myth that + needs to be encoded apparently comes from https://datatracker.ietf.org/doc/html/rfc1866#section-8.2.1, but that is only relevant for the application/x-www-form-urlencoded MIME type and for query parameters and
form submission request payloads. it is essentially irrelevant for URL paths from what I can tell.

I've started a more detailed write-up today, but it's still WIP.

[Turbo87]

for cross-referencing, I've posted my investigation results at rust-lang/crates.io#4891 (comment) :)