search

hyperium / http

Rust HTTP types
Apache License 2.0
1.14k stars 284 forks source link

Should http::Uri handle IDNA? #259

Open KiChjang opened 5 years ago

KiChjang commented 5 years ago

Or should it delegate it to the url crate somehow?

seanmonstar commented 5 years ago

That's a good question. What would be involved if we were to say that this crate should handle it? What are the use cases? What should this crate do if it were decided to not handle IDNA?

I don't believe a server needs to handle this, as the recurved URI should have been encoded correctly. So is it mostly for clients?

troelsarvin commented 2 years ago

See https://github.com/sagebind/isahc/discussions/381 and related https://github.com/sagebind/isahc/issues/382 for why this is relevant. I like to use isahc, but as it is, isahc's reliance on http::Uri has turned out to be a problem, and I probably need to convert URL strings into the url::Url and then further into http://Uri to be able to handle many perfect perfectly fine real-life URLs.

Non-ASCII domain names become more and more wide-spread, and for a good reason.

So I hope someone can find a good way to accept parse non-ASCII strings as Uris.

Xuanwo commented 2 years ago

I try to fix this issue via allow unicode in uri: https://github.com/hyperium/http/pull/565

PTAL.

troelsarvin commented 1 year ago

@Xuanwo , it seems to me your pull request was closed but never accepted into the http crate, or am I misunderstanding Github?

Xuanwo commented 1 year ago

@Xuanwo , it seems to me your pull request was closed but never accepted into the http crate, or am I misunderstanding Github?

Yes. I gave up for working on fixing this issue any more. Feel free to reuse my work.

micolous commented 9 months ago

It looks like #565 didn't implement Nameprep (Unicode "lowercasing" and normalization), so that's not a complete implementation of IDN for some languages.

Implementing Nameprep means shipping a Unicode normalization table, which has significant impacts on binary size. Some platforms have ways of dealing with this (wasm32 targets can use JavaScript's Url object, Windows has its own IDN APIs and CoreFoundation has a CFURL type), but they all make things a bit complicated.

reqwest currently uses url:Url for its public APIs but http::Uri internally on non-wasm32 targets. Adding IDN support to http::Uri would mean that they no longer have to use url::Url, but they'd have to break API compatibility for non-wasm32 targets as well.

Having http::Request use a generic Url-like trait make things more flexible – on wasm32, one could use web_sys::Url which is much smaller than url::Url (with idna and unicode_normalization crates).

kirawi commented 2 months ago

@seanmonstar Would you be open to a PR for this?

flisky commented 2 months ago

@kirawi actually, @micolous opened a PR https://github.com/servo/rust-url/pull/887

kirawi commented 2 months ago

I don't think that's what this issue is about. AFAICT, this issue is about adding IDNA support for the Uri implementation in http. The url crate already implements it and that PR seems to be an optimization for webassembly.