Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/http/SendFile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 3 additions & 0 deletions src/http/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ pub enum Error {
UnsupportedTransferEncoding,
#[error("RequestBodyNotReusable")]
RequestBodyNotReusable,
#[error("RequestBodyTruncated")]
RequestBodyTruncated,
#[error("UnsupportedRedirectProtocol")]
UnsupportedRedirectProtocol,
#[error("RedirectURLTooLong")]
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,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")
Expand Down
82 changes: 81 additions & 1 deletion test/js/bun/http/fetch-file-upload.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -159,6 +161,84 @@ 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<void>();
const socketClosed = Promise.withResolvers<void>();
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();
});
Comment thread
robobun marked this conversation as resolved.
});
await new Promise<void>((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.
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
Expand Down
Loading