http icon indicating copy to clipboard operation
http copied to clipboard

Fix case sensitivity issues w/ Uri

Open carllerche opened this issue 8 years ago • 5 comments
trafficstars

Scheme characters must be lower case, but implementations should tolerate and normalize upper case characters to lower case.

The host component is case insensitive.

The various Uri types should provide PartialEq / Hash implementations that handle the necessary case sensitivity logic.

carllerche avatar Jul 06 '17 18:07 carllerche

@carllerche Could you provide an example?

It may seem like a dumb question, but it seems to me like the scheme parsing is already normalizing case characters.

I actually added a test to check that and it runs just fine:

test_parse! {
    test_scheme_case_insensitive,
    "HTTPS://127.0.0.1",
    [],
    scheme = Some("https"),
    authority = Some("127.0.0.1"),
    path = "/",
    query = None,
    port = None,
}

Maybe I'm missing something :)

JJayet avatar Aug 08 '17 22:08 JJayet

I don't remember exactly the issue. I remember seeing something that wasn't correct in terms of treating case sensitivity. I'd have to compare the spec for each component against what the code does today.

carllerche avatar Aug 10 '17 15:08 carllerche

I'm going to remove the milestone as bug fixes can happen after 0.1... but it would still be a nice to have!

carllerche avatar Aug 12 '17 03:08 carllerche

Hi, I was looking for a "Good first issue" and I chose this one to get myself into the project.

@JJayet indeed it works fine on HTTP and HTTPS, but not on Other schemes.

""" Scheme characters must be lower case, but implementations should tolerate and normalize upper case characters to lower case. """ @carllerche You are right about this if we look at rfc : https://tools.ietf.org/html/rfc3986#section-3.1 PartialEq / Hash seems to be already implemented for Scheme struct.

I think that the missing point is that for Other(T) scheme, the str is not normalized to lower case:

#[test]
fn test_http_scheme() {
    use http::Uri;

    let uri = "FILE://www.rust-lang.org/index.html".parse::<Uri>().unwrap();

    assert_eq!(uri.scheme_str(), Some("file")); <== fail here
    assert_eq!(uri.host(), Some("www.rust-lang.org"));
    assert_eq!(uri.path(), "/index.html");
    assert_eq!(uri.query(), None);
}

@carllerche Is that what you were looking for ?

Edit: Otherwise comparaison seems to be working, the following test pass

test_parse! {
    test_scheme_other_comparaison,
    "FILE://127.0.0.1:61761/chunks",
    [],

    scheme_part = part!("file"),
    authority_part = part!("127.0.0.1:61761"),
    path = "/chunks",
    query = None,
    host = Some("127.0.0.1"),
    port_part = Port::from_str("61761").ok(),
}

Need-To-Learn avatar Jul 29 '19 20:07 Need-To-Learn

One place that does have a case sensitivity issue is in directly parsing a Scheme. For example

let scheme: Scheme = "hTTp".parse().unwrap();

does not canonicalize the "hTTp" to lower case.

The following example show that Uri parsing of the scheme works differently than direct parsing the Scheme in a way that is unintuitive:

let uri: Uri = "hTTp://example.com".parse().expect("uri invalid");
let scheme1 = uri.scheme().expect("no scheme on uri");
    
let scheme2: Scheme = "hTTp".parse().expect("scheme2 invalid");

assert_eq!(scheme1, &scheme2, "schemes not equal");

This triggers the assert_eq!() with left: "http" and right: "hTTp" (on the Playground).

sbosnick avatar Apr 18 '20 21:04 sbosnick