path_and_query allow path not starting with slash

[Stargateur]

using path_and_query():

Uri::builder()
   .scheme(Scheme::HTTP)
   .authority("localhost:8000")
   .path_and_query("hello/world")
   .build()

report no error, but rocket didn't accept it with:

Oct 12 19:24:35.028 DEBUG hyper::proto::h1::conn: parse error (invalid URI) with 115 bytes
Oct 12 19:24:35.028 DEBUG hyper::proto::h1::role: sending automatic response (400 Bad Request) for parse error

I believe this should be considered a bug either http or rocket, I needed to add a slash at the beginning "/hello/world"

[kallekankaanpaa]

I can confirm the URI built by the example is http://localhost:8000hello/world.

After reading a bit on the URI RFC it seems to be a bug since the RFC3986, section 3.3 says the when an authority is defined path should either start with / or be empty. If authority is undefined then the path can't start with //.

[kolbma]

https://github.com/SergioBenitez/Rocket/issues/1994

[lilyball]

I just hit this too. This is more general than Builder, you can do this too by writing something like

let mut parts = Uri::from_static("http://localhost:8000").into_parts();
parts.path_and_query = Some(PathAndQuery::from_static("hello/world"))
let uri = Uri::from_parts(parts).unwrap();
dbg!(uri); // http://localhost:8000hello/world

This is made even more confusing by the fact that PathAndQuery prints a leading slash (unless it starts with *), which means that stuffing a dbg!(&parts) in there shows

[src/main.rs:6] &parts = Parts {
    scheme: Some(
        "http",
    ),
    authority: Some(
        localhost:8000,
    ),
    path_and_query: Some(
        /hello/world,
    ),
    _priv: (),
}

I think what needs to happen is:

• PathAndQuery should stop adding in the / prefix when printing if it's not part of the path. Similarly as_str() should stop converting an empty value into "/". HTTP/1.1 message syntax requires a non-empty path, but URIs don't.
• When building a Uri (either with Builder or from Parts), the validation logic needs to ensure that the path is either empty or starts with a slash (see RFC 3986 §3.3 Path).
• There's other validation logic updates that should be done. RFC 3986 indicates that a relative URI (no scheme or authority) doesn't need to start with a slash, but in that case the first path component cannot contain a colon.
• Heck, right now a Uri with a scheme but no authority is treated as an error, but that should be legal as a URI. It also looks like printing a Uri always tacks on the // if there's a scheme, but again, if there's no authority then the // doesn't belong.
• Similarly a relative URI is allowed to look like //localhost/foo but this is also rejected.
• This "asterisk form" that's part of HTTP/1.1 message syntax is also handled oddly. PathAndQuery special-cases a leading * in its printing (though my above recommendations would stop doing that). I'm not sure offhand if there's any other explicit handling of this, but if there is, it probably should be removed. AFAICT the only time in which this is syntactically meaningful is when calculating an Effective Request URI, and even then, asterisk-form is only ever used with OPTIONS (and I don't see any current mechanism to join a request URI with the server configuration to generate an Effective URI anyway).

Or, more generally, http::uri should conform to RFC 3986. Any particular validation in the context of HTTP routing should be done elsewhere. In fact, I would argue that Request Targets (RFC 7230 §5.3, which is what the Uri type currently references) should be a separate type. It can be a wrapper around PathAndQuery that validates the particular forms involved, and there should be a facility by which this can be joined against a Uri using the Effective Request URI rules.

[robjtede]

related to #229