Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
42 changes: 33 additions & 9 deletions src/jsc/bindings/webcore/JSMIMEParams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,48 +165,64 @@ static String removeBackslashes(const StringView& view)
return builder.toString();
}

static void escapeQuoteOrBackslash(const StringView& view, StringBuilder& builder)
// Returns the characters it adds. The builder's own length stops at String::MaxLength.
static uint64_t escapeQuoteOrBackslash(const StringView& view, StringBuilder& builder)
{
if (view.find([](char16_t c) { return c == '"' || c == '\\'; }) == notFound) {
builder.append(view);
return;
return view.length();
}

uint64_t length = 0;
if (view.is8Bit()) {
auto span = view.span8();
for (Latin1Character c : span) {
if (c == '"' || c == '\\') {
builder.append('\\');
length++;
}
builder.append(c);
length++;
}
} else {
auto span = view.span16();
for (char16_t c : span) {
if (c == '"' || c == '\\') {
builder.append('\\');
length++;
}
builder.append(c);
length++;
}
}
return length;
}

// Encodes a parameter value for serialization.
static void encodeParamValue(const StringView& value, StringBuilder& builder)
// Encodes a parameter value for serialization. Returns the characters it adds.
static uint64_t encodeParamValue(const StringView& value, StringBuilder& builder)
{
if (value.isEmpty()) {
builder.append("\"\""_s);
return;
return 2;
}
if (findFirstInvalidHTTPTokenChar(value) == -1) {
// It's a simple token, no quoting needed.
builder.append(value);
return;
return value.length();
}
// Needs quoting and escaping.
builder.append('"');
escapeQuoteOrBackslash(value, builder);
uint64_t length = escapeQuoteOrBackslash(value, builder);
builder.append('"');
return length + 2;
}

JSC::EncodedJSValue throwMIMEStringBuildFailure(JSGlobalObject* globalObject, ThrowScope& scope, uint64_t intendedLength)
{
if (intendedLength > WTF::String::MaxLength) {
return Bun::ERR::STRING_TOO_LONG(scope, globalObject);
}
return Bun::ERR::MEMORY_ALLOCATION_FAILED(scope, globalObject);
}

// Parses the parameter string and populates the map.
Expand Down Expand Up @@ -516,7 +532,9 @@ JSC_DEFINE_HOST_FUNCTION(jsMIMEParamsProtoFuncToString, (JSGlobalObject * global
}

JSMap* map = thisObject->jsMap();
StringBuilder builder;
// RecordOverflow: the default policy aborts the process past String::MaxLength.
StringBuilder builder(WTF::OverflowPolicy::RecordOverflow);
uint64_t intendedLength = 0;
bool first = true;

JSMapIterator* iterator = JSMapIterator::create(vm, globalObject->mapIteratorStructure(), map, IterationKind::Entries);
Expand All @@ -540,12 +558,18 @@ JSC_DEFINE_HOST_FUNCTION(jsMIMEParamsProtoFuncToString, (JSGlobalObject * global

if (!first) {
builder.append(';');
intendedLength++;
}
first = false;

builder.append(key);
builder.append('=');
encodeParamValue(value, builder);
intendedLength += static_cast<uint64_t>(key.length()) + 1;
intendedLength += encodeParamValue(value, builder);
}

if (builder.hasOverflowed()) [[unlikely]] {
return throwMIMEStringBuildFailure(globalObject, scope, intendedLength);
}

return JSValue::encode(jsString(vm, builder.toString()));
Expand Down
3 changes: 3 additions & 0 deletions src/jsc/bindings/webcore/JSMIMEParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,7 @@ void setupJSMIMEParamsClassStructure(JSC::LazyClassStructure::Initializer&);
JSC::JSValue createJSMIMEBinding(Zig::GlobalObject* globalObject);
bool parseMIMEParamsString(JSGlobalObject* globalObject, JSMap* map, StringView input);

// Throws for a MIME string that could not be built. `intendedLength` is the length it would have had.
JSC::EncodedJSValue throwMIMEStringBuildFailure(JSC::JSGlobalObject*, JSC::ThrowScope&, uint64_t intendedLength);

} // namespace WebCore
18 changes: 16 additions & 2 deletions src/jsc/bindings/webcore/JSMIMEType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,14 @@ JSC_DEFINE_CUSTOM_GETTER(jsMIMETypeProtoGetterEssence, (JSGlobalObject * globalO
return {};
}

String essence = makeString(thisObject->type(), '/', thisObject->subtype());
// makeString aborts the process when the result does not fit in a string.
const String& type = thisObject->type();
const String& subtype = thisObject->subtype();
String essence = tryMakeString(type, '/', subtype);
if (essence.isNull()) [[unlikely]] {
return throwMIMEStringBuildFailure(globalObject, scope, static_cast<uint64_t>(type.length()) + 1 + subtype.length());
}

return JSValue::encode(jsString(vm, essence));
}

Expand Down Expand Up @@ -465,13 +472,20 @@ JSC_DEFINE_HOST_FUNCTION(jsMIMETypeProtoFuncToString, (JSGlobalObject * globalOb
String paramsStr = paramsStrValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, {});

StringBuilder builder;
// RecordOverflow: the default policy aborts the process past String::MaxLength.
StringBuilder builder(WTF::OverflowPolicy::RecordOverflow);
builder.append(thisObject->type());
builder.append('/');
builder.append(thisObject->subtype());
uint64_t intendedLength = static_cast<uint64_t>(thisObject->type().length()) + 1 + thisObject->subtype().length();
if (!paramsStr.isEmpty()) {
builder.append(';');
builder.append(paramsStr);
intendedLength += static_cast<uint64_t>(paramsStr.length()) + 1;
}

if (builder.hasOverflowed()) [[unlikely]] {
return throwMIMEStringBuildFailure(globalObject, scope, intendedLength);
}

return JSValue::encode(jsString(vm, builder.toString()));
Expand Down
142 changes: 141 additions & 1 deletion test/js/node/util/mime-api.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, test } from "bun:test";
import { bunExe, expectRssDeltaBelow } from "harness";
import { bunEnv, bunExe, expectRssDeltaBelow, isDebug } from "harness";
import { totalmem } from "node:os";
import { MIMEParams, MIMEType } from "util";

describe("MIME API", () => {
Expand Down Expand Up @@ -457,3 +458,142 @@ test("MIMEType releases its type and subtype strings when garbage-collected", as
// Unfixed: ~42 MiB (160 x 256 KiB). Fixed: 1 to 4 MiB of allocator slack.
await expectRssDeltaBelow(["--smol", "-e", code], { release: 20, debug: 25 });
});

// The two toString implementations build their result with a WTF
// StringBuilder, and the essence getter with makeString. Both primitives call
// CRASH() when the result passes String::MaxLength (2 ** 31 - 1 characters)
// and when the allocation fails, so a large type or parameter made each of
// them abort the process with no output. Each case below runs in a child,
// because a child that aborts prints nothing and still lets the parent assert.
describe("a MIME string that does not fit in a string", () => {
const tooLong = "Error ERR_STRING_TOO_LONG: Cannot create a string longer than 2147483647 characters\n";
const allocationFailed = "RangeError ERR_MEMORY_ALLOCATION_FAILED: Failed to allocate memory\n";

async function buildInChild(body: string, env: Record<string, string | undefined> = bunEnv) {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const { MIMEType, MIMEParams } = require("node:util");
try {
console.log("string of length " + ((() => { ${body} })()).length);
} catch (e) {
console.log(e.name + " " + e.code + ": " + e.message);
}`,
],
env,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode };
}

// The child holds 2 GiB of strings. `repeat` builds each one in a single
// allocation, which `Buffer.alloc(...).toString()` does not.
describe.skipIf(totalmem() < 10 * 1024 ** 3)("a result past String::MaxLength", () => {
test("MIMEType.prototype.toString", async () => {
expect(
await buildInChild(`
const mime = new MIMEType("text/plain");
MIMEParams.prototype.toString = () => "a".repeat(2 ** 31 - 1);
return String(mime);
`),
).toEqual({ stdout: tooLong, stderr: "", exitCode: 0 });
}, 120_000);

// A debug build validates a parameter name and a type at about 25 ns per
// character and escapes a value at about 200 ns per character, so the three
// cases below take one to four minutes each there. The case above covers
// both builds, and the capped block below covers these three.
test.skipIf(isDebug)(
"MIMEParams.prototype.toString with a long parameter name",
async () => {
expect(
await buildInChild(`
const params = new MIMEParams();
params.set("a".repeat(2 ** 31 - 1), "x");
return String(params);
`),
).toEqual({ stdout: tooLong, stderr: "", exitCode: 0 });
},
120_000,
);

// Each quote gets a backslash, so 2 ** 30 of them serialize to 2 ** 31 characters.
test.skipIf(isDebug)(
"MIMEParams.prototype.toString with a value that escaping doubles",
async () => {
expect(
await buildInChild(`
const mime = new MIMEType("text/plain");
mime.params.set("a", '"'.repeat(2 ** 30));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
return String(mime);
`),
).toEqual({ stdout: tooLong, stderr: "", exitCode: 0 });
},
120_000,
);

test.skipIf(isDebug)(
"MIMEType#essence",
async () => {
expect(
await buildInChild(`
const mime = new MIMEType("text/plain");
const half = "a".repeat(2 ** 30);
mime.type = half;
mime.subtype = half;
return mime.essence;
`),
).toEqual({ stdout: tooLong, stderr: "", exitCode: 0 });
},
120_000,
);
});

// BUN_JSC_maxSingleAllocationSize (debug WTF only) fails every allocation
// above the cap. That is the other way each primitive aborts, and it needs
// 9 MiB instead of 2 GiB. Node reports a failed string allocation as
// ERR_MEMORY_ALLOCATION_FAILED, which is what #41066 settled on.
describe.skipIf(!isDebug)("a failed allocation", () => {
const capped = { ...bunEnv, BUN_JSC_maxSingleAllocationSize: String(4 * 1024 * 1024) };

test("MIMEParams.prototype.toString", async () => {
expect(
await buildInChild(
`const params = new MIMEParams();
const value = Buffer.alloc(3 * 1024 * 1024, 97).toString("latin1");
for (const name of ["a", "b", "c"]) params.set(name, value);
return String(params);`,
capped,
),
).toEqual({ stdout: allocationFailed, stderr: "", exitCode: 0 });
});

// 3 MiB of quotes escape to 6 MiB, so the builder fails to grow inside the escape loop.
test("MIMEParams.prototype.toString with a value that escaping doubles", async () => {
expect(
await buildInChild(
`const params = new MIMEParams();
params.set("a", Buffer.alloc(3 * 1024 * 1024, '"').toString("latin1"));
return String(params);`,
capped,
),
).toEqual({ stdout: allocationFailed, stderr: "", exitCode: 0 });
});

test("MIMEType#essence", async () => {
expect(
await buildInChild(
`const mime = new MIMEType("text/plain");
const half = Buffer.alloc(3 * 1024 * 1024, 97).toString("latin1");
mime.type = half;
mime.subtype = half;
return mime.essence;`,
capped,
),
).toEqual({ stdout: allocationFailed, stderr: "", exitCode: 0 });
});
});
});