Skip to content

Commit

Permalink
Reject if the HTTP header contains duplicated Content-Length values.
Browse files Browse the repository at this point in the history
  • Loading branch information
eaufavor committed Feb 22, 2025
1 parent e823044 commit eef3576
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 1 deletion.
2 changes: 1 addition & 1 deletion .bleep
Original file line number Diff line number Diff line change
@@ -1 +1 @@
c21de982c54c255ff1893d71795ec08cf1928ade
595e312ee6c5ab80b34cae937ff818ae880eccd2
15 changes: 15 additions & 0 deletions pingora-core/src/protocols/http/v1/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,20 @@ impl HttpSession {
Ok(res)
}

// Validate the response header read. This function must be called after the response header
// read.
fn validate_response(&self) -> Result<()> {
let resp_header = self
.response_header
.as_ref()
.expect("response header must be read");

// ad-hoc checks
super::common::check_dup_content_length(&resp_header.headers)?;

Ok(())
}

/// Read the response header from the server
/// This function can be called multiple times, if the headers received are just informational
/// headers.
Expand Down Expand Up @@ -287,6 +301,7 @@ impl HttpSession {
self.buf = buf;
self.upgraded = self.is_upgrade(&response_header).unwrap_or(false);
self.response_header = Some(response_header);
self.validate_response()?;
return Ok(s);
}
HeaderParseState::Partial => { /* continue the loop */ }
Expand Down
49 changes: 49 additions & 0 deletions pingora-core/src/protocols/http/v1/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use http::{header, HeaderValue};
use log::warn;
use pingora_error::Result;
use pingora_http::{HMap, RequestHeader, ResponseHeader};
use std::str;
use std::time::Duration;
Expand Down Expand Up @@ -243,3 +244,51 @@ pub(super) fn populate_headers(
}
used_header_index
}

// RFC 7230:
// If a message is received without Transfer-Encoding and with
// either multiple Content-Length header fields having differing
// field-values or a single Content-Length header field having an
// invalid value, then the message framing is invalid and the
// recipient MUST treat it as an unrecoverable error.
pub(super) fn check_dup_content_length(headers: &HMap) -> Result<()> {
if headers.get(header::TRANSFER_ENCODING).is_some() {
// If TE header, ignore CL
return Ok(());
}
let mut cls = headers.get_all(header::CONTENT_LENGTH).into_iter();
if cls.next().is_none() {
// no CL header is fine.
return Ok(());
}
if cls.next().is_some() {
// duplicated CL is bad
return crate::Error::e_explain(
crate::ErrorType::InvalidHTTPHeader,
"duplicated Content-Length header",
);
}
Ok(())
}

#[cfg(test)]
mod test {
use super::*;
use http::header::{CONTENT_LENGTH, TRANSFER_ENCODING};

#[test]
fn test_check_dup_content_length() {
let mut headers = HMap::new();

assert!(check_dup_content_length(&headers).is_ok());

headers.append(CONTENT_LENGTH, "1".try_into().unwrap());
assert!(check_dup_content_length(&headers).is_ok());

headers.append(CONTENT_LENGTH, "2".try_into().unwrap());
assert!(check_dup_content_length(&headers).is_err());

headers.append(TRANSFER_ENCODING, "chunkeds".try_into().unwrap());
assert!(check_dup_content_length(&headers).is_ok());
}
}
12 changes: 12 additions & 0 deletions pingora-core/src/protocols/http/v1/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ impl HttpSession {
self.body_reader.reinit();
self.response_written = None;
self.respect_keepalive();
self.validate_request()?;

return Ok(Some(s));
}
Expand Down Expand Up @@ -287,6 +288,17 @@ impl HttpSession {
}
}

// Validate the request header read. This function must be called after the request header
// read.
fn validate_request(&self) -> Result<()> {
let req_header = self.req_header();

// ad-hoc checks
super::common::check_dup_content_length(&req_header.headers)?;

Ok(())
}

/// Return a reference of the `RequestHeader` this session read
/// # Panics
/// this function and most other functions will panic if called before [`Self::read_request()`]
Expand Down

0 comments on commit eef3576

Please sign in to comment.