Skip to content

Commit

Permalink
Fix token and uri parsers to disallow empty results (#111)
Browse files Browse the repository at this point in the history
This fixes cases where the parser would accept non-compliant request lines including empty methods
and paths.

For the `token` grammar, [the spec][spec-token] is:

```
  token          = 1*tchar
```

`1*` is shorthand for one-or-more, so the empty string is not a valid `token`.

For the path component of the request line, [the spec][spec-path] we're concerned with the
`absolute-path` grammar:

```
  absolute-path = 1*( "/" segment )
```

While `segment` might be empty, there must be at least a `"/"` for it to be syntactically valid.

I've added tests for these cases and their combination, and had to update the expected error of one
of the existing URI tests which now fails sooner due to the empty path.

[spec-token]: https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html#tokens
[spec-path]: https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html#uri.references
  • Loading branch information
acfoltzer authored Mar 18, 2022
1 parent 6f6ff10 commit 5654959
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
33 changes: 33 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,12 @@ fn parse_reason<'a>(bytes: &mut Bytes<'a>) -> Result<&'a str> {

#[inline]
fn parse_token<'a>(bytes: &mut Bytes<'a>) -> Result<&'a str> {
let b = next!(bytes);
if !is_token(b) {
// First char must be a token char, it can't be a space which would indicate an empty token.
return Err(Error::Token);
}

loop {
let b = next!(bytes);
if b == b' ' {
Expand All @@ -675,6 +681,12 @@ fn parse_token<'a>(bytes: &mut Bytes<'a>) -> Result<&'a str> {

#[inline]
fn parse_uri<'a>(bytes: &mut Bytes<'a>) -> Result<&'a str> {
let b = next!(bytes);
if !is_uri_token(b) {
// First char must be a URI char, it can't be a space which would indicate an empty path.
return Err(Error::Token);
}

simd::match_uri_vectored(bytes);

loop {
Expand Down Expand Up @@ -1310,6 +1322,27 @@ mod tests {
|_r| {}
}

req! {
test_request_with_empty_method,
b" / HTTP/1.1\r\n\r\n",
Err(::Error::Token),
|_r| {}
}

req! {
test_request_with_empty_path,
b"GET HTTP/1.1\r\n\r\n",
Err(::Error::Token),
|_r| {}
}

req! {
test_request_with_empty_method_and_path,
b" HTTP/1.1\r\n\r\n",
Err(::Error::Token),
|_r| {}
}

macro_rules! res {
($name:ident, $buf:expr, |$arg:ident| $body:expr) => (
res! {$name, $buf, Ok(Status::Complete($buf.len())), |$arg| $body }
Expand Down
2 changes: 1 addition & 1 deletion tests/uri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ req! {
req! {
urltest_007,
b"GET foo.com HTTP/1.1\r\nHost: \r\n\r\n",
Err(Error::Version),
Err(Error::Token),
|_r| {}
}

Expand Down

0 comments on commit 5654959

Please sign in to comment.