Skip to content

Commit

Permalink
Merge branch 'main' into jarred/v8-stack-crash
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarred-Sumner authored Sep 27, 2024
2 parents c89c585 + d09df1a commit b492364
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 26 deletions.
2 changes: 1 addition & 1 deletion packages/bun-types/bun.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2282,7 +2282,7 @@ declare module "bun" {
*/
development?: boolean;

error?: (this: Server, request: ErrorLike) => Response | Promise<Response> | undefined | Promise<undefined>;
error?: (this: Server, error: ErrorLike) => Response | Promise<Response> | undefined | Promise<undefined>;

/**
* Uniquely identify a server instance with an ID
Expand Down
1 change: 0 additions & 1 deletion src/bun.js/bindings/ErrorCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "JavaScriptCore/ErrorType.h"
#include "JavaScriptCore/ObjectConstructor.h"
#include "JavaScriptCore/WriteBarrier.h"
#include "root.h"
#include "headers-handwritten.h"
#include "BunClientData.h"
#include "helpers.h"
Expand Down
5 changes: 5 additions & 0 deletions src/bun.js/bindings/JSBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,11 @@ static inline JSC::EncodedJSValue jsBufferConstructorFunction_allocBody(JSC::JSG
}
auto startPtr = uint8Array->typedVector() + start;
auto str_ = value.toWTFString(lexicalGlobalObject);
if (str_.isEmpty()) {
memset(startPtr, 0, length);
RELEASE_AND_RETURN(scope, JSC::JSValue::encode(uint8Array));
}

ZigString str = Zig::toZigString(str_);

if (UNLIKELY(!Bun__Buffer_fill(&str, startPtr, end - start, encoding))) {
Expand Down
1 change: 0 additions & 1 deletion src/bun.js/bindings/Weak.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include "root.h"
#include <JavaScriptCore/StrongInlines.h>
#include "BunClientData.h"
#include "root.h"
#include <JavaScriptCore/Weak.h>
#include <JavaScriptCore/Strong.h>

Expand Down
14 changes: 6 additions & 8 deletions src/bun.js/modules/NodeBufferModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,8 @@ JSC_DEFINE_HOST_FUNCTION(jsBufferConstructorFunction_isUtf8,
}

if (UNLIKELY(impl->isDetached())) {
throwTypeError(lexicalGlobalObject, throwScope,
"ArrayBuffer is detached"_s);
return {};
return Bun::ERR::INVALID_STATE(throwScope, lexicalGlobalObject,
"Cannot validate on a detached buffer"_s);
}

byteLength = impl->byteLength();
Expand Down Expand Up @@ -82,9 +81,8 @@ JSC_DEFINE_HOST_FUNCTION(jsBufferConstructorFunction_isAscii,
if (bufferView) {

if (UNLIKELY(bufferView->isDetached())) {
throwTypeError(lexicalGlobalObject, throwScope,
"ArrayBufferView is detached"_s);
return {};
return Bun::ERR::INVALID_STATE(throwScope, lexicalGlobalObject,
"Cannot validate on a detached buffer"_s);
}

byteLength = bufferView->byteLength();
Expand Down Expand Up @@ -169,15 +167,15 @@ DEFINE_NATIVE_MODULE(NodeBuffer) {
JSC::jsNumber(MAX_ARRAY_BUFFER_SIZE));

put(JSC::Identifier::fromString(vm, "kStringMaxLength"_s),
JSC::jsNumber(std::numeric_limits<unsigned>().max()));
JSC::jsNumber(WTF::String::MaxLength));

JSC::JSObject *constants = JSC::constructEmptyObject(
lexicalGlobalObject, globalObject->objectPrototype(), 2);
constants->putDirect(vm, JSC::Identifier::fromString(vm, "MAX_LENGTH"_s),
JSC::jsNumber(MAX_ARRAY_BUFFER_SIZE));
constants->putDirect(vm,
JSC::Identifier::fromString(vm, "MAX_STRING_LENGTH"_s),
JSC::jsNumber(std::numeric_limits<unsigned>().max()));
JSC::jsNumber(WTF::String::MaxLength));

put(JSC::Identifier::fromString(vm, "constants"_s), constants);

Expand Down
4 changes: 3 additions & 1 deletion test/js/bun/http/body-leak-test-fixture.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions test/js/bun/http/serve-body-leak.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const payload = Buffer.alloc(512 * 1024, "1").toString("utf-8"); // decent size
const batchSize = 40;
const totalCount = 10_000;
const zeroCopyPayload = new Blob([payload]);
const zeroCopyJSONPayload = new Blob([JSON.stringify({ bun: payload })]);

let url: URL;
let process: Subprocess<"ignore", "pipe", "inherit"> | null = null;
Expand Down Expand Up @@ -63,6 +64,13 @@ async function callBuffering() {
}).then(res => res.text());
expect(result).toBe("Ok");
}
async function callJSONBuffering() {
const result = await fetch(`${url.origin}/json-buffering`, {
method: "POST",
body: zeroCopyJSONPayload,
}).then(res => res.text());
expect(result).toBe("Ok");
}

async function callBufferingBodyGetter() {
const result = await fetch(`${url.origin}/buffering+body-getter`, {
Expand Down Expand Up @@ -142,6 +150,7 @@ async function calculateMemoryLeak(fn: () => Promise<void>) {
for (const test_info of [
["#10265 should not leak memory when ignoring the body", callIgnore, false, 64],
["should not leak memory when buffering the body", callBuffering, false, 64],
["should not leak memory when buffering a JSON body", callJSONBuffering, false, 64],
["should not leak memory when buffering the body and accessing req.body", callBufferingBodyGetter, false, 64],
["should not leak memory when streaming the body", callStreaming, false, 64],
["should not leak memory when streaming the body incompletely", callIncompleteStreaming, false, 64],
Expand Down
8 changes: 6 additions & 2 deletions test/js/node/buffer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,10 @@ for (let withOverridenBufferWrite of [false, true]) {
expect(buf[4]).toBe(128);
});

it("fill(N, empty string) should be the same as fill(N) and not include any uninitialized bytes", () => {
expect(Buffer.alloc(100, "")).toEqual(Buffer.alloc(100));
});

// https://github.com/joyent/node/issues/1758
it("check for fractional length args, junk length args, etc.", () => {
// Call .fill() first, stops valgrind warning about uninitialized memory reads.
Expand Down Expand Up @@ -2003,9 +2007,9 @@ for (let withOverridenBufferWrite of [false, true]) {

it("constants", () => {
expect(BufferModule.constants.MAX_LENGTH).toBe(4294967296);
expect(BufferModule.constants.MAX_STRING_LENGTH).toBe(4294967295);
expect(BufferModule.constants.MAX_STRING_LENGTH).toBe(2147483647);
expect(BufferModule.default.constants.MAX_LENGTH).toBe(4294967296);
expect(BufferModule.default.constants.MAX_STRING_LENGTH).toBe(4294967295);
expect(BufferModule.default.constants.MAX_STRING_LENGTH).toBe(2147483647);
});

it("File", () => {
Expand Down
16 changes: 9 additions & 7 deletions test/js/node/test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,12 +409,14 @@ if (process.env.NODE_TEST_KNOWN_GLOBALS !== '0') {
return leaked;
}

process.on('exit', function() {
const leaked = leakedGlobals();
if (leaked.length > 0) {
assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
}
});
// --- Commmented out for Bun ---
// process.on('exit', function() {
// const leaked = leakedGlobals();
// if (leaked.length > 0) {
// assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
// }
// });
// --- Commmented out for Bun ---
}

const mustCallChecks = [];
Expand Down Expand Up @@ -971,7 +973,7 @@ function expectRequiredModule(mod, expectation) {
}

const common = {
allowGlobals,
allowGlobals: [],
buildType,
canCreateSymLink,
childShouldThrowAndAbort,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ test("exec with timeout and killSignal", done => {
(err, stdout, stderr) => {
console.log("[stdout]", stdout.trim());
console.log("[stderr]", stderr.trim());

expect(err.killed).toBe(true);
expect(err.code).toBeNull();
expect(err.signal).toBe("SIGKILL");
expect(err.cmd).toBe(cmd);
expect(stdout.trim()).toBe("");
expect(stderr.trim()).toBe("");

expect(err?.killed).toBe(true);
expect(err?.code).toBeNull();
expect(err?.signal).toBe("SIGKILL");
expect(err?.cmd).toBe(cmd);
done();
},
);
Expand Down

0 comments on commit b492364

Please sign in to comment.