(opendal) branch main updated: fix(services/http): read a response that carries no content length (#8385)

[xu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

Xuanwo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/opendal.git


The following commit(s) were added to refs/heads/main by this push:
     new 8984cfadc fix(services/http): read a response that carries no content length (#8385)
8984cfadc is described below

commit 8984cfadc09bf1961cc0240e7401dc7b9c035d87
Author: Ayush Baluni <73...@users.noreply.github.com>
AuthorDate: Wed Oct 21 15:30:11 2026 +0530

    fix(services/http): read a response that carries no content length (#8385)
    
    Content-Length is optional in HTTP. An HTTP/1.1 response can be framed
    with Transfer-Encoding: chunked (RFC 9112 6.1) and an HTTP/2 response is
    framed by the end of the stream (RFC 9113 8.1.1), so neither carries one.
    Since 0.59 every read of such a response failed with "HTTP response does
    not contain file content length".
    
    The requirement comes from the metadata invariant in #8196 -- a file's
    Metadata always carries its complete content length -- and that invariant
    is left intact here. parse_into_metadata still refuses to invent a length.
    
    What changes is that services-http was asking for metadata it does not
    need in order to read. RpRead holds an Option<Metadata> precisely so a
    service can decline to report metadata the read did not observe
    natively, and services-ipmfs, services-ftp and services-gcs-grpc already
    return RpRead::default() for the same reason. So when the response states
    no length, report no metadata and stream the body.
    
    The length is derived exactly as parse_into_metadata derives it, so the
    two cannot disagree about whether one is present, and a malformed
    Content-Length still errors rather than being read as absent.
    
    A caller that asks for Reader::metadata() on such a response now gets
    ErrorKind::Unsupported, which core/tests/behavior/async_read.rs already
    accepts as the answer from a service that does not report read metadata.
    
    stat is deliberately unchanged: RpStat::new takes a Metadata rather than
    an Option, so on a HEAD with no length there is nothing correct for it to
    return, and answering with EntryMode::Unknown would change what stat
    promises about a file that does exist.
    
    Signed-off-by: aayushbaluni <73...@users.noreply.github.com>
    Co-authored-by: dentiny <de...@gmail.com>
---
 core/services/http/src/reader.rs | 109 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 105 insertions(+), 4 deletions(-)

diff --git a/core/services/http/src/reader.rs b/core/services/http/src/reader.rs
index 52ef4ae59..11ba17ea0 100644
--- a/core/services/http/src/reader.rs
+++ b/core/services/http/src/reader.rs
@@ -56,10 +56,9 @@ impl oio::StreamRead for HttpReader {
         let status = resp.status();
 
         let (rp, stream) = match status {
-            StatusCode::OK | StatusCode::PARTIAL_CONTENT => (
-                RpRead::new(parse_into_metadata(path, resp.headers())?),
-                resp.into_body(),
-            ),
+            StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
+                (read_metadata(path, resp.headers())?, resp.into_body())
+            }
             _ => {
                 let (part, mut body) = resp.into_parts();
                 let buf = body.to_buffer().await?;
@@ -73,3 +72,105 @@ impl oio::StreamRead for HttpReader {
         Ok((rp, Box::new(stream) as Box<dyn oio::ReadStreamDyn>))
     }
 }
+
+/// Build the read reply from a successful response's headers.
+///
+/// `Content-Length` is optional in HTTP: an HTTP/1.1 response can be framed
+/// with `Transfer-Encoding: chunked` (RFC 9112 section 6.1) and an HTTP/2
+/// response is framed by the end of the stream (RFC 9113 section 8.1.1), so
+/// neither carries one. RFC 9110 section 8.6 only says a server *should* send
+/// it when the length is known in advance.
+///
+/// `parse_into_metadata` requires a length for a file, because `Metadata` for a
+/// file always carries its complete content length. That requirement is correct
+/// for `stat`, but a read does not need it: `RpRead` holds an
+/// `Option<Metadata>` precisely so a service can decline to report metadata
+/// that the read did not observe natively. So when the response states no
+/// length, report no metadata and let the body stream.
+///
+/// A caller that explicitly asks for `Reader::metadata()` then gets
+/// `ErrorKind::Unsupported`, which is the documented answer for a service that
+/// does not return metadata on read. Before this, the read itself failed.
+fn read_metadata(path: &str, headers: &http::HeaderMap) -> Result<RpRead> {
+    // Mirrors how parse_into_metadata derives the length, so the two cannot
+    // disagree about whether one is present. A malformed header still errors
+    // here rather than being silently treated as absent.
+    let content_length = parse_content_range(headers)?
+        .and_then(|value| value.size())
+        .or(parse_content_length(headers)?);
+
+    match content_length {
+        Some(_) => Ok(RpRead::new(parse_into_metadata(path, headers)?)),
+        None => Ok(RpRead::default()),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use http::HeaderMap;
+    use http::HeaderValue;
+
+    use super::*;
+
+    fn headers(pairs: &[(&str, &str)]) -> HeaderMap {
+        let mut headers = HeaderMap::new();
+        for (name, value) in pairs {
+            headers.insert(
+                http::header::HeaderName::from_bytes(name.as_bytes()).expect("name must be valid"),
+                HeaderValue::from_str(value).expect("value must be valid"),
+            );
+        }
+        headers
+    }
+
+    #[test]
+    fn test_read_metadata_with_content_length() {
+        let rp = read_metadata("file.txt", &headers(&[("content-length", "12")]))
+            .expect("read must be accepted");
+
+        let meta = rp.metadata().expect("metadata must be reported");
+        assert_eq!(meta.content_length(), 12);
+    }
+
+    #[test]
+    fn test_read_metadata_from_content_range() {
+        // A ranged read states the full size in Content-Range, not
+        // Content-Length, and that is still a length.
+        let rp = read_metadata(
+            "file.txt",
+            &headers(&[("content-length", "4"), ("content-range", "bytes 0-3/12")]),
+        )
+        .expect("read must be accepted");
+
+        let meta = rp.metadata().expect("metadata must be reported");
+        assert_eq!(meta.content_length(), 12);
+    }
+
+    #[test]
+    fn test_read_metadata_without_content_length_reports_none() {
+        // A chunked HTTP/1.1 response, and the shape an HTTP/2 response takes:
+        // no length anywhere. The read must still be accepted.
+        let rp = read_metadata("file.txt", &headers(&[("transfer-encoding", "chunked")]))
+            .expect("a response with no length must still be read");
+
+        assert!(rp.metadata().is_none());
+    }
+
+    #[test]
+    fn test_read_metadata_without_any_headers_reports_none() {
+        let rp = read_metadata("file.txt", &HeaderMap::new())
+            .expect("a response with no headers must still be read");
+
+        assert!(rp.metadata().is_none());
+    }
+
+    #[test]
+    fn test_read_metadata_rejects_a_malformed_content_length() {
+        // Absent is not the same as unparsable: a server that sends garbage
+        // should still be reported rather than treated as length-free.
+        let err = read_metadata("file.txt", &headers(&[("content-length", "not-a-number")]))
+            .expect_err("a malformed length must be an error");
+
+        assert_eq!(err.kind(), ErrorKind::Unexpected);
+    }
+}