mirror of
https://github.com/cloudflare/pingora.git
synced 2024-09-20 02:31:35 +02:00
Add ability to ignore informational responses when proxying downstream
This commit is contained in:
parent
999e379064
commit
be97e35031
5 changed files with 145 additions and 19 deletions
2
.bleep
2
.bleep
|
@ -1 +1 @@
|
||||||
9b88d76089e0f81c67cb502422148d4d26d4977e
|
d112ca7464812ac6e575f46ec5d37b5da365dd82
|
|
@ -218,6 +218,19 @@ impl Session {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Sets whether we ignore writing informational responses downstream.
|
||||||
|
///
|
||||||
|
/// For HTTP/1.1 this is a noop if the response is Upgrade or Continue and
|
||||||
|
/// Expect: 100-continue was set on the request.
|
||||||
|
///
|
||||||
|
/// This is a noop for h2 because informational responses are always ignored.
|
||||||
|
pub fn set_ignore_info_resp(&mut self, ignore: bool) {
|
||||||
|
match self {
|
||||||
|
Self::H1(s) => s.set_ignore_info_resp(ignore),
|
||||||
|
Self::H2(_) => {} // always ignored
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Return a digest of the request including the method, path and Host header
|
/// Return a digest of the request including the method, path and Host header
|
||||||
// TODO: make this use a `Formatter`
|
// TODO: make this use a `Formatter`
|
||||||
pub fn request_summary(&self) -> String {
|
pub fn request_summary(&self) -> String {
|
||||||
|
|
|
@ -153,6 +153,14 @@ pub(super) fn is_upgrade_req(req: &RequestHeader) -> bool {
|
||||||
req.version == http::Version::HTTP_11 && req.headers.get(header::UPGRADE).is_some()
|
req.version == http::Version::HTTP_11 && req.headers.get(header::UPGRADE).is_some()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(super) fn is_expect_continue_req(req: &RequestHeader) -> bool {
|
||||||
|
req.version == http::Version::HTTP_11
|
||||||
|
// https://www.rfc-editor.org/rfc/rfc9110#section-10.1.1
|
||||||
|
&& req.headers.get(header::EXPECT).map_or(false, |v| {
|
||||||
|
v.as_bytes().eq_ignore_ascii_case(b"100-continue")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// Unlike the upgrade check on request, this function doesn't check the Upgrade or Connection header
|
// Unlike the upgrade check on request, this function doesn't check the Upgrade or Connection header
|
||||||
// because when seeing 101, we assume the server accepts to switch protocol.
|
// because when seeing 101, we assume the server accepts to switch protocol.
|
||||||
// In reality it is not common that some servers don't send all the required headers to establish
|
// In reality it is not common that some servers don't send all the required headers to establish
|
||||||
|
|
|
@ -74,6 +74,8 @@ pub struct HttpSession {
|
||||||
digest: Box<Digest>,
|
digest: Box<Digest>,
|
||||||
/// Minimum send rate to the client
|
/// Minimum send rate to the client
|
||||||
min_send_rate: Option<usize>,
|
min_send_rate: Option<usize>,
|
||||||
|
/// When this is enabled informational response headers will not be proxied downstream
|
||||||
|
ignore_info_resp: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl HttpSession {
|
impl HttpSession {
|
||||||
|
@ -109,6 +111,7 @@ impl HttpSession {
|
||||||
upgraded: false,
|
upgraded: false,
|
||||||
digest,
|
digest,
|
||||||
min_send_rate: None,
|
min_send_rate: None,
|
||||||
|
ignore_info_resp: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -388,6 +391,11 @@ impl HttpSession {
|
||||||
/// Write the response header to the client.
|
/// Write the response header to the client.
|
||||||
/// This function can be called more than once to send 1xx informational headers excluding 101.
|
/// This function can be called more than once to send 1xx informational headers excluding 101.
|
||||||
pub async fn write_response_header(&mut self, mut header: Box<ResponseHeader>) -> Result<()> {
|
pub async fn write_response_header(&mut self, mut header: Box<ResponseHeader>) -> Result<()> {
|
||||||
|
if header.status.is_informational() && self.ignore_info_resp(header.status.into()) {
|
||||||
|
debug!("ignoring informational headers");
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
|
||||||
if let Some(resp) = self.response_written.as_ref() {
|
if let Some(resp) = self.response_written.as_ref() {
|
||||||
if !resp.status.is_informational() || self.upgraded {
|
if !resp.status.is_informational() || self.upgraded {
|
||||||
warn!("Respond header is already sent, cannot send again");
|
warn!("Respond header is already sent, cannot send again");
|
||||||
|
@ -409,7 +417,7 @@ impl HttpSession {
|
||||||
header.insert_header(header::CONNECTION, connection_value)?;
|
header.insert_header(header::CONNECTION, connection_value)?;
|
||||||
}
|
}
|
||||||
|
|
||||||
if header.status.as_u16() == 101 {
|
if header.status == 101 {
|
||||||
// make sure the connection is closed at the end when 101/upgrade is used
|
// make sure the connection is closed at the end when 101/upgrade is used
|
||||||
self.set_keepalive(None);
|
self.set_keepalive(None);
|
||||||
}
|
}
|
||||||
|
@ -510,6 +518,18 @@ impl HttpSession {
|
||||||
(None, None)
|
(None, None)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn ignore_info_resp(&self, status: u16) -> bool {
|
||||||
|
// ignore informational response if ignore flag is set and it's not an Upgrade and Expect: 100-continue isn't set
|
||||||
|
self.ignore_info_resp && status != 101 && !(status == 100 && self.is_expect_continue_req())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_expect_continue_req(&self) -> bool {
|
||||||
|
match self.request_header.as_deref() {
|
||||||
|
Some(req) => is_expect_continue_req(req),
|
||||||
|
None => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn is_connection_keepalive(&self) -> Option<bool> {
|
fn is_connection_keepalive(&self) -> Option<bool> {
|
||||||
is_buf_keepalive(self.get_header(header::CONNECTION))
|
is_buf_keepalive(self.get_header(header::CONNECTION))
|
||||||
}
|
}
|
||||||
|
@ -824,6 +844,14 @@ impl HttpSession {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Sets whether we ignore writing informational responses downstream.
|
||||||
|
///
|
||||||
|
/// This is a noop if the response is Upgrade or Continue and
|
||||||
|
/// Expect: 100-continue was set on the request.
|
||||||
|
pub fn set_ignore_info_resp(&mut self, ignore: bool) {
|
||||||
|
self.ignore_info_resp = ignore;
|
||||||
|
}
|
||||||
|
|
||||||
/// Return the [Digest] of the connection.
|
/// Return the [Digest] of the connection.
|
||||||
pub fn digest(&self) -> &Digest {
|
pub fn digest(&self) -> &Digest {
|
||||||
&self.digest
|
&self.digest
|
||||||
|
@ -1472,6 +1500,75 @@ mod tests_stream {
|
||||||
.unwrap();
|
.unwrap();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn write_informational_ignored() {
|
||||||
|
let wire = b"HTTP/1.1 200 OK\r\nFoo: Bar\r\n\r\n";
|
||||||
|
let mock_io = Builder::new().write(wire).build();
|
||||||
|
let mut http_stream = HttpSession::new(Box::new(mock_io));
|
||||||
|
// ignore the 100 Continue
|
||||||
|
http_stream.ignore_info_resp = true;
|
||||||
|
let response_100 = ResponseHeader::build(StatusCode::CONTINUE, None).unwrap();
|
||||||
|
http_stream
|
||||||
|
.write_response_header_ref(&response_100)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let mut response_200 = ResponseHeader::build(StatusCode::OK, None).unwrap();
|
||||||
|
response_200.append_header("Foo", "Bar").unwrap();
|
||||||
|
http_stream.update_resp_headers = false;
|
||||||
|
http_stream
|
||||||
|
.write_response_header_ref(&response_200)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn write_informational_100_not_ignored_if_expect_continue() {
|
||||||
|
let input = b"GET / HTTP/1.1\r\nExpect: 100-continue\r\n\r\n";
|
||||||
|
let output = b"HTTP/1.1 100 Continue\r\n\r\nHTTP/1.1 200 OK\r\nFoo: Bar\r\n\r\n";
|
||||||
|
|
||||||
|
let mock_io = Builder::new().read(&input[..]).write(output).build();
|
||||||
|
let mut http_stream = HttpSession::new(Box::new(mock_io));
|
||||||
|
http_stream.read_request().await.unwrap();
|
||||||
|
http_stream.ignore_info_resp = true;
|
||||||
|
// 100 Continue is not ignored due to Expect: 100-continue on request
|
||||||
|
let response_100 = ResponseHeader::build(StatusCode::CONTINUE, None).unwrap();
|
||||||
|
http_stream
|
||||||
|
.write_response_header_ref(&response_100)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let mut response_200 = ResponseHeader::build(StatusCode::OK, None).unwrap();
|
||||||
|
response_200.append_header("Foo", "Bar").unwrap();
|
||||||
|
http_stream.update_resp_headers = false;
|
||||||
|
http_stream
|
||||||
|
.write_response_header_ref(&response_200)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn write_informational_1xx_ignored_if_expect_continue() {
|
||||||
|
let input = b"GET / HTTP/1.1\r\nExpect: 100-continue\r\n\r\n";
|
||||||
|
let output = b"HTTP/1.1 200 OK\r\nFoo: Bar\r\n\r\n";
|
||||||
|
|
||||||
|
let mock_io = Builder::new().read(&input[..]).write(output).build();
|
||||||
|
let mut http_stream = HttpSession::new(Box::new(mock_io));
|
||||||
|
http_stream.read_request().await.unwrap();
|
||||||
|
http_stream.ignore_info_resp = true;
|
||||||
|
// 102 Processing is ignored
|
||||||
|
let response_102 = ResponseHeader::build(StatusCode::PROCESSING, None).unwrap();
|
||||||
|
http_stream
|
||||||
|
.write_response_header_ref(&response_102)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let mut response_200 = ResponseHeader::build(StatusCode::OK, None).unwrap();
|
||||||
|
response_200.append_header("Foo", "Bar").unwrap();
|
||||||
|
http_stream.update_resp_headers = false;
|
||||||
|
http_stream
|
||||||
|
.write_response_header_ref(&response_200)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn write_101_switching_protocol() {
|
async fn write_101_switching_protocol() {
|
||||||
let wire = b"HTTP/1.1 101 Switching Protocols\r\nFoo: Bar\r\n\r\n";
|
let wire = b"HTTP/1.1 101 Switching Protocols\r\nFoo: Bar\r\n\r\n";
|
||||||
|
|
|
@ -194,21 +194,20 @@ impl HttpSession {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|
||||||
// FIXME: we should ignore 1xx header because send_response() can only be called once
|
if header.status.is_informational() {
|
||||||
|
// ignore informational response 1xx header because send_response() can only be called once
|
||||||
// https://github.com/hyperium/h2/issues/167
|
// https://github.com/hyperium/h2/issues/167
|
||||||
|
debug!("ignoring informational headers");
|
||||||
if let Some(resp) = self.response_written.as_ref() {
|
|
||||||
if !resp.status.is_informational() {
|
|
||||||
warn!("Respond header is already sent, cannot send again");
|
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if self.response_written.as_ref().is_some() {
|
||||||
|
warn!("Response header is already sent, cannot send again");
|
||||||
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|
||||||
// no need to add these headers to 1xx responses
|
|
||||||
if !header.status.is_informational() {
|
|
||||||
/* update headers */
|
/* update headers */
|
||||||
header.insert_header(header::DATE, get_cached_date())?;
|
header.insert_header(header::DATE, get_cached_date())?;
|
||||||
}
|
|
||||||
|
|
||||||
// remove other h1 hop headers that cannot be present in H2
|
// remove other h1 hop headers that cannot be present in H2
|
||||||
// https://httpwg.org/specs/rfc7540.html#n-connection-specific-header-fields
|
// https://httpwg.org/specs/rfc7540.html#n-connection-specific-header-fields
|
||||||
|
@ -486,7 +485,8 @@ mod test {
|
||||||
expected_trailers.insert("test", HeaderValue::from_static("trailers"));
|
expected_trailers.insert("test", HeaderValue::from_static("trailers"));
|
||||||
let trailers = expected_trailers.clone();
|
let trailers = expected_trailers.clone();
|
||||||
|
|
||||||
tokio::spawn(async move {
|
let mut handles = vec![];
|
||||||
|
handles.push(tokio::spawn(async move {
|
||||||
let (h2, connection) = h2::client::handshake(client).await.unwrap();
|
let (h2, connection) = h2::client::handshake(client).await.unwrap();
|
||||||
tokio::spawn(async move {
|
tokio::spawn(async move {
|
||||||
connection.await.unwrap();
|
connection.await.unwrap();
|
||||||
|
@ -510,7 +510,7 @@ mod test {
|
||||||
assert_eq!(data, server_body);
|
assert_eq!(data, server_body);
|
||||||
let resp_trailers = body.trailers().await.unwrap().unwrap();
|
let resp_trailers = body.trailers().await.unwrap().unwrap();
|
||||||
assert_eq!(resp_trailers, expected_trailers);
|
assert_eq!(resp_trailers, expected_trailers);
|
||||||
});
|
}));
|
||||||
|
|
||||||
let mut connection = handshake(Box::new(server), None).await.unwrap();
|
let mut connection = handshake(Box::new(server), None).await.unwrap();
|
||||||
let digest = Arc::new(Digest::default());
|
let digest = Arc::new(Digest::default());
|
||||||
|
@ -520,7 +520,7 @@ mod test {
|
||||||
.unwrap()
|
.unwrap()
|
||||||
{
|
{
|
||||||
let trailers = trailers.clone();
|
let trailers = trailers.clone();
|
||||||
tokio::spawn(async move {
|
handles.push(tokio::spawn(async move {
|
||||||
let req = http.req_header();
|
let req = http.req_header();
|
||||||
assert_eq!(req.method, Method::GET);
|
assert_eq!(req.method, Method::GET);
|
||||||
assert_eq!(req.uri, "https://www.example.com/");
|
assert_eq!(req.uri, "https://www.example.com/");
|
||||||
|
@ -545,7 +545,11 @@ mod test {
|
||||||
}
|
}
|
||||||
|
|
||||||
let response_header = Box::new(ResponseHeader::build(200, None).unwrap());
|
let response_header = Box::new(ResponseHeader::build(200, None).unwrap());
|
||||||
http.write_response_header(response_header, false).unwrap();
|
assert!(http
|
||||||
|
.write_response_header(response_header.clone(), false)
|
||||||
|
.is_ok());
|
||||||
|
// this write should be ignored otherwise we will error
|
||||||
|
assert!(http.write_response_header(response_header, false).is_ok());
|
||||||
|
|
||||||
// test idling after response header is sent
|
// test idling after response header is sent
|
||||||
tokio::select! {
|
tokio::select! {
|
||||||
|
@ -559,7 +563,11 @@ mod test {
|
||||||
|
|
||||||
http.write_trailers(trailers).unwrap();
|
http.write_trailers(trailers).unwrap();
|
||||||
http.finish().unwrap();
|
http.finish().unwrap();
|
||||||
});
|
}));
|
||||||
|
}
|
||||||
|
for handle in handles {
|
||||||
|
// ensure no panics
|
||||||
|
assert!(handle.await.is_ok());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue