Skip to content

Commit d358a4f

Browse files
author
bors-servo
authored
Auto merge of #414 - valenting:ipv4_overflow, r=SimonSapin
Fail IPv4 parsing when the number is overflowing Right now we don't fail when parsing URLs such as http://10000000000 Parsing returns an overflow, but since we don't differentiate between overflow and it's a number, we just treat it as a regular domain. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/414) <!-- Reviewable:end -->
2 parents 471cb94 + 4899458 commit d358a4f

File tree

2 files changed

+44
-9
lines changed

2 files changed

+44
-9
lines changed

src/host.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ fn longest_zero_sequence(pieces: &[u16; 8]) -> (isize, isize) {
329329
}
330330

331331
/// <https://url.spec.whatwg.org/#ipv4-number-parser>
332-
fn parse_ipv4number(mut input: &str) -> Result<u32, ()> {
332+
fn parse_ipv4number(mut input: &str) -> Result<Option<u32>, ()> {
333333
let mut r = 10;
334334
if input.starts_with("0x") || input.starts_with("0X") {
335335
input = &input[2..];
@@ -338,14 +338,30 @@ fn parse_ipv4number(mut input: &str) -> Result<u32, ()> {
338338
input = &input[1..];
339339
r = 8;
340340
}
341+
342+
// At the moment we can't know the reason why from_str_radix fails
343+
// https://github.com/rust-lang/rust/issues/22639
344+
// So instead we check if the input looks like a real number and only return
345+
// an error when it's an overflow.
346+
let valid_number = match r {
347+
8 => input.chars().all(|c| c >= '0' && c <='7'),
348+
10 => input.chars().all(|c| c >= '0' && c <='9'),
349+
16 => input.chars().all(|c| (c >= '0' && c <='9') || (c >='a' && c <= 'f') || (c >= 'A' && c <= 'F')),
350+
_ => false
351+
};
352+
353+
if !valid_number {
354+
return Ok(None);
355+
}
356+
341357
if input.is_empty() {
342-
return Ok(0);
358+
return Ok(Some(0));
343359
}
344360
if input.starts_with('+') {
345-
return Err(())
361+
return Ok(None);
346362
}
347363
match u32::from_str_radix(input, r) {
348-
Ok(number) => Ok(number),
364+
Ok(number) => Ok(Some(number)),
349365
Err(_) => Err(()),
350366
}
351367
}
@@ -363,15 +379,19 @@ fn parse_ipv4addr(input: &str) -> ParseResult<Option<Ipv4Addr>> {
363379
return Ok(None);
364380
}
365381
let mut numbers: Vec<u32> = Vec::new();
382+
let mut overflow = false;
366383
for part in parts {
367384
if part == "" {
368385
return Ok(None);
369386
}
370-
if let Ok(n) = parse_ipv4number(part) {
371-
numbers.push(n);
372-
} else {
373-
return Ok(None);
374-
}
387+
match parse_ipv4number(part) {
388+
Ok(Some(n)) => numbers.push(n),
389+
Ok(None) => return Ok(None),
390+
Err(()) => overflow = true
391+
};
392+
}
393+
if overflow {
394+
return Err(ParseError::InvalidIpv4Address);
375395
}
376396
let mut ipv4 = numbers.pop().expect("a non-empty list of numbers");
377397
// Equivalent to: ipv4 >= 256 ** (4 − numbers.len())

tests/urltestdata.json

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4858,6 +4858,11 @@
48584858
"search": "",
48594859
"hash": ""
48604860
},
4861+
{
4862+
"input": "http://10000000000",
4863+
"base": "http://other.com/",
4864+
"failure": true
4865+
},
48614866
{
48624867
"input": "http://10000000000.com",
48634868
"base": "http://other.com/",
@@ -4888,6 +4893,11 @@
48884893
"search": "",
48894894
"hash": ""
48904895
},
4896+
{
4897+
"input": "http://4294967296",
4898+
"base": "http://other.com/",
4899+
"failure": true
4900+
},
48914901
{
48924902
"input": "http://0xffffffff",
48934903
"base": "http://other.com/",
@@ -4903,6 +4913,11 @@
49034913
"search": "",
49044914
"hash": ""
49054915
},
4916+
{
4917+
"input": "http://0xffffffff1",
4918+
"base": "http://other.com/",
4919+
"failure": true
4920+
},
49064921
{
49074922
"input": "http://256.256.256.256",
49084923
"base": "http://other.com/",

0 commit comments

Comments
 (0)