Skip to content

Commit

Permalink
revert broken fix in #2624
Browse files Browse the repository at this point in the history
  • Loading branch information
robjtede committed Jun 11, 2022
1 parent 8e76a1c commit 4341533
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 69 deletions.
73 changes: 4 additions & 69 deletions actix-http/src/h1/dispatcher.rs
Expand Up @@ -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::{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, ()),
Expand Down
3 changes: 3 additions & 0 deletions actix-http/src/h1/dispatcher_tests.rs
Expand Up @@ -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();
Expand Down

0 comments on commit 4341533

Please sign in to comment.