Skip to content

Commit 264a703

Browse files
authored
revert broken fix in #2624 (#2779)
* revert broken fix in #2624 * update changelog
1 parent 498fb95 commit 264a703

File tree

3 files changed

+12
-69
lines changed

3 files changed

+12
-69
lines changed

actix-http/CHANGES.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44
### Changed
55
- Minimum supported Rust version (MSRV) is now 1.56 due to transitive `hashbrown` dependency.
66

7+
### Fixed
8+
- Revert broken fix in [#2624] that caused erroneous 500 error responses. Temporarily re-introduces [#2357] bug. [#2779]
9+
10+
[#2779]: https://github.com/actix/actix-web/issues/2779
11+
712

813
## 3.0.4 - 2022-03-09
914
### Fixed

actix-http/src/h1/dispatcher.rs

Lines changed: 4 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ use bitflags::bitflags;
1515
use bytes::{Buf, BytesMut};
1616
use futures_core::ready;
1717
use pin_project_lite::pin_project;
18-
use tracing::{debug, error, trace};
18+
use tracing::{error, trace};
1919

2020
use crate::{
2121
body::{BodySize, BoxBody, MessageBody},
2222
config::ServiceConfig,
2323
error::{DispatchError, ParseError, PayloadError},
2424
service::HttpFlow,
25-
ConnectionType, Error, Extensions, OnConnectData, Request, Response, StatusCode,
25+
Error, Extensions, OnConnectData, Request, Response, StatusCode,
2626
};
2727

2828
use super::{
@@ -691,74 +691,12 @@ where
691691
let can_not_read = !self.can_read(cx);
692692

693693
// limit amount of non-processed requests
694-
if pipeline_queue_full {
694+
if pipeline_queue_full || can_not_read {
695695
return Ok(false);
696696
}
697697

698698
let mut this = self.as_mut().project();
699699

700-
if can_not_read {
701-
debug!("cannot read request payload");
702-
703-
if let Some(sender) = &this.payload {
704-
// ...maybe handler does not want to read any more payload...
705-
if let PayloadStatus::Dropped = sender.need_read(cx) {
706-
debug!("handler dropped payload early; attempt to clean connection");
707-
// ...in which case poll request payload a few times
708-
loop {
709-
match this.codec.decode(this.read_buf)? {
710-
Some(msg) => {
711-
match msg {
712-
// payload decoded did not yield EOF yet
713-
Message::Chunk(Some(_)) => {
714-
// if non-clean connection, next loop iter will detect empty
715-
// read buffer and close connection
716-
}
717-
718-
// connection is in clean state for next request
719-
Message::Chunk(None) => {
720-
debug!("connection successfully cleaned");
721-
722-
// reset dispatcher state
723-
let _ = this.payload.take();
724-
this.state.set(State::None);
725-
726-
// break out of payload decode loop
727-
break;
728-
}
729-
730-
// Either whole payload is read and loop is broken or more data
731-
// was expected in which case connection is closed. In both
732-
// situations dispatcher cannot get here.
733-
Message::Item(_) => {
734-
unreachable!("dispatcher is in payload receive state")
735-
}
736-
}
737-
}
738-
739-
// not enough info to decide if connection is going to be clean or not
740-
None => {
741-
error!(
742-
"handler did not read whole payload and dispatcher could not \
743-
drain read buf; return 500 and close connection"
744-
);
745-
746-
this.flags.insert(Flags::SHUTDOWN);
747-
let mut res = Response::internal_server_error().drop_body();
748-
res.head_mut().set_connection_type(ConnectionType::Close);
749-
this.messages.push_back(DispatcherMessage::Error(res));
750-
*this.error = Some(DispatchError::HandlerDroppedPayload);
751-
return Ok(true);
752-
}
753-
}
754-
}
755-
}
756-
} else {
757-
// can_not_read and no request payload
758-
return Ok(false);
759-
}
760-
}
761-
762700
let mut updated = false;
763701

764702
// decode from read buf as many full requests as possible
@@ -904,10 +842,7 @@ where
904842
if timer.as_mut().poll(cx).is_ready() {
905843
// timeout on first request (slow request) return 408
906844

907-
trace!(
908-
"timed out on slow request; \
909-
replying with 408 and closing connection"
910-
);
845+
trace!("timed out on slow request; replying with 408 and closing connection");
911846

912847
let _ = self.as_mut().send_error_response(
913848
Response::with_body(StatusCode::REQUEST_TIMEOUT, ()),

actix-http/src/h1/dispatcher_tests.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,9 @@ async fn upgrade_handling() {
783783
.await;
784784
}
785785

786+
// fix in #2624 reverted temporarily
787+
// complete fix tracked in #2745
788+
#[ignore]
786789
#[actix_rt::test]
787790
async fn handler_drop_payload() {
788791
let _ = env_logger::try_init();

0 commit comments

Comments
 (0)