diff --git a/src/http/SendFile.rs b/src/http/SendFile.rs index 2c7ed2d2236d..2294e740d290 100644 --- a/src/http/SendFile.rs +++ b/src/http/SendFile.rs @@ -57,6 +57,9 @@ impl SendFile { if errcode != bun_sys::E::SUCCESS || self.remain == 0 || val == 0 { if errcode == bun_sys::E::SUCCESS { + if self.remain > 0 { + return Status::Err(crate::Error::RequestBodyTruncated); + } return Status::Done; } @@ -87,6 +90,9 @@ impl SendFile { self.remain = (self.remain as u64).saturating_sub(wrote) as usize; if errcode != bun_sys::E::EAGAIN || self.remain == 0 || sbytes == 0 { if errcode == bun_sys::E::SUCCESS { + if self.remain > 0 { + return Status::Err(crate::Error::RequestBodyTruncated); + } return Status::Done; } return Status::Err(bun_errno::from_errno(errcode as i32).into()); @@ -118,6 +124,9 @@ impl SendFile { self.remain = (self.remain as u64).saturating_sub(wrote) as usize; if errcode != bun_sys::E::EAGAIN || self.remain == 0 || sbytes == 0 { if errcode == bun_sys::E::SUCCESS { + if self.remain > 0 { + return Status::Err(crate::Error::RequestBodyTruncated); + } return Status::Done; } diff --git a/src/http/error.rs b/src/http/error.rs index b7edb71da82a..c9c2f270ba0c 100644 --- a/src/http/error.rs +++ b/src/http/error.rs @@ -63,6 +63,8 @@ pub enum Error { UnsupportedTransferEncoding, #[error("RequestBodyNotReusable")] RequestBodyNotReusable, + #[error("RequestBodyTruncated")] + RequestBodyTruncated, #[error("UnsupportedRedirectProtocol")] UnsupportedRedirectProtocol, #[error("RedirectURLTooLong")] @@ -291,6 +293,7 @@ impl Error { Self::InvalidContentLength => "InvalidContentLength", Self::UnsupportedTransferEncoding => "UnsupportedTransferEncoding", Self::RequestBodyNotReusable => "RequestBodyNotReusable", + Self::RequestBodyTruncated => "RequestBodyTruncated", Self::UnsupportedRedirectProtocol => "UnsupportedRedirectProtocol", Self::RedirectURLTooLong => "RedirectURLTooLong", Self::RedirectURLInvalid => "RedirectURLInvalid", diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index c3eefc24d331..531c8dd00a17 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -1350,6 +1350,9 @@ impl FetchTasklet { http::Error::RedirectURLInvalid => { BunString::static_("Redirect URL in Location header is invalid.") } + http::Error::RequestBodyTruncated => BunString::static_( + "Request body source reached EOF before the advertised Content-Length was sent (the file was truncated while uploading)", + ), http::Error::Cert(http::CertError::UNABLE_TO_GET_ISSUER_CERT) => { BunString::static_("unable to get issuer certificate") diff --git a/test/js/bun/http/fetch-file-upload.test.ts b/test/js/bun/http/fetch-file-upload.test.ts index b779e3b6c55a..af1e1fccbc0d 100644 --- a/test/js/bun/http/fetch-file-upload.test.ts +++ b/test/js/bun/http/fetch-file-upload.test.ts @@ -1,5 +1,7 @@ import { expect, test } from "bun:test"; -import { isBroken, isWindows, withoutAggressiveGC } from "harness"; +import { isBroken, isWindows, tempDir, withoutAggressiveGC } from "harness"; +import fs from "node:fs"; +import net from "node:net"; import { tmpdir } from "os"; import { join } from "path"; @@ -159,6 +161,87 @@ test.todoIf(isBroken && isWindows)( 10_000, ); +// The sendfile path is only taken on POSIX for a plain-HTTP file body +// >= 32 KB; on Windows the file is read fully into memory before the +// request is sent, so a later truncation is not observable. +test.skipIf(isWindows)("fetch rejects when the Bun.file body is truncated mid-upload", async () => { + const size = 32 * 1024 * 1024; + using dir = tempDir("fetch-sendfile-truncate", {}); + const p = join(String(dir), "body.bin"); + { + const fd = fs.openSync(p, "w"); + const chunk = Buffer.alloc(1024 * 1024, 83); + for (let i = 0; i < size / chunk.length; i++) fs.writeSync(fd, chunk); + fs.closeSync(fd); + } + + let head = ""; + let buf = ""; + let socket: net.Socket | undefined; + const gotHead = Promise.withResolvers(); + const socketClosed = Promise.withResolvers(); + const server = net.createServer(s => { + socket = s; + s.once("close", () => { + gotHead.reject(new Error("socket closed before request head")); + socketClosed.resolve(); + }); + s.on("error", e => { + gotHead.reject(e); + socketClosed.resolve(); + }); + s.on("data", d => { + if (head !== "") return; + buf += d.toString("latin1"); + const i = buf.indexOf("\r\n\r\n"); + if (i === -1) return; + head = buf.slice(0, i); + s.pause(); + gotHead.resolve(); + }); + }); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + try { + const port = (server.address() as net.AddressInfo).port; + const req = fetch(`http://127.0.0.1:${port}/upload`, { + method: "POST", + body: Bun.file(p), + }); + + await gotHead.promise; + expect(head).toContain(`Content-Length: ${size}`); + + // With the server paused, sendfile fills the kernel send buffer and + // then parks on EAGAIN with the file offset well short of `size`. + // Truncating below that offset means the next sendfile(2) call returns + // 0 with bytes still owed. + fs.truncateSync(p, 64 * 1024); + socket!.resume(); + + let err: any; + try { + await req; + } catch (e) { + err = e; + } + expect(err).toBeDefined(); + expect(err?.code).toBe("RequestBodyTruncated"); + + // The client must close the connection so the origin is not left + // holding a half-sent body. On macOS the server's read side does not + // observe the client's RST on its own, so write a byte the way a real + // origin would try to respond; that surfaces ECONNRESET and 'close'. + socket!.write("\r\n"); + await socketClosed.promise; + } finally { + socket?.destroy(); + server.close(); + } +}); + test("missing file throws the expected error", async () => { Bun.gc(true); // Run this 1000 times to check for GC bugs