Skip to content

Commit

Permalink
Fix a couple important bugs (oven-sh#4560)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jarred-Sumner
Jarred-Sumner authored Sep 8, 2023
1 parent f6a621f commit 822a00c
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 42 deletions.
58 changes: 35 additions & 23 deletions src/bun.js/bindings/ImportMetaObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,14 +457,20 @@ void ImportMetaObject::finishCreation(VM& vm)
ImportMetaObject* meta = jsCast<ImportMetaObject*>(init.owner);

WTF::URL url = meta->url.startsWith('/') ? WTF::URL::fileURLWithFileSystemPath(meta->url) : WTF::URL(meta->url);
WTF::StringView path;
if (url.protocolIs("file"_s)) {
path = url.fileSystemPath();
WTF::String path;

if (url.isValid()) {

if (url.protocolIs("file"_s)) {
path = url.fileSystemPath();
} else {
path = url.path().toString();
}
} else {
path = url.path();
path = meta->url;
}

JSFunction* value = jsCast<JSFunction*>(Bun::JSCommonJSModule::createBoundRequireFunction(init.vm, meta->globalObject(), path.toString()));
JSFunction* value = jsCast<JSFunction*>(Bun::JSCommonJSModule::createBoundRequireFunction(init.vm, meta->globalObject(), path));
init.set(value);
});
this->urlProperty.initLater([](const JSC::LazyProperty<JSC::JSObject, JSC::JSString>::Initializer& init) {
Expand All @@ -477,12 +483,14 @@ void ImportMetaObject::finishCreation(VM& vm)
ImportMetaObject* meta = jsCast<ImportMetaObject*>(init.owner);

WTF::URL url = meta->url.startsWith('/') ? WTF::URL::fileURLWithFileSystemPath(meta->url) : WTF::URL(meta->url);
WTF::StringView dirname;
WTF::String dirname;

if (url.protocolIs("file"_s)) {
dirname = url.fileSystemPath();
} else {
dirname = url.path();
if (url.isValid()) {
if (url.protocolIs("file"_s)) {
dirname = url.fileSystemPath();
} else {
dirname = url.path().toString();
}
}

if (dirname.endsWith("/"_s)) {
Expand All @@ -491,42 +499,46 @@ void ImportMetaObject::finishCreation(VM& vm)
dirname = dirname.substring(0, dirname.reverseFind('/'));
}

init.set(jsString(init.vm, dirname.toString()));
init.set(jsString(init.vm, dirname));
});
this->fileProperty.initLater([](const JSC::LazyProperty<JSC::JSObject, JSC::JSString>::Initializer& init) {
ImportMetaObject* meta = jsCast<ImportMetaObject*>(init.owner);

WTF::URL url = meta->url.startsWith('/') ? WTF::URL::fileURLWithFileSystemPath(meta->url) : WTF::URL(meta->url);
WTF::StringView path;
if (url.protocolIs("file"_s)) {
path = url.fileSystemPath();
WTF::String path;

if (!url.isValid()) {
path = meta->url;
} else {
path = url.path();
if (url.protocolIs("file"_s)) {
path = url.fileSystemPath();
} else {
path = url.path().toString();
}
}

WTF::StringView filename;
WTF::String filename;

if (path.endsWith("/"_s)) {
filename = path.substring(path.reverseFind('/', path.length() - 2) + 1);
} else {
filename = path.substring(path.reverseFind('/') + 1);
}

init.set(jsString(init.vm, filename.toString()));
init.set(jsString(init.vm, filename));
});
this->pathProperty.initLater([](const JSC::LazyProperty<JSC::JSObject, JSC::JSString>::Initializer& init) {
ImportMetaObject* meta = jsCast<ImportMetaObject*>(init.owner);

WTF::URL url = meta->url.startsWith('/') ? WTF::URL::fileURLWithFileSystemPath(meta->url) : WTF::URL(meta->url);
WTF::StringView path;

if (url.protocolIs("file"_s)) {
path = url.fileSystemPath();
if (!url.isValid()) {
init.set(jsString(init.vm, meta->url));
} else if (url.protocolIs("file"_s)) {
init.set(jsString(init.vm, url.fileSystemPath()));
} else {
path = url.path();
init.set(jsString(init.vm, url.path()));
}

init.set(jsString(init.vm, path.toString()));
});
}

Expand Down
13 changes: 9 additions & 4 deletions src/bun.js/bindings/bindings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3144,11 +3144,16 @@ JSC__JSValue JSC__JSValue__getIfPropertyExistsImpl(JSC__JSValue JSValue0,
const unsigned char* arg1, uint32_t arg2)
{

JSValue value = JSC::JSValue::decode(JSValue0);
if (UNLIKELY(!value.isObject()))
return JSValue::encode({});

JSC::VM& vm = globalObject->vm();
JSC::JSObject* object = JSC::JSValue::decode(JSValue0).asCell()->getObject();
auto propertyName = JSC::PropertyName(
JSC::Identifier::fromString(vm, reinterpret_cast<const LChar*>(arg1), (int)arg2));
return JSC::JSValue::encode(object->getIfPropertyExists(globalObject, propertyName));
JSC::JSObject* object = value.getObject();
auto identifier = JSC::Identifier::fromString(vm, String(StringImpl::createWithoutCopying(arg1, arg2)));
auto property = JSC::PropertyName(identifier);

return JSC::JSValue::encode(object->getIfPropertyExists(globalObject, property));
}

JSC__JSValue JSC__JSValue__getIfPropertyExistsFromPath(JSC__JSValue JSValue0, JSC__JSGlobalObject* globalObject, JSC__JSValue arg1)
Expand Down
11 changes: 7 additions & 4 deletions src/bun.js/node/node_fs.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3045,7 +3045,7 @@ pub const Arguments = struct {
}

pub fn fromJS(ctx: JSC.C.JSContextRef, arguments: *ArgumentsSlice, exception: JSC.C.ExceptionRef) ?CopyFile {
const src = PathLike.fromJS(ctx, arguments, exception) orelse {
const src = PathLike.fromJSWithAllocator(ctx, arguments, bun.default_allocator, exception) orelse {
if (exception.* == null) {
JSC.throwInvalidArguments(
"src must be a string or buffer",
Expand All @@ -3059,7 +3059,9 @@ pub const Arguments = struct {

if (exception.* != null) return null;

const dest = PathLike.fromJS(ctx, arguments, exception) orelse {
const dest = PathLike.fromJSWithAllocator(ctx, arguments, bun.default_allocator, exception) orelse {
src.deinit();

if (exception.* == null) {
JSC.throwInvalidArguments(
"dest must be a string or buffer",
Expand Down Expand Up @@ -3107,7 +3109,7 @@ pub const Arguments = struct {
}

pub fn fromJS(ctx: JSC.C.JSContextRef, arguments: *ArgumentsSlice, exception: JSC.C.ExceptionRef) ?Cp {
const src = PathLike.fromJS(ctx, arguments, exception) orelse {
const src = PathLike.fromJSWithAllocator(ctx, arguments, bun.default_allocator, exception) orelse {
if (exception.* == null) {
JSC.throwInvalidArguments(
"src must be a string or buffer",
Expand All @@ -3121,7 +3123,8 @@ pub const Arguments = struct {

if (exception.* != null) return null;

const dest = PathLike.fromJS(ctx, arguments, exception) orelse {
const dest = PathLike.fromJSWithAllocator(ctx, arguments, bun.default_allocator, exception) orelse {
defer src.deinit();
if (exception.* == null) {
JSC.throwInvalidArguments(
"dest must be a string or buffer",
Expand Down
53 changes: 45 additions & 8 deletions src/bun.js/webcore/response.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,7 @@ pub const Fetch = struct {
}

const promise = promise_value.asAnyPromise().?;
_ = promise;
const tracker = this.tracker;
tracker.willDispatch(globalThis);
defer {
Expand All @@ -1007,15 +1008,51 @@ pub const Fetch = struct {
result.ensureStillAlive();

promise_value.ensureStillAlive();
const Holder = struct {
held: JSC.Strong,
promise: JSC.Strong,
globalObject: *JSC.JSGlobalObject,
task: JSC.AnyTask,

pub fn resolve(held: *@This()) void {
var prom = held.promise.swap().asAnyPromise().?;
var globalObject = held.globalObject;
const res = held.held.swap();
held.held.deinit();
held.promise.deinit();
res.ensureStillAlive();

bun.default_allocator.destroy(held);
prom.resolve(globalObject, res);
}

switch (success) {
true => {
promise.resolve(globalThis, result);
},
false => {
promise.reject(globalThis, result);
},
}
pub fn reject(held: *@This()) void {
var prom = held.promise.swap().asAnyPromise().?;
var globalObject = held.globalObject;
const res = held.held.swap();
held.held.deinit();
held.promise.deinit();
res.ensureStillAlive();

bun.default_allocator.destroy(held);
prom.reject(globalObject, res);
}
};

var holder = bun.default_allocator.create(Holder) catch unreachable;
holder.* = .{
.held = JSC.Strong.create(result, globalThis),
.promise = ref.strong,
.globalObject = globalThis,
.task = undefined,
};
ref.strong = .{};
holder.task = switch (success) {
true => JSC.AnyTask.New(Holder, Holder.resolve).init(holder),
false => JSC.AnyTask.New(Holder, Holder.reject).init(holder),
};

globalThis.bunVM().enqueueTask(JSC.Task.init(&holder.task));
}

pub fn checkServerIdentity(this: *FetchTasklet, certificate_info: HTTPClient.CertificateInfo) bool {
Expand Down
4 changes: 2 additions & 2 deletions src/bun.js/webcore/streams.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ pub const ReadableStream = struct {
blob.size = blobby.remain;
blob.store.?.ref();
stream.detachIfPossible(globalThis);
blobby.deinit();

return AnyBlob{ .Blob = blob };
},
Expand All @@ -120,7 +119,6 @@ pub const ReadableStream = struct {
// it should be lazy, file shouldn't have opened yet.
std.debug.assert(!blobby.started);
stream.detachIfPossible(globalThis);
blobby.deinit();
return AnyBlob{ .Blob = blob };
}
},
Expand All @@ -131,6 +129,8 @@ pub const ReadableStream = struct {
if (bytes.has_received_last_chunk) {
var blob: JSC.WebCore.AnyBlob = undefined;
blob.from(bytes.buffer);
bytes.buffer.items = &.{};
bytes.buffer.capacity = 0;
stream.detachIfPossible(globalThis);
return blob;
}
Expand Down
8 changes: 7 additions & 1 deletion test/js/node/fs/fs.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2038,6 +2038,12 @@ it("test syscall errno, issue#4198", () => {

mkdirSync(path);
expect(() => mkdirSync(path)).toThrow("File or folder exists");
expect(() => unlinkSync(path)).toThrow("Operation not permitted");
expect(() => unlinkSync(path)).toThrow(
{
["darwin"]: "Operation not permitted",
["linux"]: "Is a directory",
// TODO: windows
}[process.platform] as const,
);
rmdirSync(path);
});

0 comments on commit 822a00c

Please sign in to comment.