Fix a couple important bugs (#4560)bun-v1.0.0

6 files changed, 105 insertions, 42 deletions

diff --git a/src/bun.js/bindings/ImportMetaObject.cpp b/src/bun.js/bindings/ImportMetaObject.cpp
index c53235824..4160102a5 100644
--- a/src/bun.js/bindings/ImportMetaObject.cpp
+++ b/src/bun.js/bindings/ImportMetaObject.cpp
@@ -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) {
@@ -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)) {
@@ -491,20 +499,25 @@ 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);
@@ -512,21 +525,20 @@ void ImportMetaObject::finishCreation(VM& vm)
             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()));
     });
 }
 
diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp
index 6413e0470..e46982aad 100644
--- a/src/bun.js/bindings/bindings.cpp
+++ b/src/bun.js/bindings/bindings.cpp
@@ -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)
diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig
index 33b3589b3..d9e3ed119 100644
--- a/src/bun.js/node/node_fs.zig
+++ b/src/bun.js/node/node_fs.zig
@@ -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",
@@ -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",
@@ -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",
@@ -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",
diff --git a/src/bun.js/webcore/response.zig b/src/bun.js/webcore/response.zig
index 70a44fd60..6e0f92f9c 100644
--- a/src/bun.js/webcore/response.zig
+++ b/src/bun.js/webcore/response.zig
@@ -985,6 +985,7 @@ pub const Fetch = struct {
             }
 
             const promise = promise_value.asAnyPromise().?;
+            _ = promise;
             const tracker = this.tracker;
             tracker.willDispatch(globalThis);
             defer {
@@ -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 {
diff --git a/src/bun.js/webcore/streams.zig b/src/bun.js/webcore/streams.zig
index f40548eb8..a578313d7 100644
--- a/src/bun.js/webcore/streams.zig
+++ b/src/bun.js/webcore/streams.zig
@@ -109,7 +109,6 @@ pub const ReadableStream = struct {
                 blob.size = blobby.remain;
                 blob.store.?.ref();
                 stream.detachIfPossible(globalThis);
-                blobby.deinit();
 
                 return AnyBlob{ .Blob = blob };
             },
@@ -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 };
                 }
             },
@@ -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;
                 }
diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts
index 4cfe93dc9..8ea9522e1 100644
--- a/test/js/node/fs/fs.test.ts
+++ b/test/js/node/fs/fs.test.ts
@@ -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);
 });