Skip to content

Commit

Permalink
Merge pull request #753 from lucacasonato/ipv4_parsing_fix
Browse files Browse the repository at this point in the history 
Align IPv4 parsing to spec
  • Loading branch information
@valenting
valenting committed Jan 31, 2022
2 parents 0c7a07f + d40c4f1 commit 8e3e91d
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 32 deletions.
70 changes: 40 additions & 30 deletions url/src/host.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ impl Host<String> {

if domain.find(is_invalid_domain_char).is_some() {
Err(ParseError::InvalidDomainCharacter)
} else if let Some(address) = parse_ipv4addr(&domain)? {
} else if ends_in_a_number(&domain) {
let address = parse_ipv4addr(&domain)?;
Ok(Host::Ipv4(address))
} else {
Ok(Host::Domain(domain))
Expand Down Expand Up @@ -264,8 +265,33 @@ fn longest_zero_sequence(pieces: &[u16; 8]) -> (isize, isize) {
}
}

/// <https://url.spec.whatwg.org/#ends-in-a-number-checker>
fn ends_in_a_number(input: &str) -> bool {
let mut parts = input.rsplit('.');
let last = parts.next().unwrap();
let last = if last.is_empty() {
if let Some(last) = parts.next() {
last
} else {
return false;
}
} else {
last
};
if !last.is_empty() && last.chars().all(|c| ('0'..='9').contains(&c)) {
return true;
}

parse_ipv4number(last).is_ok()
}

/// <https://url.spec.whatwg.org/#ipv4-number-parser>
/// Ok(None) means the input is a valid number, but it overflows a `u32`.
fn parse_ipv4number(mut input: &str) -> Result<Option<u32>, ()> {
if input.is_empty() {
return Err(());
}

let mut r = 10;
if input.starts_with("0x") || input.starts_with("0X") {
input = &input[2..];
Expand All @@ -275,10 +301,10 @@ fn parse_ipv4number(mut input: &str) -> Result<Option<u32>, ()> {
r = 8;
}

// At the moment we can't know the reason why from_str_radix fails
// https://github.com/rust-lang/rust/issues/22639
// So instead we check if the input looks like a real number and only return
// an error when it's an overflow.
if input.is_empty() {
return Ok(Some(0));
}

let valid_number = match r {
8 => input.chars().all(|c| ('0'..='7').contains(&c)),
10 => input.chars().all(|c| ('0'..='9').contains(&c)),
Expand All @@ -287,50 +313,34 @@ fn parse_ipv4number(mut input: &str) -> Result<Option<u32>, ()> {
}),
_ => false,
};

if !valid_number {
return Ok(None);
return Err(());
}

if input.is_empty() {
return Ok(Some(0));
}
if input.starts_with('+') {
return Ok(None);
}
match u32::from_str_radix(input, r) {
Ok(number) => Ok(Some(number)),
Err(_) => Err(()),
Ok(num) => Ok(Some(num)),
Err(_) => Ok(None), // The only possible error kind here is an integer overflow.
// The validity of the chars in the input is checked above.
}
}

/// <https://url.spec.whatwg.org/#concept-ipv4-parser>
fn parse_ipv4addr(input: &str) -> ParseResult<Option<Ipv4Addr>> {
if input.is_empty() {
return Ok(None);
}
fn parse_ipv4addr(input: &str) -> ParseResult<Ipv4Addr> {
let mut parts: Vec<&str> = input.split('.').collect();
if parts.last() == Some(&"") {
parts.pop();
}
if parts.len() > 4 {
return Ok(None);
return Err(ParseError::InvalidIpv4Address);
}
let mut numbers: Vec<u32> = Vec::new();
let mut overflow = false;
for part in parts {
if part.is_empty() {
return Ok(None);
}
match parse_ipv4number(part) {
Ok(Some(n)) => numbers.push(n),
Ok(None) => return Ok(None),
Err(()) => overflow = true,
Ok(None) => return Err(ParseError::InvalidIpv4Address), // u32 overflow
Err(()) => return Err(ParseError::InvalidIpv4Address),
};
}
if overflow {
return Err(ParseError::InvalidIpv4Address);
}
let mut ipv4 = numbers.pop().expect("a non-empty list of numbers");
// Equivalent to: ipv4 >= 256 ** (4 − numbers.len())
if ipv4 > u32::max_value() >> (8 * numbers.len() as u32) {
Expand All @@ -342,7 +352,7 @@ fn parse_ipv4addr(input: &str) -> ParseResult<Option<Ipv4Addr>> {
for (counter, n) in numbers.iter().enumerate() {
ipv4 += n << (8 * (3 - counter as u32))
}
Ok(Some(Ipv4Addr::from(ipv4)))
Ok(Ipv4Addr::from(ipv4))
}

/// <https://url.spec.whatwg.org/#concept-ipv6-parser>
Expand Down
4 changes: 2 additions & 2 deletions url/tests/unit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@ fn host() {
0x2001, 0x0db8, 0x85a3, 0x08d3, 0x1319, 0x8a2e, 0x0370, 0x7344,
)),
);
assert_host("http://1.35.+33.49", Host::Domain("1.35.+33.49"));
assert_host(
"http://[::]",
Host::Ipv6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0)),
Expand All @@ -271,7 +270,8 @@ fn host() {
);
assert_host("http://0x1232131", Host::Ipv4(Ipv4Addr::new(1, 35, 33, 49)));
assert_host("http://111", Host::Ipv4(Ipv4Addr::new(0, 0, 0, 111)));
assert_host("http://2..2.3", Host::Domain("2..2.3"));
assert!(Url::parse("http://1.35.+33.49").is_err());
assert!(Url::parse("http://2..2.3").is_err());
assert!(Url::parse("http://42.0x1232131").is_err());
assert!(Url::parse("http://192.168.0.257").is_err());

Expand Down
160 changes: 160 additions & 0 deletions url/tests/urltestdata.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7744,5 +7744,165 @@
"input": "?",
"base": null,
"failure": true
},
"Last component looks like a number, but not valid IPv4",
{
"input": "http://1.2.3.4.5",
"base": "http://other.com/",
"failure": true
},
{
"input": "http://1.2.3.4.5.",
"base": "http://other.com/",
"failure": true
},
{
"input": "http://0..0x300/",
"base": "about:blank",
"failure": true
},
{
"input": "http://0..0x300./",
"base": "about:blank",
"failure": true
},
{
"input": "http://256.256.256.256.256",
"base": "http://other.com/",
"failure": true
},
{
"input": "http://256.256.256.256.256.",
"base": "http://other.com/",
"failure": true
},
{
"input": "http://1.2.3.08",
"base": "about:blank",
"failure": true
},
{
"input": "http://1.2.3.08.",
"base": "about:blank",
"failure": true
},
{
"input": "http://1.2.3.09",
"base": "about:blank",
"failure": true
},
{
"input": "http://09.2.3.4",
"base": "about:blank",
"failure": true
},
{
"input": "http://09.2.3.4.",
"base": "about:blank",
"failure": true
},
{
"input": "http://01.2.3.4.5",
"base": "about:blank",
"failure": true
},
{
"input": "http://01.2.3.4.5.",
"base": "about:blank",
"failure": true
},
{
"input": "http://0x100.2.3.4",
"base": "about:blank",
"failure": true
},
{
"input": "http://0x100.2.3.4.",
"base": "about:blank",
"failure": true
},
{
"input": "http://0x1.2.3.4.5",
"base": "about:blank",
"failure": true
},
{
"input": "http://0x1.2.3.4.5.",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.1.2.3.4",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.1.2.3.4.",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.2.3.4",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.2.3.4.",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.09",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.09.",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.0x4",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.0x4.",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.09..",
"base": "about:blank",
"hash": "",
"host": "foo.09..",
"hostname": "foo.09..",
"href":"http://foo.09../",
"password": "",
"pathname": "/",
"port":"",
"protocol": "http:",
"search": "",
"username": ""
},
{
"input": "http://0999999999999999999/",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.0x",
"base": "about:blank",
"failure": true
},
{
"input": "http://foo.0XFfFfFfFfFfFfFfFfFfAcE123",
"base": "about:blank",
"failure": true
},
{
"input": "http://💩.123/",
"base": "about:blank",
"failure": true
}
]

0 comments on commit 8e3e91d

Please sign in to comment.