actix / actix-web

Actix Web is a powerful, pragmatic, and extremely fast web framework for Rust.
https://actix.rs
Apache License 2.0
21.45k stars 1.66k forks source link

actix-files: Range request for static HTML file results in Content Encoding Error #3191

Open puzzlepaint opened 10 months ago

puzzlepaint commented 10 months ago

Expected Behavior

Serving static HTML files via actix-files should allow viewing these files in a web browser.

Current Behavior

Serving static HTML files via actix-files works partially, but under some circumstances, trying to view a file results in a "Content Encoding Error" (tested with Firefox).

Possible Solution

I investigated the error using Firefox' developer tools and it turned out that it occurs if the browser sends a range request for the HTML file. Not being a web developer, I am not aware of why the browser sometimes sends a range request when normally browsing to a file; they seemed to be conditional range requests related to the ETag, which seems like a caching mechanism.

The responses to the range requests contained an HTTP header "Content-Encoding: identity". Looking at https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding, "identity" does not appear on this page. Thus, it seems to me that "identity" is not a valid value for the Content-Encoding header, which likely causes the "Content Encoding Error" display.

I had a look at the actix-files code and found that this snippet is responsible for adding that header to replies to range requests:

https://github.com/actix/actix-web/blob/e50bceb91493087d4059e605d94c13a11b09e747/actix-files/src/named.rs#L545

// When a Content-Encoding header is present in a 206 partial content response
// for video content, it prevents browser video players from starting playback
// before loading the whole video and also prevents seeking.
//
// See: https://github.com/actix/actix-web/issues/2815
//
// The assumption of this fix is that the video player knows to not send an
// Accept-Encoding header for this request and that downstream middleware will
// not attempt compression for requests without it.
//
// TODO: Solve question around what to do if self.encoding is set and partial
// range is requested. Reject request? Ignoring self.encoding seems wrong, too.
// In practice, it should not come up.
if req.headers().contains_key(&header::ACCEPT_ENCODING) {
    // don't allow compression middleware to modify partial content
    res.insert_header((
        header::CONTENT_ENCODING,
        HeaderValue::from_static("identity"),
    ));
}

Testing with a patched version of actix-files, commenting out this snippet seemed to solve the issue. However, according to the comment in the code, it seems that this will break cases where compression middleware is used.

I am not familiar with how actix-web works internally, but given the above, it seems to me that a proper solution might be to either:

Steps to Reproduce (for bugs)

Serve static HTML files via actix-files and view them in a browser, having the browser use range requests. It is not clear to me under which conditions browsers use range requests for HTML files.

Context

Serving some static HTML files via actix-files.

Your Environment

silverpill commented 7 months ago

I'm experiencing a similar problem with actix-files 0.6.5 and Firefox. The browser fails to display an image when the server returns 206 Partial Content.

It's not easy to reproduce. I think Firefox adds the Range header only if previous attempt to load the resource was interrupted either by a page reload or due to a broken network connection.

semirix commented 6 months ago

I've managed to find a hack workaround:

use futures::future::FutureExt;
use actix_web::{dev::Service as _};

app.wrap_fn(|request, service| {
    let path = request.path().to_owned();

    service.call(request).map(move |response| {
        if path.contains("/static") {
            response.and_then(|mut response| {
                response.headers_mut().remove("Content-Encoding");
                Ok(response)
            })
        } else {
            response
        }
    })
});

I'm serving files from the /static directory so modify to fit your use case accordingly.

Diomendius commented 6 months ago

Per RFC 2616 (https://datatracker.ietf.org/doc/html/rfc2616#section-3.5), Content-Encoding: identity should never be used:

identity The default (identity) encoding; the use of no transformation whatsoever. This content-coding is used only in the Accept-Encoding header, and SHOULD NOT be used in the Content-Encoding header.

Actix is doing the wrong thing here, regardless of how any browser behaves or misbehaves. One could argue that Firefox should silently ignore Content-Encoding: identity instead of failing with an error, since RFC 2616 uses "SHOULD NOT" and not "MUST NOT", and refusing to even attempt to display the content to the user is not particularly helpful, but this is not relevant to Actix.

The most reliable way to reproduce this using Firefox is probably to have actix-files serve a video file, since actix-files serves media files with inline disposition by default. This isn't perfectly reliable, since some unknown factor causes Firefox to sometimes load the entire file in one request and sometimes stream it in pieces via Range, and caching isn't the cause of this, but when Firefox chooses to stream the file through multiple requests, each request fails with NS_ERROR_INVALID_CONTENT_ENCODING. Firefox actually retries these requests aggressively in my case, maxing out a CPU core in the process, but every one fails.

actix-files reliably serves responses with the incorrect Content-Encoding: identity header to any request with both a valid Range header and an Accept-Encoding header, even an empty Accept-Encoding header. This is easy to verify using curl, netcat, or any other means of sending manual HTTP requests.

Minimal reproduction `Cargo.toml`: ```toml [package] name = "example" version = "0.1.0" edition = "2021" [dependencies] actix-web = "=4.5.1" actix-files = "=0.6.5" ``` `src/main.rs`: ```rust use actix_files::NamedFile; use actix_web::{get, Responder}; #[get("/")] async fn index() -> impl Responder { NamedFile::open("test_file") } #[actix_web::main] async fn main() -> std::io::Result<()> { use actix_web::{App, HttpServer}; HttpServer::new(|| App::new().service(index)) .bind(("127.0.0.1", 8080))? .run() .await } ``` `test.sh`: ```sh #!/bin/bash echo foo > test_file cargo b cargo r& sleep 1 nc 127.0.0.1 8080 << EOF GET / HTTP/1.1 Range: bytes=0- Accept-Encoding: EOF kill %1 ``` Create these files, then run `chmod +x test.sh && ./test.sh`. You should see a plaintext HTTP/1.1 response with the `Content-Encoding: identity` header.
Revertron commented 1 month ago

I've made a following workaround:

struct CustomFileResponder {
    file: NamedFile,
}

impl CustomFileResponder {
    fn new(file: NamedFile) -> Self {
        Self { file }
    }
}

impl Responder for CustomFileResponder {
    type Body = BoxBody;

    fn respond_to(self, req: &HttpRequest) -> HttpResponse<Self::Body> {
        let mut response = self.file.respond_to(req);

        // Modify the headers as needed
        let headers = response.headers_mut();
        if let Some(header) = headers.get_mut(actix_web::http::header::CONTENT_ENCODING) {
            if let Ok(h) = header.to_str() {
                if h == "identity" {
                    headers.remove(actix_web::http::header::CONTENT_ENCODING);
                }
            }
        }

        response
    }
}

And where you serve the file you make:

// Return the file as a response
let file = NamedFile::open_async(file_path).await?;
Ok(CustomFileResponder::new(file))

Now all .mp4 files served inline are opening flawlessly. But without this workaround they either get in a loop of errors or just give an error about wrong content encoding.