Align IPv4 parsing to spec

url/src/host.rs

@@ -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))
@@ -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..];
@@ -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)),
@@ -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) {
@@ -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>

url/tests/unit.rs

@@ -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)),
@@ -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());
 

url/tests/urltestdata.json

@@ -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
  }
 ]