Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements on streaming() #3015

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Improvements on streaming() #3015

wants to merge 5 commits into from

Conversation

dynos01
Copy link

@dynos01 dynos01 commented Apr 14, 2023

PR Type

Bug Fix

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

This PR changes the behavior of actix_web::response::builder::HttpResponseBuilder::streaming().

Before this PR, if Content-Type is not set by the user, then there is no such header in the final response. This PR adds a default Content-Type of application/octet-stream, just like actix_web::response::builder::HttpResponseBuilder::json() defaults the Content-Type to application/json.

Before this PR, if the user does not call actix_web::response::builder::HttpResponseBuilder::no_chunking() but sets Content-Length, then the header is silently dropped, as described in #2306. This PR respects the user's input: if the user sets Content-Length, then actix_web::response::builder::HttpResponseBuilder::no_chunking() is automatically called with the corresponding stream length.

A small illustration program for the change:

use std::{
    pin::Pin,
    task::{Context, Poll},
};

use actix_web::{get, web, App, HttpRequest, HttpResponse, HttpServer, Responder, http::header};
use futures::stream::Stream;

struct ZeroStream {
    n: u64,
}

impl Stream for ZeroStream {
    type Item = Result<web::Bytes, actix_web::Error>;

    fn poll_next(self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll<Option<Self::Item>> {
        let resp = vec![0u8; self.n as usize];
        Poll::Ready(Some(Ok(web::Bytes::copy_from_slice(&resp))))
    }
}

#[get("/{n}")]
async fn zero(req: HttpRequest) -> impl Responder {
    let n: &str = match req.match_info().get("n") {
        Some(n) => n,
        None => return HttpResponse::BadRequest().finish(),
    };
    let n: u64 = match n.parse() {
        Ok(n) => n,
        Err(_) => return HttpResponse::BadRequest().finish(),
    };

    let stream = ZeroStream {
        n,
    };

    HttpResponse::Ok()
        .append_header((header::CONTENT_LENGTH, format!("{n}")))
        .streaming(stream)
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(|| {
        App::new()
            .service(zero)
    })
    .bind(("127.0.0.1", 8080))?
    .run()
    .await
}

Request:

GET /100 HTTP/1.1

The response is now:

< HTTP/1.1 200 OK
< content-length: 100
< content-type: application/octet-stream
< date: Fri, 14 Apr 2023 15:49:33 GMT

instead of:

< HTTP/1.1 200 OK
< transfer-encoding: chunked
< date: Fri, 14 Apr 2023 16:19:26 GMT
// Downloading endlessly

Closes #2306

@robjtede robjtede added B-semver-patch A-web project: actix-web labels Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web project: actix-web B-semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[awc] HTTP client ignores Content-Length if Body is stream.
2 participants