diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index 5295b888775d..d89e51914bae 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -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", {}); diff --git a/src/http/AsyncHTTP.rs b/src/http/AsyncHTTP.rs index a632cc72ec67..66b5d771d46f 100644 --- a/src/http/AsyncHTTP.rs +++ b/src/http/AsyncHTTP.rs @@ -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> { @@ -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, diff --git a/src/http/lib.rs b/src/http/lib.rs index bdf241a2578d..00760388b11c 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -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, @@ -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, @@ -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. @@ -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) @@ -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 @@ -814,9 +812,6 @@ pub struct HTTPClient<'a> { pub(crate) custom_ssl_ctx: Option>, 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>, @@ -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; @@ -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") @@ -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::(err, socket); @@ -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 { 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; @@ -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 @@ -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; } // Don't do this for proxies because those connections will be open for awhile. diff --git a/src/install/NetworkTask.rs b/src/install/NetworkTask.rs index fe70b5b5e4b2..aec21c1bc654 100644 --- a/src/install/NetworkTask.rs +++ b/src/install/NetworkTask.rs @@ -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); @@ -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 { @@ -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(()) } diff --git a/test/js/web/fetch/fetch.test.ts b/test/js/web/fetch/fetch.test.ts index 1672c07ece4f..621c5e63a45c 100644 --- a/test/js/web/fetch/fetch.test.ts +++ b/test/js/web/fetch/fetch.test.ts @@ -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 = { "/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. @@ -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 @@ -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 },