actix / actix/actix-web

Failure to flush Payload when it is dropped before handler consumes all bytes

Offen
#2,764 2 Kommentare 1 Reaktion 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Rust
Sterne
24.8k
Forks
1.9k
Ø Merge
23 Std. 10 Min.
Gemergte PRs (30 T.)
26

Beschreibung

In cases where a handler exits early without reading a supplied `Payload`, there is some new code in actix-web (see PR #2624) that consumes the rest of the bytes in the request.

It seems not to work with large request bodies (approaching 512k).

## Expected Behavior
If a `Payload` is dropped before all bytes are consumed, the rest of the client request should still be consumed.

## Current Behavior
If a `Payload` is dropped before all bytes are consumed, the rest of the request is consumed if the request is small, but larger requests stop reading and drop the connection with a message `"handler did not read whole payload and dispatcher could not drain read buf; return 500 and close connection"`.

## Possible Solution
Haven't quite found the root cause yet, but it appears that the `read_buf` in the `Dispatcher` is empty. When `PayloadDecoder::decode()` is called with `read_buf` as `src`, it reports zero bytes available, but `self.kind` contains a nonzero `Kind::Length` of remaining bytes. If they were both correctly zero, it would return `PayloadItem::Eof`, and the flush would terminate correctly. If they were both correctly nonzero, it would successfully return `PayloadItem::Chunk(buf)`, which would continue the flush loop until exhausted.

I'm not sure which one is wrong here, but it only seems to happen if there are a certain number of reads required to flush the request.

## Steps to Reproduce (for bugs)
Minimal server:
```rust
use actix_web::{post, web::Payload, App, HttpServer};

#[post("/hello")]
async fn greet(_: Payload) -> &'static str {
"Ok\n"
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
env_logger::init();
HttpServer::new(|| App::new().service(greet))
.bind(("127.0.0.1", 5000))?
.run()
.await
}
```
Client-side - I was able to duplicate this by sending a 511k file (succeeded), and a 512k file (failed), but managed to narrow down to exact number of bytes by using `curl`'s `-C` to skip initial bytes in the file. Byte ranges may vary across OSs, not sure.
```bash
# create a 512k file of random data
dd if=/dev/random of=512k bs=1024 count=512
# this succeeds - continue sending data from byte 1753
curl -C 1753 -H 'Content-Type: application/octet-stream' -d @512k -v http://127.0.0.1:5000/hello
# this fails - continue sending data from byte 1752
curl -C 1752 -H 'Content-Type: application/octet-stream' -d @512k -v http://127.0.0.1:5000/hello
```

## Context
I'd like to be able to return from a handler without having to worry about whether I've finished reading the `Payload` completely. For example, streaming a file to disk, then having a write fail, it's much easier to handle with a simple `?` and a handler returning a `Result`, than having to wrap the whole thing in a handler that cleans up the `Payload` before returning an error.

## Your Environment
- MacOS 12.4 x86_64
- actix-web 4.0.1
- actix-http 3.0.0
- rustc 1.61.0 (fe5b13d68 2022-05-18)

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.