Actix-web: Streaming response - inconsistent behaviour

Created on 10 Jan 2020 · 6Comments · Source: actix/actix-web

I've ran into the following issue recently while returning chunked encoded response:

I stream some response by chunk and some chunks are empty.
What happens is that if I request the endpoint requiring compression I'm getting the full response, but if I request it uncompressed I get only the part of data (the one before the first empty chunk).

It aligns with HTTP standard in a way: chunked response is terminated by a chunk with 0 length, but makes the usage of the framework a bit confusing (it took me a day to figure out the reason of this wired behavior).

My current suggestion would be that actix-web should just drop empty chunks from the given stream (this is what I did to solve the issue), but there might be another view on that.

Code to reproduce

Cargo.toml

[package]
name = "actix-web-streaming-bug"
version = "0.1.0"
edition = "2018"

[dependencies]
actix-web="2.0.0"
actix-rt = "1.0.0"
bytes = "0.5.3"
futures = "0.3.1"

main.rs

use actix_web::{middleware, web, App, HttpRequest, HttpResponse, HttpServer, Responder};
use bytes::Bytes;

use futures::stream::iter;

use futures::{self, StreamExt};

async fn handle_request(_req: HttpRequest) -> impl Responder {
    let response = vec![
        Bytes::from("1\n"),
        Bytes::from(""),
        Bytes::from("2\n")
    ];
    HttpResponse::Ok().streaming(iter(response).map(|b| Ok(b) as Result<Bytes, ()>))
}

#[actix_rt::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(move || {
        App::new()
            .wrap(middleware::Compress::default())
            .service(web::resource("/").to(handle_request))
    })
    .bind("127.0.0.1:8080")
    .unwrap()
    .workers(1)
    .run()
    .await
}

$ curl 127.0.0.1:8080
1

$ curl --compressed 127.0.0.1:8080
1
2

C-bug needs-investigation

Source

bekh6ex

Most helpful comment

@JohnTitor I'll try to dig and propose a fix.

tyranron on 20 Jan 2020

👍3

All 6 comments

@JohnTitor I'll try to dig and propose a fix.

tyranron on 20 Jan 2020

👍3

@JohnTitor ok, in the nutshell @bekh6ex has give a problem description and cause:

It aligns with HTTP standard in a way: chunked response is terminated by a chunk with 0 length, but makes the usage of the framework a bit confusing (it took me a day to figure out the reason of this wired behavior).

From Wiki about Chunked transfer encoding:

Each chunk is preceded by its size in bytes. The transmission ends when a zero-length chunk is received. The chunked keyword in the Transfer-Encoding header is used to indicate chunked transfer.

Currently, any Stream provided into Response just polled "as is":

impl<S, E> MessageBody for BodyStream<S, E>
where
    S: Stream<Item = Result<Bytes, E>>,
    E: Into<Error>,
{
    fn size(&self) -> BodySize {
        BodySize::Stream
    }

    fn poll_next(&mut self, cx: &mut Context<'_>) -> Poll<Option<Result<Bytes, Error>>> {
        unsafe { Pin::new_unchecked(self) }
            .project()
            .stream
            .poll_next(cx)
            .map(|res| res.map(|res| res.map_err(std::convert::Into::into)))
    }
}

So, I assume there are following ways to deal with this situation:

Do not do anything. Consider this behavior as intentional to align with HTTP standard and __document this explicitly__ in API docs.
- 👍 no performance impact at all
- 👎 possible source of future confusions despite the docs
Filter out empty chunks from the given Stream, so finish streaming when the Stream ends, but not on the first empty chunk. Still document this in API docs.
- 👍 obvious and natural behavior for the user code, no gotchas
- 👎 minor performance impact as every chunk should be examined for emptiness
Provide an alternative API method (like .streaming_skip_empty()) to ResponseBuilder with the behavior described in point 2 above. Document both properly.
- 👍 obvious and natural behavior for the user code, no gotchas
- 👍 no performance impact, as API user chooses the desired behavior for his situation
- 👎 complicates API