Skip to content
14 changes: 9 additions & 5 deletions src/http/client.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ struct Client {
transport : ClientTransport
tls : @tls.Tls?
sender : Sender
mut request_method : RequestMethod?
}

///|
Expand All @@ -45,7 +46,7 @@ pub suberror ProxyError {
/// so `headers` should not be used by the caller later.
/// The following headers is automatically set,
/// and must not be specified in `headers`:
///
///
/// - Host
/// - Content-Length, Transfer-Encoding
///
Expand Down Expand Up @@ -106,7 +107,7 @@ pub async fn Client::connect(
(Some(tls), Reader::new(tls), Sender::new(tls, headers~))
}
}
{ transport, tls, reader, sender }
{ transport, tls, reader, sender, request_method: None }
}

///|
Expand All @@ -122,7 +123,7 @@ pub async fn Client::connect(
/// so `headers` should not be used by the caller later.
/// The following headers is automatically set,
/// and must not be specified in `headers`:
///
///
/// - Host
/// - Content-Length, Transfer-Encoding
///
Expand Down Expand Up @@ -211,7 +212,9 @@ pub async fn Client::flush(self : Client) -> Unit {
pub async fn Client::end_request(self : Client) -> Response {
self.sender.end_body()
self.reader.skip_body()
self.reader.read_response()
let response = self.reader.read_response(request_method=self.request_method)
self.request_method = None
response
}

///|
Expand All @@ -228,7 +231,7 @@ pub async fn Client::end_request(self : Client) -> Response {
/// extra HTTP headers can be passed via `extra_headers`.
/// The following headers is automatically set by `request`,
/// and must not be specified in `extra_headers`:
///
///
/// - Host
/// - Content-Length, Transfer-Encoding
pub async fn Client::request(
Expand All @@ -238,6 +241,7 @@ pub async fn Client::request(
extra_headers? : Map[String, String] = {},
) -> Unit {
self.sender.send_request(meth, path, extra_headers~)
self.request_method = Some(meth)
}

///|
Expand Down
48 changes: 46 additions & 2 deletions src/http/parser.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ priv enum BodyKind {
Fixed(Int)
Chunked(Int)
PassThrough
WaitConnectionClose
}

///|
Expand Down Expand Up @@ -90,12 +91,13 @@ impl @io.Reader for Reader with _direct_read(self, buf, offset~, max_len~) {
n
}
PassThrough => self.transport._direct_read(buf, offset~, max_len~)
WaitConnectionClose => self.transport._direct_read(buf, offset~, max_len~)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Drain transport buffer when using WaitConnectionClose

The new WaitConnectionClose branch reads via self.transport._direct_read, but _get_internal_buffer is not updated to expose the transport buffer for this state. Because headers are parsed with self.transport.read_until, body bytes can already be buffered in the transport reader; bypassing that buffer drops/truncates close-delimited response bodies before EOF is reached.

Useful? React with 👍 / 👎.

}
}

///|
impl @io.Reader for Reader with _get_internal_buffer(self) {
if self.body is PassThrough {
if self.body is PassThrough || self.body is WaitConnectionClose {
self.transport._get_internal_buffer()
} else {
self.read_buf
Expand Down Expand Up @@ -185,6 +187,38 @@ async fn Reader::read_request(self : Reader) -> Request {
{ meth, path, headers }
}

///|
/// Determines if the response body should be read until connection close.
/// Returns true when:
/// - No Content-Length or Transfer-Encoding header is present
/// - Body has not been determined by other means (still Empty)
/// - Status code is not 1xx, 204, 205, or 304 (which MUST NOT have a message body)
/// - Not a successful (2xx) response to a CONNECT request (which becomes a tunnel)
/// - Not a response to a HEAD request (which never has a message body)
fn should_read_body_until_close(
code : Int,
headers : Map[String, String],
current_body : BodyKind,
request_method : RequestMethod?,
) -> Bool {
// HEAD responses never have a message body, regardless of headers
if request_method is Some(Head) {
return false
}
// Successful CONNECT responses (2xx) switch connection to tunnel mode
// and have no HTTP message body
if request_method is Some(Connect) && code >= 200 && code < 300 {
return false
Comment on lines +210 to +211
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Exclude HEAD responses from close-delimited fallback

Client::end_request now passes the originating request method into read_response, but should_read_body_until_close only exempts successful CONNECT and still treats HEAD responses without Content-Length/Transfer-Encoding as WaitConnectionClose. For a keep-alive server that correctly returns a header-only HEAD response, skip_body()/read_all() will block waiting for connection close even though HEAD responses are always bodyless.

Useful? React with 👍 / 👎.

}
current_body is Empty &&
headers.get("content-length") is None &&
headers.get("transfer-encoding") is None &&
code >= 200 &&
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Skip close-delimited fallback for successful CONNECT

This predicate classifies all 2xx responses without Content-Length/Transfer-Encoding as body-until-close, which is incorrect for successful CONNECT (there is no HTTP message body; the stream becomes a tunnel). In this repo, Client::connect calls end_request() and then skip_response_body() before entering passthrough; when read_response sets WaitConnectionClose, skip_response_body waits for EOF and can hang against compliant proxies that keep the tunnel open.

Useful? React with 👍 / 👎.

code != 204 &&
code != 205 &&
code != 304
Comment on lines +217 to +219
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exclude 205 Reset Content from body-until-close fallback

205 Reset Content is a no-body response status, but this condition only excludes 204 and 304, so a header-only 205 response is treated as close-delimited. On keep-alive connections without length headers, callers like read_all()/skip_body() will wait for connection close instead of finishing immediately, blocking subsequent requests.

Useful? React with 👍 / 👎.

}

///|
/// Read the header of a HTTP/1.1 response from a HTTP reader.
/// This function must be called after the body of the last response is consumed,
Expand All @@ -196,7 +230,10 @@ async fn Reader::read_request(self : Reader) -> Request {
///
/// HTTP header fields are case insensitive.
/// In the result of `read_response`, all header fields will be in lower case.
async fn Reader::read_response(self : Reader) -> Response {
async fn Reader::read_response(
self : Reader,
request_method~ : RequestMethod?,
) -> Response {
guard self.body is Empty
guard self.transport.read_until("\r\n") is Some(response_line) else {
raise @io.ReaderClosed
Expand All @@ -213,6 +250,13 @@ async fn Reader::read_response(self : Reader) -> Response {
}
let reason = response_line[code_len + 1:].to_string()
let headers = self.read_headers()

// HTTP/1.1 Spec (RFC 7230 Section 3.3.3): Message Body Length Determination
// Other cases are handled in `read_headers` when parsing the headers, which will set the body kind accordingly.
if should_read_body_until_close(code, headers, self.body, request_method) {
self.body = WaitConnectionClose // Read until connection close
}

{ code, reason, headers }
}

Expand Down
Loading