Hyper: Setters for `Uri`

[jimmycuadra]

I noticed hyper::Uri doesn't have any setter methods. It seems the only way to construct one is to parse it from a string or use Default.

I have a function that takes a Uri which is expected to have a scheme and authority, and adds path/query/fragment information to it. Currently, I'd have to do this by converting the input Uri to a string, manipulating it as a string, and then re-parsing it to a Uri. This seems error prone and involves fallible conversions, but if I was able to use a signature like this:

fn add_path_to_uri(uri: &mut Uri)

Then I'd be able to do statically-verifiable in-place modifications, like you can with the Url type from the url crate.

Would it be possible to add setters to the Uri type, or is there a reason it's so opaque?

[seanmonstar]

The reason was to start conservatively. I'm not against mutator methods on Uri, but I'd probably want to keep them pretty basic.

For now, it's possible to do this outside of hyper, such as making use of the url crate, so I feel that the urgency is lower, and we can focus on just getting it right.

[jimmycuadra]

I should have specified: The reason I'm using hyper's Uri instead of the url crate is because hyper::client::Request::new takes a hyper Uri, and there are no conversion traits implemented to convert from a url::Url and a hyper::Uri (without going through an intermediate string form.)

[seanmonstar]

That's correct. I meant that manipulating the string is possible with something like Url, and converting that into a Uri is just a parse after, which isn't actually that expensive, as it does much less then Url's parsing does.

[jimmycuadra]

Right, but if I'm using strings for URL manipulation it kinda defeats the purpose of having a nice type for them. Would you be amenable to a PR adding simple setters for path/scheme/authority/host/port/query? Or some subset of those?

[jimmycuadra]

Or a PR for adding impl From<url::Url> for hyper::Uri?

[jimmycuadra]

Still happy to submit code for this if given the go-ahead for any particular approach.

[seanmonstar]

As I said, I'm not against the idea, I just want to be conservative. Is there a minimal API that would help? Maybe a Uri::from_components(), with possible components being scheme, authority, path, query?

[jimmycuadra]

Is impl From<url::Url> for hyper::Uri inappropriate because Url can represent more values than are valid Uris? I see the reference to the HTTP spec in the docs that talks about the URI needing to be one of four specific forms. I was hoping to have an infallible way to convert from Url to Uri, but this doesn't actually seem possible. If it's going to be necessary to check for errors during the conversion anyway, maybe just converting the Url to a &str and parsing it again is the best we can do.

[seanmonstar]

No, it's the other way around. All Urls are valid Uris, but not all Uris are Urls. Example URIs that aren't URLs: *, google.com:443, /foo/bar. So it is semantically correct to have From<Url> for Uri. However, I'd like to remove the url crate from hyper's dependencies (currently only needs it for some percent decoding, but working to get that into a separate much smaller crate).

[bittrance]

I have a very similar use case to OP:s description, but setters has the problem that each would have to return Result<(), UriError> which would be rather cumbersome. I think I would prefer a builder:

let builder = UriBuilder::new("https", "example.com").path("/foo");
builder.add_query_param("key", "value");
let uri = builder.build().expect("Bad uri input");

.build() would of course be -> Result<Uri, UriError>.

How does that sound?

[seanmonstar]

setters has the problem that each would have to return Result<(), UriError>

Yes indeed, there is complexity to setters that made me omit them at first.

A builder could be a good way to go. It seems like it could be explored out of crate, and if it's a monumental improvement over calling Uri::from_str(url.as_str()), we could consider its inclusion.

[softprops]

Would love to see setters for encoding query parameters

[softprops]

And also a getter for query params pairs

[radix]

It would be nice to be able to easily append path segments, too.

[avranju]

The problem with using the url crate for building URLs from string fragments is that there doesn't seem to be an easy way to map the ParseError type from the url crate to the UriError type from hyper.

let base_path = "http://localhost";
let path = "/boo";
let uri: Result<Uri, UriError> = Url::parse(base_path)
    .and_then(|base| base.join(path))
    .map_err(|_| /* How to construct UriError here? */)
    .and_then(|url| url.as_str().parse());

Playground link.

The problem is that the ErrorKind enum that UriError uses is not public. Right now I unwrap as a workaround - but that feels wrong.

[seanmonstar]

Since hyper is replacing its own type for http::Uri, this issue isn't directly actionable here.