From 4f5a83ba9c61ca448ad38362711c5804a2db7e86 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Wed, 3 May 2023 17:36:47 -0700 Subject: [PATCH] Fix missing port parsing in 'Authority'. If a port part was missing, the 'Authority' parser previously set the port to `0`. This is incorrect. As in RFC#3986 3.2.3: > URI producers and normalizers should omit the port component and its ":" delimiter if port is empty [..] This commit fixes the parser's behavior to align with the RFC. --- core/http/src/parse/uri/parser.rs | 4 +++- core/http/src/parse/uri/tests.rs | 12 ++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/core/http/src/parse/uri/parser.rs b/core/http/src/parse/uri/parser.rs index e9438da8..d7bac58b 100644 --- a/core/http/src/parse/uri/parser.rs +++ b/core/http/src/parse/uri/parser.rs @@ -167,7 +167,9 @@ fn port<'a>( // current - bytes.len(). Or something like that. #[parser] fn maybe_port<'a>(input: &mut RawInput<'a>, bytes: &[u8]) -> Result<'a, Option> { - if bytes.len() > 5 { + if bytes.is_empty() { + return Ok(None); + } else if bytes.len() > 5 { parse_error!("port len is out of range")?; } else if !bytes.iter().all(|b| b.is_ascii_digit()) { parse_error!("invalid port bytes")?; diff --git a/core/http/src/parse/uri/tests.rs b/core/http/src/parse/uri/tests.rs index 0f8a262e..7106e4b2 100644 --- a/core/http/src/parse/uri/tests.rs +++ b/core/http/src/parse/uri/tests.rs @@ -139,7 +139,7 @@ fn single_byte() { "%" => Authority::new(None, "%", None), "?" => Reference::new(None, None, "", "", None), "#" => Reference::new(None, None, "", None, ""), - ":" => Authority::new(None, "", 0), + ":" => Authority::new(None, "", None), "@" => Authority::new("", "", None), ); @@ -169,13 +169,13 @@ fn origin() { #[test] fn authority() { assert_parse_eq!( - "@:" => Authority::new("", "", 0), + "@:" => Authority::new("", "", None), "abc" => Authority::new(None, "abc", None), "@abc" => Authority::new("", "abc", None), "a@b" => Authority::new("a", "b", None), "a@" => Authority::new("a", "", None), ":@" => Authority::new(":", "", None), - ":@:" => Authority::new(":", "", 0), + ":@:" => Authority::new(":", "", None), "sergio:benitez@spark" => Authority::new("sergio:benitez", "spark", None), "a:b:c@1.2.3:12121" => Authority::new("a:b:c", "1.2.3", 12121), "sergio@spark" => Authority::new("sergio", "spark", None), @@ -183,7 +183,7 @@ fn authority() { "sergio@[1::]:230" => Authority::new("sergio", "[1::]", 230), "rocket.rs:8000" => Authority::new(None, "rocket.rs", 8000), "[1::2::3]:80" => Authority::new(None, "[1::2::3]", 80), - "bar:" => Authority::new(None, "bar", 0), // could be absolute too + "bar:" => Authority::new(None, "bar", None), // could be absolute too ); } @@ -227,7 +227,7 @@ fn absolute() { "git://:@rocket.rs:443/abc?q" => Absolute::new("git", Authority::new(":", "rocket.rs", 443), "/abc", "q"), "a://b?test" => Absolute::new("a", Authority::new(None, "b", None), "", "test"), - "a://b:?test" => Absolute::new("a", Authority::new(None, "b", 0), "", "test"), + "a://b:?test" => Absolute::new("a", Authority::new(None, "b", None), "", "test"), "a://b:1?test" => Absolute::new("a", Authority::new(None, "b", 1), "", "test"), }; } @@ -256,7 +256,7 @@ fn reference() { "a:/?a#b" => Reference::new("a", None, "/", "a", "b"), "a:?a#b" => Reference::new("a", None, "", "a", "b"), "a://?a#b" => Reference::new("a", Authority::new(None, "", None), "", "a", "b"), - "a://:?a#b" => Reference::new("a", Authority::new(None, "", 0), "", "a", "b"), + "a://:?a#b" => Reference::new("a", Authority::new(None, "", None), "", "a", "b"), "a://:2000?a#b" => Reference::new("a", Authority::new(None, "", 2000), "", "a", "b"), "a://a:2000?a#b" => Reference::new("a", Authority::new(None, "a", 2000), "", "a", "b"), "a://a:@2000?a#b" => Reference::new("a", Authority::new("a:", "2000", None), "", "a", "b"),