From 264a703d947d4eaa3b39f8e90d02bb7800ddea54 Mon Sep 17 00:00:00 2001
From: Rob Ede <robjtede@icloud.com>
Date: Sat, 11 Jun 2022 13:43:13 +0100
Subject: [PATCH] revert broken fix in #2624 (#2779)
* revert broken fix in #2624
* update changelog
---
actix-http/CHANGES.md | 5 ++
actix-http/src/h1/dispatcher.rs | 73 ++-------------------------
actix-http/src/h1/dispatcher_tests.rs | 3 ++
3 files changed, 12 insertions(+), 69 deletions(-)
diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md
index 75c13151..d431ec5f 100644
--- a/actix-http/CHANGES.md
+++ b/actix-http/CHANGES.md
@@ -4,6 +4,11 @@
### Changed
- Minimum supported Rust version (MSRV) is now 1.56 due to transitive `hashbrown` dependency.
+### Fixed
+- Revert broken fix in [#2624] that caused erroneous 500 error responses. Temporarily re-introduces [#2357] bug. [#2779]
+
+[#2779]: https://github.com/actix/actix-web/issues/2779
+
## 3.0.4 - 2022-03-09
### Fixed
diff --git a/actix-http/src/h1/dispatcher.rs b/actix-http/src/h1/dispatcher.rs
index dea8a4be..c2ddc06b 100644
--- a/actix-http/src/h1/dispatcher.rs
+++ b/actix-http/src/h1/dispatcher.rs
@@ -15,14 +15,14 @@ use bitflags::bitflags;
use bytes::{Buf, BytesMut};
use futures_core::ready;
use pin_project_lite::pin_project;
-use tracing::{debug, error, trace};
+use tracing::{error, trace};
use crate::{
body::{BodySize, BoxBody, MessageBody},
config::ServiceConfig,
error::{DispatchError, ParseError, PayloadError},
service::HttpFlow,
- ConnectionType, Error, Extensions, OnConnectData, Request, Response, StatusCode,
+ Error, Extensions, OnConnectData, Request, Response, StatusCode,
};
use super::{
@@ -691,74 +691,12 @@ where
let can_not_read = !self.can_read(cx);
// limit amount of non-processed requests
- if pipeline_queue_full {
+ if pipeline_queue_full || can_not_read {
return Ok(false);
}
let mut this = self.as_mut().project();
- if can_not_read {
- debug!("cannot read request payload");
-
- if let Some(sender) = &this.payload {
- // ...maybe handler does not want to read any more payload...
- if let PayloadStatus::Dropped = sender.need_read(cx) {
- debug!("handler dropped payload early; attempt to clean connection");
- // ...in which case poll request payload a few times
- loop {
- match this.codec.decode(this.read_buf)? {
- Some(msg) => {
- match msg {
- // payload decoded did not yield EOF yet
- Message::Chunk(Some(_)) => {
- // if non-clean connection, next loop iter will detect empty
- // read buffer and close connection
- }
-
- // connection is in clean state for next request
- Message::Chunk(None) => {
- debug!("connection successfully cleaned");
-
- // reset dispatcher state
- let _ = this.payload.take();
- this.state.set(State::None);
-
- // break out of payload decode loop
- break;
- }
-
- // Either whole payload is read and loop is broken or more data
- // was expected in which case connection is closed. In both
- // situations dispatcher cannot get here.
- Message::Item(_) => {
- unreachable!("dispatcher is in payload receive state")
- }
- }
- }
-
- // not enough info to decide if connection is going to be clean or not
- None => {
- error!(
- "handler did not read whole payload and dispatcher could not \
- drain read buf; return 500 and close connection"
- );
-
- this.flags.insert(Flags::SHUTDOWN);
- let mut res = Response::internal_server_error().drop_body();
- res.head_mut().set_connection_type(ConnectionType::Close);
- this.messages.push_back(DispatcherMessage::Error(res));
- *this.error = Some(DispatchError::HandlerDroppedPayload);
- return Ok(true);
- }
- }
- }
- }
- } else {
- // can_not_read and no request payload
- return Ok(false);
- }
- }
-
let mut updated = false;
// decode from read buf as many full requests as possible
@@ -904,10 +842,7 @@ where
if timer.as_mut().poll(cx).is_ready() {
// timeout on first request (slow request) return 408
- trace!(
- "timed out on slow request; \
- replying with 408 and closing connection"
- );
+ trace!("timed out on slow request; replying with 408 and closing connection");
let _ = self.as_mut().send_error_response(
Response::with_body(StatusCode::REQUEST_TIMEOUT, ()),
diff --git a/actix-http/src/h1/dispatcher_tests.rs b/actix-http/src/h1/dispatcher_tests.rs
index 40454d45..b3ee3d2b 100644
--- a/actix-http/src/h1/dispatcher_tests.rs
+++ b/actix-http/src/h1/dispatcher_tests.rs
@@ -783,6 +783,9 @@ async fn upgrade_handling() {
.await;
}
+// fix in #2624 reverted temporarily
+// complete fix tracked in #2745
+#[ignore]
#[actix_rt::test]
async fn handler_drop_payload() {
let _ = env_logger::try_init();