Skip to content
Open
1 change: 0 additions & 1 deletion src/bun_core/env_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ pub mod feature_flag {
new_feature_flag!(pub BUN_INTERNAL_SUPPRESS_CRASH_IN_BUN_RUN, "BUN_INTERNAL_SUPPRESS_CRASH_IN_BUN_RUN", {});
new_feature_flag!(pub BUN_INTERNAL_SUPPRESS_CRASH_ON_NAPI_ABORT, "BUN_INTERNAL_SUPPRESS_CRASH_ON_NAPI_ABORT", {});
new_feature_flag!(pub BUN_INTERNAL_SUPPRESS_CRASH_ON_UV_STUB, "BUN_INTERNAL_SUPPRESS_CRASH_ON_UV_STUB", {});
new_feature_flag!(pub BUN_FEATURE_FLAG_LAST_MODIFIED_PRETEND_304, "BUN_FEATURE_FLAG_LAST_MODIFIED_PRETEND_304", {});
new_feature_flag!(pub BUN_NO_CODESIGN_MACHO_BINARY, "BUN_NO_CODESIGN_MACHO_BINARY", {});
new_feature_flag!(pub BUN_FEATURE_FLAG_NO_LIBDEFLATE, "BUN_FEATURE_FLAG_NO_LIBDEFLATE", {});
new_feature_flag!(pub BUN_TRACE, "BUN_TRACE", {});
Expand Down
3 changes: 1 addition & 2 deletions src/http/AsyncHTTP.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ bun_core::declare_scope!(AsyncHTTP, visible);

// Lifetime `'a` covers every borrowed input the caller hands in: `url`,
// `http_proxy`, `request_header_buf`, the borrowed `HTTPRequestBody::Bytes`
// payload, and `client.{header_buf,unix_socket_path,if_modified_since}`.
// payload, and `client.{header_buf,unix_socket_path}`.
// Intrusive fields (`real`, `next`) are raw pointers and thus lifetime-erased;
// the HTTP-thread copy uses the same `'a` as the JS-thread original it mirrors.
pub struct AsyncHTTP<'a> {
Expand Down Expand Up @@ -177,7 +177,6 @@ fn make_client<'a>(
tls_props: None,
custom_ssl_ctx: None,
result_callback: noop_callback(),
if_modified_since: b"",
request_content_len_buf: [0u8; b"18446744073709551615".len()],
http_proxy,
proxy_settings: None,
Expand Down
50 changes: 9 additions & 41 deletions src/http/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ pub struct Flags {
/// `PooledSocket::verification` of the socket this request took from the
/// pool, so a weaker request re-pooling it doesn't downgrade the record.
pub(crate) reused_socket_verification: PeerVerification,
pub force_last_modified: bool,
pub(crate) redirected: bool,
pub(crate) proxy_tunneling: bool,
pub reject_unauthorized: bool,
Expand All @@ -215,7 +214,6 @@ impl Default for Flags {
disable_decompression: false,
did_have_handshaking_error: false,
reused_socket_verification: PeerVerification::None,
force_last_modified: false,
redirected: false,
proxy_tunneling: false,
reject_unauthorized: true,
Expand Down Expand Up @@ -380,7 +378,7 @@ impl HTTPClient<'_> {
/// undo the HTTP/1.1-specific framing decisions that don't apply when the
/// transport delimits the body (h2 DATA frames / h3 STREAM frames).
///
/// Returns the (possibly-mutated) response so the caller can pass it to
/// Returns the response so the caller can pass it to
/// `clone_metadata` once the redirect decision has been made; the borrow of
/// `headers` flows through, so the deep copy is checked by the compiler
/// rather than by a call-ordering contract.
Expand All @@ -390,14 +388,14 @@ impl HTTPClient<'_> {
status_code: u32,
headers: &'h [picohttp::Header],
) -> crate::Result<(HeaderResult, picohttp::Response<'h>)> {
let mut response = picohttp::Response {
let response = picohttp::Response {
minor_version: 0,
status_code,
status: b"",
headers: picohttp::HeaderList { list: headers },
bytes_read: 0,
};
let should_continue = self.handle_response_metadata(&mut response)?;
let should_continue = self.handle_response_metadata(&response)?;
// h2/h3 framing delimits the body; chunked transfer-encoding and the
// HTTP/1.x persistence rules (no Content-Length ⇒ no keep-alive, and the
// HTTP/1.0 default that the synthetic `minor_version: 0` above trips)
Expand Down Expand Up @@ -770,7 +768,7 @@ fn no_proxy_matches(no_proxy_text: &[u8], hostname: &[u8], host: &[u8]) -> bool
// Many of these fields can be moved to a packed struct and use less space
//
// Lifetime `'a` ties every borrowed input — `url`, `http_proxy`, `header_buf`,
// `if_modified_since`, `unix_socket_path`, and the borrowed
// `unix_socket_path`, and the borrowed
// `HTTPRequestBody::Bytes` payload — to the caller's storage. The original port erased these to `'static`
// and lifetime-erased at every call site; threading the lifetime removes that hazard.
// Intrusive raw-pointer backrefs (socket ext, h2/h3 streams) store the
Expand Down Expand Up @@ -814,9 +812,6 @@ pub struct HTTPClient<'a> {
pub(crate) custom_ssl_ctx: Option<RefPtr<HttpsContext>>,
pub(crate) result_callback: HTTPClientResultCallback,

/// Some HTTP servers (such as npm) report Last-Modified times but ignore If-Modified-Since.
/// This is a workaround for that.
pub if_modified_since: &'a [u8],
pub(crate) request_content_len_buf: [u8; b"18446744073709551615".len()],

pub(crate) http_proxy: Option<URL<'a>>,
Expand Down Expand Up @@ -2400,15 +2395,6 @@ impl<'a> HTTPClient<'a> {
}
}
}
h if h == hash_header_const(b"if-modified-since") => {
if self.flags.force_last_modified && self.if_modified_since.is_empty() {
// SAFETY: header_str() returns a slice into self.header_buf which outlives
// this client; lifetime is erased here only because we don't yet thread
// struct lifetime params. The borrow is valid for the life of `self`.
self.if_modified_since =
unsafe { bun_ptr::detach_lifetime(self.header_str(header_values[i])) };
}
}
h if h == hash_header_const(HOST_HEADER_NAME) => {
if will_append {
override_host_header = true;
Expand Down Expand Up @@ -3633,7 +3619,7 @@ impl<'a> HTTPClient<'a> {
}

let shared_resp = scratch::response_headers();
let mut response = loop {
let response = loop {
let mut amount_read: usize = 0;

// minimal http/1.1 response is 16 bytes ("HTTP/1.1 200\r\n\r\n")
Expand Down Expand Up @@ -3707,7 +3693,7 @@ impl<'a> HTTPClient<'a> {

break parsed;
};
let should_continue = match self.handle_response_metadata(&mut response) {
let should_continue = match self.handle_response_metadata(&response) {
Ok(s) => s,
Err(err) => {
self.close_and_fail::<IS_SSL>(err, socket);
Expand Down Expand Up @@ -4722,10 +4708,9 @@ impl<'a> HTTPClient<'a> {

pub(crate) fn handle_response_metadata(
&mut self,
response: &mut picohttp::Response,
response: &picohttp::Response,
) -> crate::Result<ShouldContinue> {
let mut location: &[u8] = b"";
let mut pretend_304 = false;
let mut is_server_sent_events = false;
let mut content_codings: u32 = 0;
let mut has_keep_alive_token = false;
Expand Down Expand Up @@ -4830,13 +4815,6 @@ impl<'a> HTTPClient<'a> {
None => {}
}
}
h if h == hash_header_const(b"Last-Modified") => {
pretend_304 = self.flags.force_last_modified
&& response.status_code > 199
&& response.status_code < 300
&& !self.if_modified_since.is_empty()
&& self.if_modified_since == header.value();
}
h if h == hash_header_const(b"Alt-Svc") => {
// Record regardless of *this* request's shape — a future
// request to the same origin may be h3-eligible even if this
Expand All @@ -4861,23 +4839,13 @@ impl<'a> HTTPClient<'a> {
print_response(response);
}

if pretend_304 {
response.status_code = 304;
}

// According to RFC 7230 section 3.3.3:
// 1. Any response to a HEAD request and any response with a 1xx (Informational),
// 204 (No Content), or 304 (Not Modified) status code
// [...] cannot contain a message body or trailer section.
// Therefore in these cases set content-length to 0, so the response body is always ignored
// and is not waited for (which could cause a timeout).
// This applies regardless of whether we're using a proxy tunnel or not,
// since these status codes NEVER have a body per the HTTP spec.
// RFC 9112 §6.3: 1xx/204/304 have no body regardless of framing headers.
if (response.status_code >= 100 && response.status_code < 200)
|| response.status_code == 204
|| response.status_code == 304
{
self.state.content_length = Some(0);
self.state.transfer_encoding = Encoding::Identity;
Comment thread
robobun marked this conversation as resolved.
Resolved
}
Comment thread
robobun marked this conversation as resolved.
Resolved

// Don't do this for proxies because those connections will be open for awhile.
Expand Down
27 changes: 1 addition & 26 deletions src/install/NetworkTask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,10 +613,6 @@ impl NetworkTask {
ACCEPT_HEADER_VALUE
};
header_builder.count("Accept", accept_header);
let trailing_last_modified = !last_modified.is_empty() && !etag.is_empty();
if trailing_last_modified {
header_builder.content.count(last_modified);
}
header_builder.allocate()?;

append_auth(&mut header_builder, scope);
Expand All @@ -629,18 +625,9 @@ impl NetworkTask {

header_builder.append("Accept", accept_header);

let last_modified_start = header_builder.content.len;
if trailing_last_modified {
let _ = header_builder.content.append(last_modified);
}
debug_assert_eq!(header_builder.content.len, header_builder.content.cap);
self.header_buf = header_builder.content.move_to_slice();
if trailing_last_modified {
// SAFETY: `self.header_buf` outlives the request; it is freed when the slot returns to the pool.
last_modified =
unsafe { bun_ptr::detach_lifetime(&self.header_buf[last_modified_start..]) };
}
// SAFETY: same invariant as `last_modified` above.
// SAFETY: `self.header_buf` outlives the request; it is freed when the slot returns to the pool.
unsafe { bun_ptr::detach_lifetime(&*self.header_buf) }
} else {
let header_buf: &'static str = if needs_extended {
Expand Down Expand Up @@ -704,18 +691,6 @@ impl NetworkTask {
self.http_mut().client.verbose = HTTPVerboseLevel::Headers;
}

// Incase the ETag causes invalidation, we fallback to the last modified date.
if !last_modified.is_empty()
&& bun_core::env_var::feature_flag::BUN_FEATURE_FLAG_LAST_MODIFIED_PRETEND_304
.get()
.unwrap_or(false)
{
self.http_mut().client.flags.force_last_modified = true;
// SAFETY: `last_modified` points into `self.header_buf` or into the manifest `string_buf` cloned into `self.callback`; both outlive the request.
self.http_mut().client.if_modified_since =
unsafe { bun_ptr::detach_lifetime(last_modified) };
}

Ok(())
}

Expand Down
18 changes: 18 additions & 0 deletions test/js/web/fetch/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3201,9 +3201,21 @@ it("does not reuse a keep-alive connection when bytes follow a response that end
// redirect path, which pools the socket itself), so trailing bytes must cost the
// connection while the same responses without them must still be pooled.
const injected = "HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nContent-Length: 8\r\n\r\ninjected";
// A well-formed chunked body. RFC 9112 §6.3: a 204/304 ends at its header block no
// matter what framing headers it carries, so these bytes are trailing junk too, not
// a body for the chunked decoder to consume.
const chunkedBody = "5\r\nhello\r\n0\r\n\r\n";
const responses: Record<string, { head: string; junk: string }> = {
"/204": { head: "HTTP/1.1 204 No Content\r\nConnection: keep-alive\r\n\r\n", junk: injected },
"/304": { head: 'HTTP/1.1 304 Not Modified\r\nETag: "x"\r\nConnection: keep-alive\r\n\r\n', junk: injected },
"/204-chunked": {
head: "HTTP/1.1 204 No Content\r\nTransfer-Encoding: chunked\r\nConnection: keep-alive\r\n\r\n",
junk: chunkedBody,
},
"/304-chunked": {
head: 'HTTP/1.1 304 Not Modified\r\nETag: "x"\r\nTransfer-Encoding: chunked\r\nConnection: keep-alive\r\n\r\n',
junk: chunkedBody,
},
"/empty": { head: "HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: keep-alive\r\n\r\n", junk: injected },
// Answered with HEAD: Content-Length describes the GET body, nothing may follow.
// The junk variant sends that body anyway.
Expand Down Expand Up @@ -3262,6 +3274,8 @@ it("does not reuse a keep-alive connection when bytes follow a response that end
const cases: { path: string; init?: () => RequestInit }[] = [
{ path: "/204" },
{ path: "/304" },
{ path: "/204-chunked" },
{ path: "/304-chunked" },
{ path: "/empty" },
{ path: "/head", init: () => ({ method: "HEAD" }) },
// The redirect path only pools the socket once the upload is known to be
Expand Down Expand Up @@ -3310,6 +3324,10 @@ it("does not reuse a keep-alive connection when bytes follow a response that end
{ request: "/204", status: 204, body: "", newConnections: 0 },
{ request: "/304 + trailing bytes", status: 304, body: "", newConnections: 1 },
{ request: "/304", status: 304, body: "", newConnections: 0 },
{ request: "/204-chunked + trailing bytes", status: 204, body: "", newConnections: 1 },
{ request: "/204-chunked", status: 204, body: "", newConnections: 0 },
{ request: "/304-chunked + trailing bytes", status: 304, body: "", newConnections: 1 },
{ request: "/304-chunked", status: 304, body: "", newConnections: 0 },
{ request: "/empty + trailing bytes", status: 200, body: "", newConnections: 1 },
{ request: "/empty", status: 200, body: "", newConnections: 0 },
{ request: "/head + trailing bytes", status: 200, body: "", newConnections: 1 },
Expand Down
Loading