From 148c6e7551f5b8af267dbe37a9275f720b3b93e8 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Tue, 30 May 2023 15:41:27 -0700 Subject: Fix wasi --- src/linker.zig | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/linker.zig b/src/linker.zig index ca57e2f85..a2f1dab71 100644 --- a/src/linker.zig +++ b/src/linker.zig @@ -1016,10 +1016,8 @@ pub const Linker = struct { .napi => { import_record.print_mode = .napi_module; }, - .wasm => { - import_record.print_mode = .import_path; - }, - .file => { + + .wasm, .file => { // if we're building for web/node, always print as import path // if we're building for bun -- cgit v1.2.3 From 0aab11a95d1e81f786a25a03d730987181f4d661 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Tue, 30 May 2023 19:39:12 -0700 Subject: always clone these --- src/bun.js/webcore/blob.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/bun.js/webcore/blob.zig b/src/bun.js/webcore/blob.zig index 591150e12..dd19e1da0 100644 --- a/src/bun.js/webcore/blob.zig +++ b/src/bun.js/webcore/blob.zig @@ -2548,13 +2548,13 @@ pub const Blob = struct { ) callconv(.C) JSValue { if (this.content_type.len > 0) { if (this.content_type_allocated) { - return ZigString.init(this.content_type).toValue(globalThis); + return ZigString.init(this.content_type).toValueGC(globalThis); } return ZigString.init(this.content_type).toValueGC(globalThis); } if (this.store) |store| { - return ZigString.init(store.mime_type.value).toValue(globalThis); + return ZigString.init(store.mime_type.value).toValueGC(globalThis); } return ZigString.Empty.toValue(globalThis); -- cgit v1.2.3 From 557aac6a34bcd06bba98060f67bb7295d6c462d9 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 31 May 2023 00:43:59 -0700 Subject: Support FormData & file uploads in `fetch` body (#3123) * Fixes #2264 * fixup * Don't leak HTTP headers * Include the mime type. It's cleaner --------- Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> --- src/bun.js/webcore/blob.zig | 45 +++++++- src/bun.js/webcore/response.zig | 175 +++++++++++++++++++++-------- src/url.zig | 14 ++- test/js/bun/http/fetch-file-upload.test.ts | 51 +++++++++ test/js/bun/http/serve.test.ts | 52 +++++++++ test/js/web/html/FormData.test.ts | 119 ++++++++++++++++++++ 6 files changed, 406 insertions(+), 50 deletions(-) create mode 100644 test/js/bun/http/fetch-file-upload.test.ts (limited to 'src') diff --git a/src/bun.js/webcore/blob.zig b/src/bun.js/webcore/blob.zig index dd19e1da0..a5d3c968d 100644 --- a/src/bun.js/webcore/blob.zig +++ b/src/bun.js/webcore/blob.zig @@ -85,6 +85,7 @@ pub const Blob = struct { store: ?*Store = null, content_type: string = "", content_type_allocated: bool = false, + content_type_was_set: bool = false, /// JavaScriptCore strings are either latin1 or UTF-16 /// When UTF-16, they're nearly always due to non-ascii characters @@ -111,6 +112,10 @@ pub const Blob = struct { return bun.FormData.AsyncFormData.init(this.allocator orelse bun.default_allocator, encoding) catch unreachable; } + pub fn hasContentTypeFromUser(this: *const Blob) bool { + return this.content_type_was_set or (this.store != null and this.store.?.data == .file); + } + const FormDataContext = struct { allocator: std.mem.Allocator, joiner: StringJoiner, @@ -228,6 +233,7 @@ pub const Blob = struct { var blob = Blob.initWithStore(store, globalThis); blob.content_type = store.mime_type.value; + blob.content_type_was_set = true; return blob; } @@ -268,6 +274,7 @@ pub const Blob = struct { var blob = Blob.initWithStore(store, globalThis); blob.content_type = std.fmt.allocPrint(allocator, "multipart/form-data; boundary=\"{s}\"", .{boundary}) catch unreachable; blob.content_type_allocated = true; + blob.content_type_was_set = true; return blob; } @@ -288,7 +295,7 @@ pub const Blob = struct { export fn Blob__dupe(ptr: *anyopaque) *Blob { var this = bun.cast(*Blob, ptr); var new = bun.default_allocator.create(Blob) catch unreachable; - new.* = this.dupe(); + new.* = this.dupeWithContentType(true); new.allocator = bun.default_allocator; return new; } @@ -2527,6 +2534,7 @@ pub const Blob = struct { blob.content_type = content_type; blob.content_type_allocated = content_type_was_allocated; + blob.content_type_was_set = this.content_type_was_set or content_type_was_allocated; var blob_ = allocator.create(Blob) catch unreachable; blob_.* = blob; @@ -2754,6 +2762,8 @@ pub const Blob = struct { if (!strings.isAllASCII(slice)) { break :inner; } + blob.content_type_was_set = true; + if (globalThis.bunVM().mimeType(slice)) |mime| { blob.content_type = mime.value; break :inner; @@ -2769,6 +2779,7 @@ pub const Blob = struct { if (blob.content_type.len == 0) { blob.content_type = ""; + blob.content_type_was_set = false; } }, } @@ -2870,8 +2881,33 @@ pub const Blob = struct { /// This creates a new view /// and increment the reference count pub fn dupe(this: *const Blob) Blob { + return this.dupeWithContentType(false); + } + + pub fn dupeWithContentType(this: *const Blob, include_content_type: bool) Blob { if (this.store != null) this.store.?.ref(); var duped = this.*; + if (duped.content_type_allocated and duped.allocator != null and !include_content_type) { + + // for now, we just want to avoid a use-after-free here + if (JSC.VirtualMachine.get().mimeType(duped.content_type)) |mime| { + duped.content_type = mime.value; + } else { + // TODO: fix this + // this is a bug. + // it means whenever + duped.content_type = ""; + } + + duped.content_type_allocated = false; + duped.content_type_was_set = false; + if (this.content_type_was_set) { + duped.content_type_was_set = duped.content_type.len > 0; + } + } else if (duped.content_type_allocated and duped.allocator != null and include_content_type) { + duped.content_type = bun.default_allocator.dupe(u8, this.content_type) catch @panic("Out of memory"); + } + duped.allocator = null; return duped; } @@ -3477,6 +3513,13 @@ pub const AnyBlob = union(enum) { // InlineBlob: InlineBlob, InternalBlob: InternalBlob, + pub fn hasContentTypeFromUser(this: AnyBlob) bool { + return switch (this) { + .Blob => this.Blob.hasContentTypeFromUser(), + .InternalBlob => false, + }; + } + pub fn toJSON(this: *AnyBlob, global: *JSGlobalObject, comptime lifetime: JSC.WebCore.Lifetime) JSValue { switch (this.*) { .Blob => return this.Blob.toJSON(global, lifetime), diff --git a/src/bun.js/webcore/response.zig b/src/bun.js/webcore/response.zig index 8d1bfb961..1ba0c82cc 100644 --- a/src/bun.js/webcore/response.zig +++ b/src/bun.js/webcore/response.zig @@ -961,6 +961,14 @@ pub const Fetch = struct { var url = ZigURL{}; var first_arg = args.nextEat().?; + + // We must always get the Body before the Headers That way, we can set + // the Content-Type header from the Blob if no Content-Type header is + // set in the Headers + // + // which is important for FormData. + // https://github.com/oven-sh/bun/issues/2264 + // var body: AnyBlob = AnyBlob{ .Blob = .{}, }; @@ -988,46 +996,45 @@ pub const Fetch = struct { method = request.method; } + if (options.fastGet(ctx.ptr(), .body)) |body__| { + if (Body.Value.fromJS(ctx.ptr(), body__)) |body_const| { + var body_value = body_const; + // TODO: buffer ReadableStream? + // we have to explicitly check for InternalBlob + body = body_value.useAsAnyBlob(); + } else { + // clean hostname if any + if (hostname) |host| { + bun.default_allocator.free(host); + } + // an error was thrown + return JSC.JSValue.jsUndefined(); + } + } else { + body = request.body.value.useAsAnyBlob(); + } + if (options.fastGet(ctx.ptr(), .headers)) |headers_| { if (headers_.as(FetchHeaders)) |headers__| { if (headers__.fastGet(JSC.FetchHeaders.HTTPHeaderName.Host)) |_hostname| { hostname = _hostname.toOwnedSliceZ(bun.default_allocator) catch unreachable; } - headers = Headers.from(headers__, bun.default_allocator) catch unreachable; + headers = Headers.from(headers__, bun.default_allocator, .{ .body = &body }) catch unreachable; // TODO: make this one pass } else if (FetchHeaders.createFromJS(ctx.ptr(), headers_)) |headers__| { if (headers__.fastGet(JSC.FetchHeaders.HTTPHeaderName.Host)) |_hostname| { hostname = _hostname.toOwnedSliceZ(bun.default_allocator) catch unreachable; } - headers = Headers.from(headers__, bun.default_allocator) catch unreachable; + headers = Headers.from(headers__, bun.default_allocator, .{ .body = &body }) catch unreachable; headers__.deref(); } else if (request.headers) |head| { if (head.fastGet(JSC.FetchHeaders.HTTPHeaderName.Host)) |_hostname| { hostname = _hostname.toOwnedSliceZ(bun.default_allocator) catch unreachable; } - headers = Headers.from(head, bun.default_allocator) catch unreachable; + headers = Headers.from(head, bun.default_allocator, .{ .body = &body }) catch unreachable; } } else if (request.headers) |head| { - headers = Headers.from(head, bun.default_allocator) catch unreachable; - } - - if (options.fastGet(ctx.ptr(), .body)) |body__| { - if (Body.Value.fromJS(ctx.ptr(), body__)) |body_const| { - var body_value = body_const; - // TODO: buffer ReadableStream? - // we have to explicitly check for InternalBlob - - body = body_value.useAsAnyBlob(); - } else { - // clean hostname if any - if (hostname) |host| { - bun.default_allocator.free(host); - } - // an error was thrown - return JSC.JSValue.jsUndefined(); - } - } else { - body = request.body.value.useAsAnyBlob(); + headers = Headers.from(head, bun.default_allocator, .{ .body = &body }) catch unreachable; } if (options.get(ctx, "timeout")) |timeout_value| { @@ -1100,13 +1107,13 @@ pub const Fetch = struct { } } else { method = request.method; + body = request.body.value.useAsAnyBlob(); if (request.headers) |head| { if (head.fastGet(JSC.FetchHeaders.HTTPHeaderName.Host)) |_hostname| { hostname = _hostname.toOwnedSliceZ(bun.default_allocator) catch unreachable; } - headers = Headers.from(head, bun.default_allocator) catch unreachable; + headers = Headers.from(head, bun.default_allocator, .{ .body = &body }) catch unreachable; } - body = request.body.value.useAsAnyBlob(); // no proxy only url url = ZigURL.parse(getAllocator(ctx).dupe(u8, request.url) catch unreachable); url_proxy_buffer = url.href; @@ -1124,19 +1131,35 @@ pub const Fetch = struct { method = Method.which(slice_.slice()) orelse .GET; } + if (options.fastGet(ctx.ptr(), .body)) |body__| { + if (Body.Value.fromJS(ctx.ptr(), body__)) |body_const| { + var body_value = body_const; + // TODO: buffer ReadableStream? + // we have to explicitly check for InternalBlob + body = body_value.useAsAnyBlob(); + } else { + // clean hostname if any + if (hostname) |host| { + bun.default_allocator.free(host); + } + // an error was thrown + return JSC.JSValue.jsUndefined(); + } + } + if (options.fastGet(ctx.ptr(), .headers)) |headers_| { if (headers_.as(FetchHeaders)) |headers__| { if (headers__.fastGet(JSC.FetchHeaders.HTTPHeaderName.Host)) |_hostname| { hostname = _hostname.toOwnedSliceZ(bun.default_allocator) catch unreachable; } - headers = Headers.from(headers__, bun.default_allocator) catch unreachable; + headers = Headers.from(headers__, bun.default_allocator, .{ .body = &body }) catch unreachable; // TODO: make this one pass } else if (FetchHeaders.createFromJS(ctx.ptr(), headers_)) |headers__| { defer headers__.deref(); if (headers__.fastGet(JSC.FetchHeaders.HTTPHeaderName.Host)) |_hostname| { hostname = _hostname.toOwnedSliceZ(bun.default_allocator) catch unreachable; } - headers = Headers.from(headers__, bun.default_allocator) catch unreachable; + headers = Headers.from(headers__, bun.default_allocator, .{ .body = &body }) catch unreachable; } else { // Converting the headers failed; return null and // let the set exception get thrown @@ -1144,22 +1167,6 @@ pub const Fetch = struct { } } - if (options.fastGet(ctx.ptr(), .body)) |body__| { - if (Body.Value.fromJS(ctx.ptr(), body__)) |body_const| { - var body_value = body_const; - // TODO: buffer ReadableStream? - // we have to explicitly check for InternalBlob - body = body_value.useAsAnyBlob(); - } else { - // clean hostname if any - if (hostname) |host| { - bun.default_allocator.free(host); - } - // an error was thrown - return JSC.JSValue.jsUndefined(); - } - } - if (options.get(ctx, "timeout")) |timeout_value| { if (timeout_value.isBoolean()) { disable_timeout = !timeout_value.asBoolean(); @@ -1324,6 +1331,47 @@ pub const Fetch = struct { return JSPromise.rejectedPromiseValue(globalThis, err); } + if (headers == null and body.size() > 0 and body.hasContentTypeFromUser()) { + headers = Headers.from( + null, + bun.default_allocator, + .{ .body = &body }, + ) catch unreachable; + } + + if (body.needsToReadFile()) { + // TODO: make this async + lazy + const res = JSC.Node.NodeFS.readFile( + globalThis.bunVM().nodeFS(), + .{ + .encoding = .buffer, + .path = body.Blob.store.?.data.file.pathlike, + .offset = body.Blob.offset, + .max_size = body.Blob.size, + }, + .sync, + ); + + switch (res) { + .err => |err| { + bun.default_allocator.free(url_proxy_buffer); + + const rejected_value = JSPromise.rejectedPromiseValue(globalThis, err.toJSC(globalThis)); + body.detach(); + if (headers) |*headers_| { + headers_.buf.deinit(bun.default_allocator); + headers_.entries.deinit(bun.default_allocator); + } + + return rejected_value; + }, + .result => |result| { + body.detach(); + body.from(std.ArrayList(u8).fromOwnedSlice(bun.default_allocator, @constCast(result.slice()))); + }, + } + } + // Only create this after we have validated all the input. // or else we will leak it var promise = JSPromise.Strong.init(globalThis); @@ -1376,15 +1424,31 @@ pub const Headers = struct { ""; } - pub fn from(headers_ref: *FetchHeaders, allocator: std.mem.Allocator) !Headers { + pub const Options = struct { + body: ?*const AnyBlob = null, + }; + + pub fn from(fetch_headers_ref: ?*FetchHeaders, allocator: std.mem.Allocator, options: Options) !Headers { var header_count: u32 = 0; var buf_len: u32 = 0; - headers_ref.count(&header_count, &buf_len); + if (fetch_headers_ref) |headers_ref| + headers_ref.count(&header_count, &buf_len); var headers = Headers{ .entries = .{}, .buf = .{}, .allocator = allocator, }; + const buf_len_before_content_type = buf_len; + const needs_content_type = brk: { + if (options.body) |body| { + if (body.hasContentTypeFromUser() and (fetch_headers_ref == null or !fetch_headers_ref.?.fastHas(.ContentType))) { + header_count += 1; + buf_len += @truncate(u32, body.contentType().len + "Content-Type".len); + break :brk true; + } + } + break :brk false; + }; headers.entries.ensureTotalCapacity(allocator, header_count) catch unreachable; headers.entries.len = header_count; headers.buf.ensureTotalCapacityPrecise(allocator, buf_len) catch unreachable; @@ -1392,7 +1456,24 @@ pub const Headers = struct { var sliced = headers.entries.slice(); var names = sliced.items(.name); var values = sliced.items(.value); - headers_ref.copyTo(names.ptr, values.ptr, headers.buf.items.ptr); + if (fetch_headers_ref) |headers_ref| + headers_ref.copyTo(names.ptr, values.ptr, headers.buf.items.ptr); + + // TODO: maybe we should send Content-Type header first instead of last? + if (needs_content_type) { + bun.copy(u8, headers.buf.items[buf_len_before_content_type..], "Content-Type"); + names[header_count - 1] = .{ + .offset = buf_len_before_content_type, + .length = "Content-Type".len, + }; + + bun.copy(u8, headers.buf.items[buf_len_before_content_type + "Content-Type".len ..], options.body.?.contentType()); + values[header_count - 1] = .{ + .offset = buf_len_before_content_type + @as(u32, "Content-Type".len), + .length = @truncate(u32, options.body.?.contentType().len), + }; + } + return headers; } }; @@ -1567,7 +1648,7 @@ pub const FetchEvent = struct { var content_length: ?usize = null; if (response.body.init.headers) |headers_ref| { - var headers = Headers.from(headers_ref, request_context.allocator) catch unreachable; + var headers = Headers.from(headers_ref, request_context.allocator, .{}) catch unreachable; var i: usize = 0; while (i < headers.entries.len) : (i += 1) { diff --git a/src/url.zig b/src/url.zig index 1e1b284b5..d0fcdab38 100644 --- a/src/url.zig +++ b/src/url.zig @@ -997,6 +997,9 @@ pub const FormData = struct { defer blob.detach(); var filename = bun.JSC.ZigString.initUTF8(filename_str); const content_type: []const u8 = brk: { + if (!field.content_type.isEmpty()) { + break :brk field.content_type.slice(buf); + } if (filename_str.len > 0) { if (bun.HTTP.MimeType.byExtensionNoDefault(std.fs.path.extension(filename_str))) |mime| { break :brk mime.value; @@ -1011,8 +1014,15 @@ pub const FormData = struct { }; if (content_type.len > 0) { - blob.content_type = content_type; - blob.content_type_allocated = false; + if (!field.content_type.isEmpty()) { + blob.content_type_allocated = true; + blob.content_type = bun.default_allocator.dupe(u8, content_type) catch @panic("failed to allocate memory for blob content type"); + blob.content_type_was_set = true; + } else { + blob.content_type = content_type; + blob.content_type_was_set = false; + blob.content_type_allocated = false; + } } wrap.form.appendBlob(wrap.globalThis, &key, &blob, &filename); diff --git a/test/js/bun/http/fetch-file-upload.test.ts b/test/js/bun/http/fetch-file-upload.test.ts new file mode 100644 index 000000000..33f0d0581 --- /dev/null +++ b/test/js/bun/http/fetch-file-upload.test.ts @@ -0,0 +1,51 @@ +import { expect, test, describe } from "bun:test"; +import { withoutAggressiveGC } from "harness"; + +test("uploads roundtrip", async () => { + const body = Bun.file(import.meta.dir + "/fetch.js.txt"); + const bodyText = await body.text(); + + const server = Bun.serve({ + port: 0, + development: false, + async fetch(req) { + const text = await req.text(); + expect(text).toBe(bodyText); + + return new Response(Bun.file(import.meta.dir + "/fetch.js.txt")); + }, + }); + + // @ts-ignore + const reqBody = new Request(`http://${server.hostname}:${server.port}`, { + body, + method: "POST", + }); + const res = await fetch(reqBody); + expect(res.status).toBe(200); + + // but it does for Response + expect(res.headers.get("Content-Type")).toBe("text/plain;charset=utf-8"); + const resText = await res.text(); + expect(resText).toBe(bodyText); + + server.stop(true); +}); + +test("missing file throws the expected error", async () => { + Bun.gc(true); + // Run this 1000 times to check for GC bugs + withoutAggressiveGC(() => { + const body = Bun.file(import.meta.dir + "/fetch123123231123.js.txt"); + for (let i = 0; i < 1000; i++) { + const resp = fetch(`http://example.com`, { + body, + method: "POST", + proxy: "http://localhost:3000", + }); + expect(Bun.peek.status(resp)).toBe("rejected"); + expect(async () => await resp).toThrow("No such file or directory"); + } + }); + Bun.gc(true); +}); diff --git a/test/js/bun/http/serve.test.ts b/test/js/bun/http/serve.test.ts index 73de6a381..a152e7e09 100644 --- a/test/js/bun/http/serve.test.ts +++ b/test/js/bun/http/serve.test.ts @@ -1006,3 +1006,55 @@ it("request body and signal life cycle", async () => { server.stop(true); } }); + +it("propagates content-type from a Bun.file()'s file path in fetch()", async () => { + const body = Bun.file(import.meta.dir + "/fetch.js.txt"); + const bodyText = await body.text(); + + const server = Bun.serve({ + port: 0, + development: false, + async fetch(req) { + expect(req.headers.get("Content-Type")).toBe("text/plain;charset=utf-8"); + const text = await req.text(); + expect(text).toBe(bodyText); + + return new Response(Bun.file(import.meta.dir + "/fetch.js.txt")); + }, + }); + + // @ts-ignore + const reqBody = new Request(`http://${server.hostname}:${server.port}`, { + body, + method: "POST", + }); + const res = await fetch(reqBody); + expect(res.status).toBe(200); + + // but it does for Response + expect(res.headers.get("Content-Type")).toBe("text/plain;charset=utf-8"); + + server.stop(true); +}); + +it("does propagate type for Blob", async () => { + const server = Bun.serve({ + port: 0, + development: false, + async fetch(req) { + expect(req.headers.get("Content-Type")).toBeNull(); + return new Response(new Blob(["hey"], { type: "text/plain;charset=utf-8" })); + }, + }); + + const body = new Blob(["hey"], { type: "text/plain;charset=utf-8" }); + // @ts-ignore + const res = await fetch(`http://${server.hostname}:${server.port}`, { + body, + method: "POST", + }); + expect(res.status).toBe(200); + expect(res.headers.get("Content-Type")).toBe("text/plain;charset=utf-8"); + + server.stop(true); +}); diff --git a/test/js/web/html/FormData.test.ts b/test/js/web/html/FormData.test.ts index abb298c1a..cbaf5aaa7 100644 --- a/test/js/web/html/FormData.test.ts +++ b/test/js/web/html/FormData.test.ts @@ -302,6 +302,125 @@ describe("FormData", () => { server.stop(true); }); + for (let useRequestConstructor of [true, false]) { + describe(useRequestConstructor ? "Request constructor" : "fetch()", () => { + function send(args: Parameters) { + if (useRequestConstructor) { + return fetch(new Request(...args)); + } else { + return fetch(...args); + } + } + for (let headers of [{}, undefined, { headers: { X: "Y" } }]) { + describe("headers: " + Bun.inspect(headers).replaceAll(/([\n ])/gim, ""), () => { + it("send on HTTP server with FormData & Blob (roundtrip)", async () => { + let contentType = ""; + const server = Bun.serve({ + port: 0, + development: false, + async fetch(req) { + const formData = await req.formData(); + contentType = req.headers.get("Content-Type")!; + return new Response(formData); + }, + }); + + const form = new FormData(); + form.append("foo", new Blob(["baz"], { type: "text/plain" }), "bar"); + form.append("bar", "baz"); + + // @ts-ignore + const reqBody = [ + `http://${server.hostname}:${server.port}`, + { + body: form, + + headers, + method: "POST", + }, + ]; + const res = await send(reqBody); + const body = await res.formData(); + expect(await (body.get("foo") as Blob).text()).toBe("baz"); + expect(body.get("bar")).toBe("baz"); + server.stop(true); + }); + + it("send on HTTP server with FormData & Bun.file (roundtrip)", async () => { + let contentType = ""; + const server = Bun.serve({ + port: 0, + development: false, + async fetch(req) { + const formData = await req.formData(); + contentType = req.headers.get("Content-Type")!; + return new Response(formData); + }, + }); + + const form = new FormData(); + const file = Bun.file(import.meta.dir + "/form-data-fixture.txt"); + const text = await file.text(); + form.append("foo", file); + form.append("bar", "baz"); + + // @ts-ignore + const reqBody = [ + `http://${server.hostname}:${server.port}`, + { + body: form, + + headers, + method: "POST", + }, + ]; + const res = await send(reqBody); + const body = await res.formData(); + expect(await (body.get("foo") as Blob).text()).toBe(text); + expect(contentType).toContain("multipart/form-data"); + expect(body.get("bar")).toBe("baz"); + expect(contentType).toContain("multipart/form-data"); + + server.stop(true); + }); + + it("send on HTTP server with FormData (roundtrip)", async () => { + let contentType = ""; + const server = Bun.serve({ + port: 0, + development: false, + async fetch(req) { + const formData = await req.formData(); + contentType = req.headers.get("Content-Type")!; + return new Response(formData); + }, + }); + + const form = new FormData(); + form.append("foo", "boop"); + form.append("bar", "baz"); + + // @ts-ignore + const reqBody = [ + `http://${server.hostname}:${server.port}`, + { + body: form, + + headers, + method: "POST", + }, + ]; + const res = await send(reqBody); + const body = await res.formData(); + expect(contentType).toContain("multipart/form-data"); + expect(body.get("foo")).toBe("boop"); + expect(body.get("bar")).toBe("baz"); + server.stop(true); + }); + }); + } + }); + } describe("Bun.file support", () => { describe("roundtrip", () => { const path = import.meta.dir + "/form-data-fixture.txt"; -- cgit v1.2.3 From 58fcb60831f346a7f07a258f67bdf40e55e094d2 Mon Sep 17 00:00:00 2001 From: Ciro Spaciari Date: Wed, 31 May 2023 16:07:52 -0300 Subject: fix(path) fix parse behavior (#3134) --- src/bun.js/node/types.zig | 15 +++++---- src/fs.zig | 43 ++++++++++++++++++-------- test/js/node/path/path.test.js | 70 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index bbe2ea654..063e6bd8e 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -1776,14 +1776,17 @@ pub const Path = struct { defer path_slice.deinit(); var path = path_slice.slice(); var path_name = Fs.PathName.init(path); - var root = JSC.ZigString.init(path_name.dir); - const is_absolute = (isWindows and isZigStringAbsoluteWindows(root)) or (!isWindows and path_name.dir.len > 0 and path_name.dir[0] == '/'); - var dir = JSC.ZigString.init(path_name.dir); + const is_absolute = (isWindows and isZigStringAbsoluteWindows(dir)) or (!isWindows and path.len > 0 and path[0] == '/'); + + // if its not absolute root must be empty + var root = JSC.ZigString.Empty; if (is_absolute) { - root = JSC.ZigString.Empty; - if (path_name.dir.len == 0) - dir = JSC.ZigString.init(if (isWindows) std.fs.path.sep_str_windows else std.fs.path.sep_str_posix); + root = JSC.ZigString.init(if (isWindows) std.fs.path.sep_str_windows else std.fs.path.sep_str_posix); + // if is absolute and dir is empty, then dir = root + if (path_name.dir.len == 0) { + dir = root; + } } var base = JSC.ZigString.init(path_name.base); diff --git a/src/fs.zig b/src/fs.zig index 6e1da47f4..636f6a13e 100644 --- a/src/fs.zig +++ b/src/fs.zig @@ -1196,7 +1196,6 @@ pub const PathName = struct { /// includes the leading . ext: string, filename: string, - pub fn nonUniqueNameStringBase(self: *const PathName) string { // /bar/foo/index.js -> foo if (self.dir.len > 0 and strings.eqlComptime(self.base, "index")) { @@ -1255,12 +1254,14 @@ pub const PathName = struct { pub fn init(_path: string) PathName { var path = _path; var base = path; - var ext = path; + // ext must be empty if not detected + var ext: string = ""; var dir = path; var is_absolute = true; - var _i = strings.lastIndexOfChar(path, '/'); + var first = true; while (_i) |i| { + // Stop if we found a non-trailing slash if (i + 1 != path.len) { base = path[i + 1 ..]; @@ -1269,32 +1270,48 @@ pub const PathName = struct { break; } + // If the path starts with a slash and it's the only slash, it's absolute + if (i == 0 and first) { + base = path[1..]; + dir = &([_]u8{}); + break; + } + + first = false; // Ignore trailing slashes + path = path[0..i]; _i = strings.lastIndexOfChar(path, '/'); } - // Strip off the extension - var _dot = strings.lastIndexOfChar(base, '.'); - if (_dot) |dot| { - ext = base[dot..]; - base = base[0..dot]; + // clean trailing slashs + if (base.len > 1 and base[base.len - 1] == '/') { + base = base[0 .. base.len - 1]; } - if (is_absolute) { - dir = &([_]u8{}); + // filename is base without extension + var filename = base; + + // if only one character ext = "" even if filename it's "." + if (filename.len > 1) { + // Strip off the extension + var _dot = strings.lastIndexOfChar(filename, '.'); + if (_dot) |dot| { + ext = filename[dot..]; + filename = filename[0..dot]; + } } - if (base.len > 1 and base[base.len - 1] == '/') { - base = base[0 .. base.len - 1]; + if (is_absolute) { + dir = &([_]u8{}); } return PathName{ .dir = dir, .base = base, .ext = ext, - .filename = if (dir.len > 0) _path[dir.len + 1 ..] else _path, + .filename = filename, }; } }; diff --git a/test/js/node/path/path.test.js b/test/js/node/path/path.test.js index d2880f124..caaf12db0 100644 --- a/test/js/node/path/path.test.js +++ b/test/js/node/path/path.test.js @@ -450,3 +450,73 @@ it("path.resolve", () => { }); strictEqual(failures.length, 0, failures.join("\n")); }); + +it("path.parse", () => { + expect(path.parse("/tmp")).toStrictEqual({ root: "/", dir: "/", base: "tmp", ext: "", name: "tmp" }); + + expect(path.parse("/tmp/test.txt")).toStrictEqual({ + root: "/", + dir: "/tmp", + base: "test.txt", + ext: ".txt", + name: "test", + }); + + expect(path.parse("/tmp/test/file.txt")).toStrictEqual({ + root: "/", + dir: "/tmp/test", + base: "file.txt", + ext: ".txt", + name: "file", + }); + + expect(path.parse("/tmp/test/dir")).toStrictEqual({ root: "/", dir: "/tmp/test", base: "dir", ext: "", name: "dir" }); + expect(path.parse("/tmp/test/dir/")).toStrictEqual({ + root: "/", + dir: "/tmp/test", + base: "dir", + ext: "", + name: "dir", + }); + + expect(path.parse(".")).toStrictEqual({ root: "", dir: "", base: ".", ext: "", name: "." }); + expect(path.parse("./")).toStrictEqual({ root: "", dir: "", base: ".", ext: "", name: "." }); + expect(path.parse("/.")).toStrictEqual({ root: "/", dir: "/", base: ".", ext: "", name: "." }); + expect(path.parse("/../")).toStrictEqual({ root: "/", dir: "/", base: "..", ext: ".", name: "." }); + + expect(path.parse("./file.txt")).toStrictEqual({ root: "", dir: ".", base: "file.txt", ext: ".txt", name: "file" }); + expect(path.parse("../file.txt")).toStrictEqual({ root: "", dir: "..", base: "file.txt", ext: ".txt", name: "file" }); + expect(path.parse("../test/file.txt")).toStrictEqual({ + root: "", + dir: "../test", + base: "file.txt", + ext: ".txt", + name: "file", + }); + expect(path.parse("test/file.txt")).toStrictEqual({ + root: "", + dir: "test", + base: "file.txt", + ext: ".txt", + name: "file", + }); + + expect(path.parse("test/dir")).toStrictEqual({ root: "", dir: "test", base: "dir", ext: "", name: "dir" }); + expect(path.parse("test/dir/another_dir")).toStrictEqual({ + root: "", + dir: "test/dir", + base: "another_dir", + ext: "", + name: "another_dir", + }); + + expect(path.parse("./dir")).toStrictEqual({ root: "", dir: ".", base: "dir", ext: "", name: "dir" }); + expect(path.parse("../dir")).toStrictEqual({ root: "", dir: "..", base: "dir", ext: "", name: "dir" }); + expect(path.parse("../dir/another_dir")).toStrictEqual({ + root: "", + dir: "../dir", + base: "another_dir", + ext: "", + name: "another_dir", + }); +}); -- cgit v1.2.3 From 7057cb1982782dcf95ae3f97331fdb9c6d283f76 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Wed, 31 May 2023 15:05:38 -0700 Subject: `--no-macros` flag, disable macros in node_modules --- src/bun.js/api/JSBundler.zig | 7 +++++++ src/bun.js/api/JSTranspiler.zig | 7 ++++++- src/bun.js/javascript.zig | 1 + src/bun_js.zig | 20 ++++++++++++++++---- src/bundler.zig | 1 + src/bundler/bundle_v2.zig | 1 + src/bunfig.zig | 9 +++++++-- src/cli.zig | 9 ++++++++- src/cli/build_command.zig | 10 ++++++++-- src/http.zig | 11 ++++++++++- src/js_parser.zig | 23 +++++++++++++++++++++++ src/options.zig | 1 + src/runtime.zig | 2 ++ 13 files changed, 91 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/bun.js/api/JSBundler.zig b/src/bun.js/api/JSBundler.zig index 741d956bf..8e85f1190 100644 --- a/src/bun.js/api/JSBundler.zig +++ b/src/bun.js/api/JSBundler.zig @@ -61,6 +61,7 @@ pub const JSBundler = struct { code_splitting: bool = false, minify: Minify = .{}, server_components: ServerComponents = ServerComponents{}, + no_macros: bool = false, names: Names = .{}, external: bun.StringSet = bun.StringSet.init(bun.default_allocator), @@ -188,6 +189,12 @@ pub const JSBundler = struct { } } + if (config.getTruthy(globalThis, "macros")) |macros_flag| { + if (!macros_flag.coerce(bool, globalThis)) { + this.no_macros = true; + } + } + if (try config.getOptionalEnum(globalThis, "target", options.Target)) |target| { this.target = target; } diff --git a/src/bun.js/api/JSTranspiler.zig b/src/bun.js/api/JSTranspiler.zig index 8a59f59e7..c58029c5e 100644 --- a/src/bun.js/api/JSTranspiler.zig +++ b/src/bun.js/api/JSTranspiler.zig @@ -75,6 +75,7 @@ const TranspilerOptions = struct { minify_whitespace: bool = false, minify_identifiers: bool = false, minify_syntax: bool = false, + no_macros: bool = false, }; // Mimalloc gets unstable if we try to move this to a different thread @@ -479,6 +480,10 @@ fn transformOptionsFromJSC(globalObject: JSC.C.JSContextRef, temp_allocator: std if (object.getIfPropertyExists(globalThis, "macro")) |macros| { macros: { if (macros.isUndefinedOrNull()) break :macros; + if (macros.isBoolean()) { + transpiler.no_macros = !macros.asBoolean(); + break :macros; + } const kind = macros.jsType(); const is_object = kind.isObject(); if (!(kind.isStringLike() or is_object)) { @@ -775,7 +780,7 @@ pub fn constructor( globalThis.throwError(err, "Error creating transpiler"); return null; }; - + bundler.options.no_macros = transpiler_options.no_macros; bundler.configureLinkerWithAutoJSX(false); bundler.options.env.behavior = .disable; bundler.configureDefines() catch |err| { diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index 4a1fcbcb1..5c158a4fb 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -392,6 +392,7 @@ pub const VirtualMachine = struct { macros: MacroMap, macro_entry_points: std.AutoArrayHashMap(i32, *MacroEntryPoint), macro_mode: bool = false, + no_macros: bool = false, has_any_macro_remappings: bool = false, is_from_devserver: bool = false, diff --git a/src/bun_js.zig b/src/bun_js.zig index 12876cae8..fd124a8ac 100644 --- a/src/bun_js.zig +++ b/src/bun_js.zig @@ -90,8 +90,14 @@ pub const Run = struct { // b.options.minify_syntax = ctx.bundler_options.minify_syntax; - if (ctx.debug.macros) |macros| { - b.options.macro_remap = macros; + switch (ctx.debug.macros) { + .disable => { + b.options.no_macros = true; + }, + .map => |macros| { + b.options.macro_remap = macros; + }, + .unspecified => {}, } b.configureRouter(false) catch { @@ -175,8 +181,14 @@ pub const Run = struct { // b.options.minify_syntax = ctx.bundler_options.minify_syntax; - if (ctx.debug.macros) |macros| { - b.options.macro_remap = macros; + switch (ctx.debug.macros) { + .disable => { + b.options.no_macros = true; + }, + .map => |macros| { + b.options.macro_remap = macros; + }, + .unspecified => {}, } b.configureRouter(false) catch { diff --git a/src/bundler.zig b/src/bundler.zig index f3296134e..ea8222870 100644 --- a/src/bundler.zig +++ b/src/bundler.zig @@ -1388,6 +1388,7 @@ pub const Bundler = struct { opts.features.trim_unused_imports = bundler.options.trim_unused_imports orelse loader.isTypeScript(); opts.features.should_fold_typescript_constant_expressions = loader.isTypeScript() or target.isBun() or bundler.options.minify_syntax; opts.features.dynamic_require = target.isBun(); + opts.features.no_macros = bundler.options.no_macros; opts.transform_only = bundler.options.transform_only; // @bun annotation diff --git a/src/bundler/bundle_v2.zig b/src/bundler/bundle_v2.zig index 967bfaa36..2a97b98ed 100644 --- a/src/bundler/bundle_v2.zig +++ b/src/bundler/bundle_v2.zig @@ -1599,6 +1599,7 @@ pub const BundleV2 = struct { completion.env, ); bundler.options.jsx = config.jsx; + bundler.options.no_macros = config.no_macros; bundler.options.react_server_components = config.server_components.client.items.len > 0 or config.server_components.server.items.len > 0; bundler.options.loaders = try options.loadersFromTransformOptions(allocator, config.loaders, config.target); bundler.options.entry_naming = config.names.entry_point.data; diff --git a/src/bunfig.zig b/src/bunfig.zig index af3842451..9df2978b0 100644 --- a/src/bunfig.zig +++ b/src/bunfig.zig @@ -579,8 +579,13 @@ pub const Bunfig = struct { } if (json.get("macros")) |expr| { - // technical debt - this.ctx.debug.macros = PackageJSON.parseMacrosJSON(allocator, expr, this.log, this.source); + if (expr.data == .e_boolean) { + if (expr.data.e_boolean.value == false) { + this.ctx.debug.macros = .{ .disable = {} }; + } + } else { + this.ctx.debug.macros = .{ .map = PackageJSON.parseMacrosJSON(allocator, expr, this.log, this.source) }; + } Analytics.Features.macros = true; } diff --git a/src/cli.zig b/src/cli.zig index 0ad948ac7..dc1ae0cdc 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -157,6 +157,7 @@ pub const Arguments = struct { clap.parseParam("--minify-syntax Minify syntax and inline data (experimental)") catch unreachable, clap.parseParam("--minify-whitespace Minify whitespace (experimental)") catch unreachable, clap.parseParam("--minify-identifiers Minify identifiers") catch unreachable, + clap.parseParam("--no-macros Disable macros from being executed in the bundler, transpiler and runtime") catch unreachable, clap.parseParam("--target The intended execution environment for the bundle. \"browser\", \"bun\" or \"node\"") catch unreachable, clap.parseParam("... ") catch unreachable, }; @@ -744,6 +745,10 @@ pub const Arguments = struct { ctx.log.level = logger.Log.default_log_level; } + if (args.flag("--no-macros")) { + ctx.debug.macros = .{ .disable = {} }; + } + opts.output_dir = output_dir; if (output_file != null) ctx.debug.output_file = output_file.?; @@ -898,7 +903,7 @@ pub const Command = struct { loaded_bunfig: bool = false, // technical debt - macros: ?MacroMap = null, + macros: MacroOptions = MacroOptions.unspecified, editor: string = "", package_bundle_map: bun.StringArrayHashMapUnmanaged(options.BundlePackage) = bun.StringArrayHashMapUnmanaged(options.BundlePackage){}, @@ -906,6 +911,8 @@ pub const Command = struct { output_file: []const u8 = "", }; + pub const MacroOptions = union(enum) { unspecified: void, disable: void, map: MacroMap }; + pub const HotReload = enum { none, hot, diff --git a/src/cli/build_command.zig b/src/cli/build_command.zig index 52b45c493..14414c7de 100644 --- a/src/cli/build_command.zig +++ b/src/cli/build_command.zig @@ -207,8 +207,14 @@ pub const BuildCommand = struct { this_bundler.options.jsx.development = !this_bundler.options.production; this_bundler.resolver.opts.jsx.development = this_bundler.options.jsx.development; - if (ctx.debug.macros) |macros| { - this_bundler.options.macro_remap = macros; + switch (ctx.debug.macros) { + .disable => { + this_bundler.options.no_macros = true; + }, + .map => |macros| { + this_bundler.options.macro_remap = macros; + }, + .unspecified => {}, } // var env_loader = this_bundler.env; diff --git a/src/http.zig b/src/http.zig index c54f4ea9c..f26a0e985 100644 --- a/src/http.zig +++ b/src/http.zig @@ -1484,6 +1484,7 @@ pub const RequestContext = struct { std.debug.assert(JavaScript.VirtualMachine.isLoaded()); javascript_vm = vm; vm.bundler.options.origin = handler.origin; + vm.bundler.options.no_macros = handler.client_bundler.options.no_macros; const boot = vm.bundler.options.framework.?.server.path; std.debug.assert(boot.len > 0); errdefer vm.deinit(); @@ -3972,7 +3973,15 @@ pub const Server = struct { http_editor_context.name = debug.editor; - server.bundler.options.macro_remap = debug.macros orelse .{}; + switch (debug.macros) { + .disable => { + server.bundler.options.no_macros = true; + }, + .map => |macros| { + server.bundler.options.macro_remap = macros; + }, + .unspecified => {}, + } if (debug.fallback_only or server.bundler.env.map.get("BUN_DISABLE_BUN_JS") != null) { RequestContext.fallback_only = true; diff --git a/src/js_parser.zig b/src/js_parser.zig index 0fc4f794a..0c4b5dcb3 100644 --- a/src/js_parser.zig +++ b/src/js_parser.zig @@ -15361,6 +15361,18 @@ fn NewParser_( if (p.is_control_flow_dead) { return p.newExpr(E.Undefined{}, e_.tag.?.loc); } + + // this ordering incase someone wants ot use a macro in a node_module conditionally + if (p.options.features.no_macros) { + p.log.addError(p.source, tag.loc, "Macros are disabled") catch unreachable; + return p.newExpr(E.Undefined{}, e_.tag.?.loc); + } + + if (p.source.path.isNodeModule()) { + p.log.addError(p.source, expr.loc, "For security reasons, macros cannot be run from node_modules.") catch unreachable; + return p.newExpr(E.Undefined{}, expr.loc); + } + p.macro_call_count += 1; const record = &p.import_records.items[import_record_id]; // We must visit it to convert inline_identifiers and record usage @@ -16510,6 +16522,17 @@ fn NewParser_( if (p.is_control_flow_dead) { return p.newExpr(E.Undefined{}, e_.target.loc); } + + if (p.options.features.no_macros) { + p.log.addError(p.source, expr.loc, "Macros are disabled") catch unreachable; + return p.newExpr(E.Undefined{}, expr.loc); + } + + if (p.source.path.isNodeModule()) { + p.log.addError(p.source, expr.loc, "For security reasons, macros cannot be run from node_modules.") catch unreachable; + return p.newExpr(E.Undefined{}, expr.loc); + } + const name = p.symbols.items[ref.innerIndex()].original_name; const record = &p.import_records.items[import_record_id]; const copied = Expr{ .loc = expr.loc, .data = .{ .e_call = e_ } }; diff --git a/src/options.zig b/src/options.zig index 4133e95f7..d2752471d 100644 --- a/src/options.zig +++ b/src/options.zig @@ -1436,6 +1436,7 @@ pub const BundleOptions = struct { rewrite_jest_for_tests: bool = false, macro_remap: MacroRemap = MacroRemap{}, + no_macros: bool = false, conditions: ESMConditions = undefined, tree_shaking: bool = false, diff --git a/src/runtime.zig b/src/runtime.zig index f09e16378..7312aa4bd 100644 --- a/src/runtime.zig +++ b/src/runtime.zig @@ -283,6 +283,8 @@ pub const Runtime = struct { inject_jest_globals: bool = false, + no_macros: bool = false, + commonjs_named_exports: bool = true, minify_syntax: bool = false, -- cgit v1.2.3 From 52c6609792a7fede177c931e482b878b3e806a93 Mon Sep 17 00:00:00 2001 From: Ciro Spaciari Date: Wed, 31 May 2023 19:13:37 -0300 Subject: invert base/filename internally (#3141) --- src/bun.js/node/types.zig | 6 +++--- src/fs.zig | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index 063e6bd8e..da0459866 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -1788,9 +1788,9 @@ pub const Path = struct { dir = root; } } - - var base = JSC.ZigString.init(path_name.base); - var name_ = JSC.ZigString.init(path_name.filename); + // we use filename as base, and base as name because node.js/internals compatibilty + var base = JSC.ZigString.init(path_name.filename); + var name_ = JSC.ZigString.init(path_name.base); var ext = JSC.ZigString.init(path_name.ext); dir.setOutputEncoding(); root.setOutputEncoding(); diff --git a/src/fs.zig b/src/fs.zig index 636f6a13e..e22b6b0b5 100644 --- a/src/fs.zig +++ b/src/fs.zig @@ -1290,16 +1290,16 @@ pub const PathName = struct { base = base[0 .. base.len - 1]; } - // filename is base without extension + // filename is base with extension var filename = base; - // if only one character ext = "" even if filename it's "." - if (filename.len > 1) { + // if only one character ext = "" even if base it's "." + if (base.len > 1) { // Strip off the extension - var _dot = strings.lastIndexOfChar(filename, '.'); + var _dot = strings.lastIndexOfChar(base, '.'); if (_dot) |dot| { - ext = filename[dot..]; - filename = filename[0..dot]; + ext = base[dot..]; + base = base[0..dot]; } } -- cgit v1.2.3 From 611f1d0e241d264747dda7288c018ab383906ccc Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Wed, 31 May 2023 16:13:24 -0700 Subject: set `optional_chain` instead of `optional_start` (#3142) --- src/js_parser.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/js_parser.zig b/src/js_parser.zig index 0c4b5dcb3..a9cd4379c 100644 --- a/src/js_parser.zig +++ b/src/js_parser.zig @@ -12750,7 +12750,7 @@ fn NewParser_( // Only continue if we have started if ((optional_start orelse .ccontinue) == .start) { - optional_start = .ccontinue; + optional_chain = .ccontinue; } }, .t_no_substitution_template_literal => { -- cgit v1.2.3 From 4c0145437679f879329df69c9a56e395e74e8280 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 31 May 2023 17:20:30 -0700 Subject: Make uploading files with `fetch()`fast (#3125) * Make file uploads fast * Add benchmark * Update README.md * defaults * print * prettier * smaller * fix(path) fix parse behavior (#3134) * Add macro docs (#3139) * Add macro doc * Updates * Tweaks * Update doc * Update macro serialization doc * Update macro doc * `--no-macros` flag, disable macros in node_modules * invert base/filename internally (#3141) * always false * Fix broken test * Add a test sendfile() test with large file --------- Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Co-authored-by: Ciro Spaciari Co-authored-by: Colin McDonnell --- bench/stream-file-upload-client/.gitignore | 1 + bench/stream-file-upload-client/README.md | 35 ++++ bench/stream-file-upload-client/generate-file.js | 8 + bench/stream-file-upload-client/server-node.mjs | 15 ++ bench/stream-file-upload-client/stream-file-bun.js | 9 + .../stream-file-upload-client/stream-file-deno.js | 12 ++ .../stream-file-upload-client/stream-file-node.mjs | 19 ++ src/bun.js/webcore/response.zig | 198 +++++++++++++++++---- src/deps/libuwsockets.cpp | 5 + src/deps/uws.zig | 19 ++ src/feature_flags.zig | 2 + src/http_client_async.zig | 174 +++++++++++++++--- test/js/bun/http/fetch-file-upload.test.ts | 31 ++++ test/regression/issue/02499.test.ts | 7 +- 14 files changed, 475 insertions(+), 60 deletions(-) create mode 100644 bench/stream-file-upload-client/.gitignore create mode 100644 bench/stream-file-upload-client/README.md create mode 100644 bench/stream-file-upload-client/generate-file.js create mode 100644 bench/stream-file-upload-client/server-node.mjs create mode 100644 bench/stream-file-upload-client/stream-file-bun.js create mode 100644 bench/stream-file-upload-client/stream-file-deno.js create mode 100644 bench/stream-file-upload-client/stream-file-node.mjs (limited to 'src') diff --git a/bench/stream-file-upload-client/.gitignore b/bench/stream-file-upload-client/.gitignore new file mode 100644 index 000000000..f0ad0dec6 --- /dev/null +++ b/bench/stream-file-upload-client/.gitignore @@ -0,0 +1 @@ +hello.txt diff --git a/bench/stream-file-upload-client/README.md b/bench/stream-file-upload-client/README.md new file mode 100644 index 000000000..0035cfcf5 --- /dev/null +++ b/bench/stream-file-upload-client/README.md @@ -0,0 +1,35 @@ +# HTTP request file upload benchmark + +This is a simple benchmark of uploading a file to a web server in different runtimes. + +## Usage + +Generate a file to upload (default is `hello.txt`): + +```bash +bun generate-file.js +``` + +Run the server: + +```bash +node server-node.mjs +``` + +Run the benchmark in bun: + +```bash +bun stream-file-bun.js +``` + +Run the benchmark in node: + +```bash +node stream-file-node.mjs +``` + +Run the benchmark in deno: + +```bash +deno run -A stream-file-deno.js +``` diff --git a/bench/stream-file-upload-client/generate-file.js b/bench/stream-file-upload-client/generate-file.js new file mode 100644 index 000000000..b3b2080a1 --- /dev/null +++ b/bench/stream-file-upload-client/generate-file.js @@ -0,0 +1,8 @@ +var hey = + "abcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghiabcdefghi".repeat( + 1024, + ); + +hey += hey.repeat(2); +require("fs").writeFileSync("hello.txt", Buffer.from(hey)); +console.log("Wrote", hey.length, "bytes", "to", "hello.txt"); diff --git a/bench/stream-file-upload-client/server-node.mjs b/bench/stream-file-upload-client/server-node.mjs new file mode 100644 index 000000000..10a7b19ed --- /dev/null +++ b/bench/stream-file-upload-client/server-node.mjs @@ -0,0 +1,15 @@ +import { createServer } from "node:http"; +const server = createServer((req, res) => { + var chunkSize = 0; + req.on("data", chunk => { + chunkSize += chunk.byteLength; + }); + + req.on("end", () => { + console.log("Received", chunkSize, "bytes"); + res.end(`${chunkSize}`); + }); +}); +server.listen(parseInt(process.env.PORT ?? "3000"), (err, port) => { + console.log(`http://localhost:${server.address().port}`); +}); diff --git a/bench/stream-file-upload-client/stream-file-bun.js b/bench/stream-file-upload-client/stream-file-bun.js new file mode 100644 index 000000000..e3499bd29 --- /dev/null +++ b/bench/stream-file-upload-client/stream-file-bun.js @@ -0,0 +1,9 @@ +import { file } from "bun"; +console.time("stream-file-bun"); +const response = await fetch(process.env.URL ?? "http://localhost:3000", { + method: "POST", + body: file(process.env.FILE ?? "hello.txt"), +}); +console.timeEnd("stream-file-bun"); + +console.log("Sent", await response.text(), "bytes"); diff --git a/bench/stream-file-upload-client/stream-file-deno.js b/bench/stream-file-upload-client/stream-file-deno.js new file mode 100644 index 000000000..a87d56252 --- /dev/null +++ b/bench/stream-file-upload-client/stream-file-deno.js @@ -0,0 +1,12 @@ +const file = await Deno.open(Deno.env.get("FILE") ?? "hello.txt", { + read: true, +}); + +console.time("stream-file-deno"); +const response = await fetch(Deno.env.get("URL") ?? "http://localhost:3000", { + method: "POST", + body: file.readable, +}); +console.timeEnd("stream-file-deno"); + +console.log("Sent", await response.text(), "bytes"); diff --git a/bench/stream-file-upload-client/stream-file-node.mjs b/bench/stream-file-upload-client/stream-file-node.mjs new file mode 100644 index 000000000..9a0957285 --- /dev/null +++ b/bench/stream-file-upload-client/stream-file-node.mjs @@ -0,0 +1,19 @@ +import { createReadStream } from "node:fs"; +import http from "node:http"; + +console.time("stream-file-node"); +createReadStream(process.env.FILE ?? "hello.txt") + .pipe( + http + .request(process.env.URL ?? "http://localhost:3000", { + method: "POST", + }) + .on("response", response => { + response.on("data", data => { + console.log("Sent", parseInt(data.toString(), 10), "bytes"); + }); + }), + ) + .on("close", () => { + console.timeEnd("stream-file-node"); + }); diff --git a/src/bun.js/webcore/response.zig b/src/bun.js/webcore/response.zig index 1ba0c82cc..ad3857685 100644 --- a/src/bun.js/webcore/response.zig +++ b/src/bun.js/webcore/response.zig @@ -629,7 +629,7 @@ pub const Fetch = struct { result: HTTPClient.HTTPClientResult = .{}, javascript_vm: *VirtualMachine = undefined, global_this: *JSGlobalObject = undefined, - request_body: AnyBlob = undefined, + request_body: HTTPRequestBody = undefined, response_buffer: MutableString = undefined, request_headers: Headers = Headers{ .allocator = undefined }, promise: JSC.JSPromise.Strong, @@ -647,6 +647,38 @@ pub const Fetch = struct { abort_reason: JSValue = JSValue.zero, // Custom Hostname hostname: ?[]u8 = null, + + pub const HTTPRequestBody = union(enum) { + AnyBlob: AnyBlob, + Sendfile: HTTPClient.Sendfile, + + pub fn store(this: *HTTPRequestBody) ?*JSC.WebCore.Blob.Store { + return switch (this.*) { + .AnyBlob => this.AnyBlob.store(), + else => null, + }; + } + + pub fn slice(this: *const HTTPRequestBody) []const u8 { + return switch (this.*) { + .AnyBlob => this.AnyBlob.slice(), + else => "", + }; + } + + pub fn detach(this: *HTTPRequestBody) void { + switch (this.*) { + .AnyBlob => this.AnyBlob.detach(), + .Sendfile => { + if (@max(this.Sendfile.offset, this.Sendfile.remain) > 0) + _ = JSC.Node.Syscall.close(this.Sendfile.fd); + this.Sendfile.offset = 0; + this.Sendfile.remain = 0; + }, + } + } + }; + pub fn init(_: std.mem.Allocator) anyerror!FetchTasklet { return FetchTasklet{}; } @@ -850,12 +882,26 @@ pub const Fetch = struct { proxy = jsc_vm.bundler.env.getHttpProxy(fetch_options.url); } - fetch_tasklet.http.?.* = HTTPClient.AsyncHTTP.init(allocator, fetch_options.method, fetch_options.url, fetch_options.headers.entries, fetch_options.headers.buf.items, &fetch_tasklet.response_buffer, fetch_tasklet.request_body.slice(), fetch_options.timeout, HTTPClient.HTTPClientResult.Callback.New( - *FetchTasklet, - FetchTasklet.callback, - ).init( - fetch_tasklet, - ), proxy, if (fetch_tasklet.signal != null) &fetch_tasklet.aborted else null, fetch_options.hostname, fetch_options.redirect_type); + fetch_tasklet.http.?.* = HTTPClient.AsyncHTTP.init( + allocator, + fetch_options.method, + fetch_options.url, + fetch_options.headers.entries, + fetch_options.headers.buf.items, + &fetch_tasklet.response_buffer, + fetch_tasklet.request_body.slice(), + fetch_options.timeout, + HTTPClient.HTTPClientResult.Callback.New( + *FetchTasklet, + FetchTasklet.callback, + ).init( + fetch_tasklet, + ), + proxy, + if (fetch_tasklet.signal != null) &fetch_tasklet.aborted else null, + fetch_options.hostname, + fetch_options.redirect_type, + ); if (fetch_options.redirect_type != FetchRedirect.follow) { fetch_tasklet.http.?.client.remaining_redirect_count = 0; @@ -865,6 +911,12 @@ pub const Fetch = struct { fetch_tasklet.http.?.client.verbose = fetch_options.verbose; fetch_tasklet.http.?.client.disable_keepalive = fetch_options.disable_keepalive; + if (fetch_tasklet.request_body == .Sendfile) { + std.debug.assert(fetch_options.url.isHTTP()); + std.debug.assert(fetch_options.proxy == null); + fetch_tasklet.http.?.request_body = .{ .sendfile = fetch_tasklet.request_body.Sendfile }; + } + if (fetch_tasklet.signal) |signal| { fetch_tasklet.signal = signal.listen(FetchTasklet, fetch_tasklet, FetchTasklet.abortListener); } @@ -886,7 +938,7 @@ pub const Fetch = struct { const FetchOptions = struct { method: Method, headers: Headers, - body: AnyBlob, + body: HTTPRequestBody, timeout: usize, disable_timeout: bool, disable_keepalive: bool, @@ -1339,36 +1391,114 @@ pub const Fetch = struct { ) catch unreachable; } + var http_body = FetchTasklet.HTTPRequestBody{ + .AnyBlob = body, + }; + if (body.needsToReadFile()) { - // TODO: make this async + lazy - const res = JSC.Node.NodeFS.readFile( - globalThis.bunVM().nodeFS(), - .{ - .encoding = .buffer, - .path = body.Blob.store.?.data.file.pathlike, - .offset = body.Blob.offset, - .max_size = body.Blob.size, - }, - .sync, - ); + prepare_body: { + const opened_fd_res: JSC.Node.Maybe(bun.FileDescriptor) = switch (body.Blob.store.?.data.file.pathlike) { + .fd => |fd| JSC.Node.Maybe(bun.FileDescriptor).errnoSysFd(JSC.Node.Syscall.system.dup(fd), .open, fd) orelse .{ .result = fd }, + .path => |path| JSC.Node.Syscall.open(path.sliceZ(&globalThis.bunVM().nodeFS().sync_error_buf), std.os.O.RDONLY | std.os.O.NOCTTY, 0), + }; + + const opened_fd = switch (opened_fd_res) { + .err => |err| { + bun.default_allocator.free(url_proxy_buffer); + + const rejected_value = JSPromise.rejectedPromiseValue(globalThis, err.toJSC(globalThis)); + body.detach(); + if (headers) |*headers_| { + headers_.buf.deinit(bun.default_allocator); + headers_.entries.deinit(bun.default_allocator); + } - switch (res) { - .err => |err| { - bun.default_allocator.free(url_proxy_buffer); + return rejected_value; + }, + .result => |fd| fd, + }; - const rejected_value = JSPromise.rejectedPromiseValue(globalThis, err.toJSC(globalThis)); - body.detach(); - if (headers) |*headers_| { - headers_.buf.deinit(bun.default_allocator); - headers_.entries.deinit(bun.default_allocator); + if (proxy == null and bun.HTTP.Sendfile.isEligible(url)) { + use_sendfile: { + const stat: std.os.Stat = switch (JSC.Node.Syscall.fstat(opened_fd)) { + .result => |result| result, + // bail out for any reason + .err => break :use_sendfile, + }; + + if (Environment.isMac) { + // macOS only supports regular files for sendfile() + if (!std.os.S.ISREG(stat.mode)) { + break :use_sendfile; + } + } + + // if it's < 32 KB, it's not worth it + if (stat.size < 32 * 1024) { + break :use_sendfile; + } + + const original_size = body.Blob.size; + const stat_size = @intCast(Blob.SizeType, stat.size); + const blob_size = if (std.os.S.ISREG(stat.mode)) + stat_size + else + @min(original_size, stat_size); + + http_body = .{ + .Sendfile = .{ + .fd = opened_fd, + .remain = body.Blob.offset + original_size, + .offset = body.Blob.offset, + .content_size = blob_size, + }, + }; + + if (std.os.S.ISREG(stat.mode)) { + http_body.Sendfile.offset = @min(http_body.Sendfile.offset, stat_size); + http_body.Sendfile.remain = @min(@max(http_body.Sendfile.remain, http_body.Sendfile.offset), stat_size) -| http_body.Sendfile.offset; + } + body.detach(); + + break :prepare_body; } + } - return rejected_value; - }, - .result => |result| { - body.detach(); - body.from(std.ArrayList(u8).fromOwnedSlice(bun.default_allocator, @constCast(result.slice()))); - }, + // TODO: make this async + lazy + const res = JSC.Node.NodeFS.readFile( + globalThis.bunVM().nodeFS(), + .{ + .encoding = .buffer, + .path = .{ .fd = opened_fd }, + .offset = body.Blob.offset, + .max_size = body.Blob.size, + }, + .sync, + ); + + if (body.Blob.store.?.data.file.pathlike == .path) { + _ = JSC.Node.Syscall.close(opened_fd); + } + + switch (res) { + .err => |err| { + bun.default_allocator.free(url_proxy_buffer); + + const rejected_value = JSPromise.rejectedPromiseValue(globalThis, err.toJSC(globalThis)); + body.detach(); + if (headers) |*headers_| { + headers_.buf.deinit(bun.default_allocator); + headers_.entries.deinit(bun.default_allocator); + } + + return rejected_value; + }, + .result => |result| { + body.detach(); + body.from(std.ArrayList(u8).fromOwnedSlice(bun.default_allocator, @constCast(result.slice()))); + http_body = .{ .AnyBlob = body }; + }, + } } } @@ -1388,7 +1518,7 @@ pub const Fetch = struct { .headers = headers orelse Headers{ .allocator = bun.default_allocator, }, - .body = body, + .body = http_body, .timeout = std.time.ns_per_hour, .disable_keepalive = disable_keepalive, .disable_timeout = disable_timeout, diff --git a/src/deps/libuwsockets.cpp b/src/deps/libuwsockets.cpp index ae6443cba..aa4889892 100644 --- a/src/deps/libuwsockets.cpp +++ b/src/deps/libuwsockets.cpp @@ -1572,4 +1572,9 @@ extern "C" return uwsRes->getNativeHandle(); } } + + void us_socket_sendfile_needs_more(us_socket_t *s) { + s->context->loop->data.last_write_failed = 1; + us_poll_change(&s->p, s->context->loop, LIBUS_SOCKET_READABLE | LIBUS_SOCKET_WRITABLE); + } } diff --git a/src/deps/uws.zig b/src/deps/uws.zig index c9f350a37..538756b71 100644 --- a/src/deps/uws.zig +++ b/src/deps/uws.zig @@ -43,6 +43,23 @@ pub fn NewSocketHandler(comptime ssl: bool) type { pub fn getNativeHandle(this: ThisSocket) *NativeSocketHandleType(ssl) { return @ptrCast(*NativeSocketHandleType(ssl), us_socket_get_native_handle(comptime ssl_int, this.socket).?); } + + pub fn fd(this: ThisSocket) i32 { + if (comptime ssl) { + @compileError("SSL sockets do not have a file descriptor accessible this way"); + } + + return @intCast(i32, @ptrToInt(us_socket_get_native_handle(0, this.socket))); + } + + pub fn markNeedsMoreForSendfile(this: ThisSocket) void { + if (comptime ssl) { + @compileError("SSL sockets do not support sendfile yet"); + } + + us_socket_sendfile_needs_more(this.socket); + } + pub fn ext(this: ThisSocket, comptime ContextType: type) ?*ContextType { const alignment = if (ContextType == *anyopaque) @sizeOf(usize) @@ -1882,3 +1899,5 @@ pub const State = enum(i32) { return @enumToInt(this) & @enumToInt(State.HTTP_CONNECTION_CLOSE) != 0; } }; + +extern fn us_socket_sendfile_needs_more(socket: *Socket) void; diff --git a/src/feature_flags.zig b/src/feature_flags.zig index cdfeacb10..0a0c920a4 100644 --- a/src/feature_flags.zig +++ b/src/feature_flags.zig @@ -170,3 +170,5 @@ pub const source_map_debug_id = true; pub const alignment_tweak = false; pub const export_star_redirect = false; + +pub const streaming_file_uploads_for_http_client = true; diff --git a/src/http_client_async.zig b/src/http_client_async.zig index 4e0926baa..fe5f34f48 100644 --- a/src/http_client_async.zig +++ b/src/http_client_async.zig @@ -71,6 +71,89 @@ pub const FetchRedirect = enum(u8) { }); }; +pub const HTTPRequestBody = union(enum) { + bytes: []const u8, + sendfile: Sendfile, + + pub fn len(this: *const HTTPRequestBody) usize { + return switch (this.*) { + .bytes => this.bytes.len, + .sendfile => this.sendfile.content_size, + }; + } +}; + +pub const Sendfile = struct { + fd: bun.FileDescriptor, + remain: usize = 0, + offset: usize = 0, + content_size: usize = 0, + + pub fn isEligible(url: bun.URL) bool { + return url.isHTTP() and url.href.len > 0 and FeatureFlags.streaming_file_uploads_for_http_client; + } + + pub fn write( + this: *Sendfile, + socket: NewHTTPContext(false).HTTPSocket, + ) Status { + const adjusted_count_temporary = @min(@as(u64, this.remain), @as(u63, std.math.maxInt(u63))); + // TODO we should not need this int cast; improve the return type of `@min` + const adjusted_count = @intCast(u63, adjusted_count_temporary); + + if (Environment.isLinux) { + var signed_offset = @intCast(i64, this.offset); + const begin = this.offset; + const val = + // this does the syscall directly, without libc + std.os.linux.sendfile(socket.fd(), this.fd, &signed_offset, this.remain); + this.offset = @intCast(u64, signed_offset); + + const errcode = std.os.linux.getErrno(val); + + this.remain -|= @intCast(u64, this.offset -| begin); + + if (errcode != .SUCCESS or this.remain == 0 or val == 0) { + if (errcode == .SUCCESS) { + return .{ .done = {} }; + } + + return .{ .err = AsyncIO.asError(errcode) }; + } + } else { + var sbytes: std.os.off_t = adjusted_count; + const signed_offset = @bitCast(i64, @as(u64, this.offset)); + const errcode = std.c.getErrno(std.c.sendfile( + this.fd, + socket.fd(), + + signed_offset, + &sbytes, + null, + 0, + )); + const wrote = @intCast(u64, sbytes); + this.offset +|= wrote; + this.remain -|= wrote; + if (errcode != .AGAIN or this.remain == 0 or sbytes == 0) { + if (errcode == .SUCCESS) { + return .{ .done = {} }; + } + + return .{ .err = AsyncIO.asError(errcode) }; + } + } + + return .{ .again = {} }; + } + + pub const Status = union(enum) { + done: void, + err: anyerror, + again: void, + }; +}; + const ProxySSLData = struct { buffer: std.ArrayList(u8), partial: bool, @@ -738,7 +821,7 @@ pub fn onClose( if (client.allow_retry) { client.allow_retry = false; - client.start(client.state.request_body, client.state.body_out_str.?); + client.start(client.state.original_request_body, client.state.body_out_str.?); return; } @@ -915,14 +998,16 @@ pub const InternalState = struct { compressed_body: MutableString = undefined, body_size: usize = 0, request_body: []const u8 = "", + original_request_body: HTTPRequestBody = .{ .bytes = "" }, request_sent_len: usize = 0, fail: anyerror = error.NoError, request_stage: HTTPStage = .pending, response_stage: HTTPStage = .pending, - pub fn init(body: []const u8, body_out_str: *MutableString) InternalState { + pub fn init(body: HTTPRequestBody, body_out_str: *MutableString) InternalState { return .{ - .request_body = body, + .original_request_body = body, + .request_body = if (body == .bytes) body.bytes else "", .compressed_body = MutableString{ .allocator = default_allocator, .list = .{} }, .response_message_buffer = MutableString{ .allocator = default_allocator, .list = .{} }, .body_out_str = body_out_str, @@ -942,6 +1027,7 @@ pub const InternalState = struct { .body_out_str = body_msg, .compressed_body = MutableString{ .allocator = default_allocator, .list = .{} }, .response_message_buffer = MutableString{ .allocator = default_allocator, .list = .{} }, + .original_request_body = .{ .bytes = "" }, .request_body = "", }; } @@ -1191,7 +1277,7 @@ pub const AsyncHTTP = struct { request_headers: Headers.Entries = Headers.Entries{}, response_headers: Headers.Entries = Headers.Entries{}, response_buffer: *MutableString, - request_body: []const u8 = "", + request_body: HTTPRequestBody = .{ .bytes = "" }, allocator: std.mem.Allocator, request_header_buf: string = "", method: Method = Method.GET, @@ -1278,7 +1364,18 @@ pub const AsyncHTTP = struct { hostname: ?[]u8, redirect_type: FetchRedirect, ) AsyncHTTP { - var this = AsyncHTTP{ .allocator = allocator, .url = url, .method = method, .request_headers = headers, .request_header_buf = headers_buf, .request_body = request_body, .response_buffer = response_buffer, .completion_callback = callback, .http_proxy = http_proxy, .async_http_id = if (signal != null) async_http_id.fetchAdd(1, .Monotonic) else 0 }; + var this = AsyncHTTP{ + .allocator = allocator, + .url = url, + .method = method, + .request_headers = headers, + .request_header_buf = headers_buf, + .request_body = .{ .bytes = request_body }, + .response_buffer = response_buffer, + .completion_callback = callback, + .http_proxy = http_proxy, + .async_http_id = if (signal != null) async_http_id.fetchAdd(1, .Monotonic) else 0, + }; this.client = HTTPClient.init(allocator, method, url, headers, headers_buf, signal, hostname); this.client.async_http_id = this.async_http_id; @@ -1648,7 +1745,7 @@ pub fn doRedirect(this: *HTTPClient) void { if (this.aborted != null) { _ = socket_async_http_abort_tracker.swapRemove(this.async_http_id); } - return this.start("", body_out_str); + return this.start(.{ .bytes = "" }, body_out_str); } pub fn isHTTPS(this: *HTTPClient) bool { if (this.http_proxy) |proxy| { @@ -1662,7 +1759,7 @@ pub fn isHTTPS(this: *HTTPClient) bool { } return false; } -pub fn start(this: *HTTPClient, body: []const u8, body_out_str: *MutableString) void { +pub fn start(this: *HTTPClient, body: HTTPRequestBody, body_out_str: *MutableString) void { body_out_str.reset(); std.debug.assert(this.state.response_message_buffer.list.capacity == 0); @@ -1730,7 +1827,7 @@ pub fn onWritable(this: *HTTPClient, comptime is_first_call: bool, comptime is_s this.setTimeout(socket, 60); - const request = this.buildRequest(this.state.request_body.len); + const request = this.buildRequest(this.state.original_request_body.len()); if (this.http_proxy) |_| { if (this.url.isHTTPS()) { @@ -1784,7 +1881,10 @@ pub fn onWritable(this: *HTTPClient, comptime is_first_call: bool, comptime is_s std.debug.assert(!socket.isShutdown()); std.debug.assert(!socket.isClosed()); } - const amount = socket.write(to_send, false); + const amount = socket.write( + to_send, + false, + ); if (comptime is_first_call) { if (amount == 0) { // don't worry about it @@ -1804,7 +1904,10 @@ pub fn onWritable(this: *HTTPClient, comptime is_first_call: bool, comptime is_s this.state.request_body = this.state.request_body[this.state.request_sent_len - headers_len ..]; } - const has_sent_body = this.state.request_body.len == 0; + const has_sent_body = if (this.state.original_request_body == .bytes) + this.state.request_body.len == 0 + else + false; if (has_sent_headers and has_sent_body) { this.state.request_stage = .done; @@ -1813,7 +1916,11 @@ pub fn onWritable(this: *HTTPClient, comptime is_first_call: bool, comptime is_s if (has_sent_headers) { this.state.request_stage = .body; - std.debug.assert(this.state.request_body.len > 0); + std.debug.assert( + // we should have leftover data OR we use sendfile() + (this.state.original_request_body == .bytes and this.state.request_body.len > 0) or + this.state.original_request_body == .sendfile, + ); // we sent everything, but there's some body leftover if (amount == @intCast(c_int, to_send.len)) { @@ -1826,19 +1933,42 @@ pub fn onWritable(this: *HTTPClient, comptime is_first_call: bool, comptime is_s .body => { this.setTimeout(socket, 60); - const to_send = this.state.request_body; - const amount = socket.write(to_send, true); - if (amount < 0) { - this.closeAndFail(error.WriteFailed, is_ssl, socket); - return; - } + switch (this.state.original_request_body) { + .bytes => { + const to_send = this.state.request_body; + const amount = socket.write(to_send, true); + if (amount < 0) { + this.closeAndFail(error.WriteFailed, is_ssl, socket); + return; + } - this.state.request_sent_len += @intCast(usize, amount); - this.state.request_body = this.state.request_body[@intCast(usize, amount)..]; + this.state.request_sent_len += @intCast(usize, amount); + this.state.request_body = this.state.request_body[@intCast(usize, amount)..]; - if (this.state.request_body.len == 0) { - this.state.request_stage = .done; - return; + if (this.state.request_body.len == 0) { + this.state.request_stage = .done; + return; + } + }, + .sendfile => |*sendfile| { + if (comptime is_ssl) { + @panic("sendfile is only supported without SSL. This code should never have been reached!"); + } + + switch (sendfile.write(socket)) { + .done => { + this.state.request_stage = .done; + return; + }, + .err => |err| { + this.closeAndFail(err, false, socket); + return; + }, + .again => { + socket.markNeedsMoreForSendfile(); + }, + } + }, } }, .proxy_body => { diff --git a/test/js/bun/http/fetch-file-upload.test.ts b/test/js/bun/http/fetch-file-upload.test.ts index 33f0d0581..b070fbd6e 100644 --- a/test/js/bun/http/fetch-file-upload.test.ts +++ b/test/js/bun/http/fetch-file-upload.test.ts @@ -1,5 +1,7 @@ import { expect, test, describe } from "bun:test"; import { withoutAggressiveGC } from "harness"; +import { tmpdir } from "os"; +import { join } from "path"; test("uploads roundtrip", async () => { const body = Bun.file(import.meta.dir + "/fetch.js.txt"); @@ -32,6 +34,35 @@ test("uploads roundtrip", async () => { server.stop(true); }); +test("uploads roundtrip with sendfile()", async () => { + var hugeTxt = "huge".repeat(1024 * 1024 * 32); + const path = join(tmpdir(), "huge.txt"); + require("fs").writeFileSync(path, hugeTxt); + + const server = Bun.serve({ + maxRequestBodySize: 1024 * 1024 * 1024 * 8, + async fetch(req) { + var count = 0; + for await (let chunk of req.body!) { + count += chunk.byteLength; + } + return new Response(count + ""); + }, + }); + + const resp = await fetch("http://" + server.hostname + ":" + server.port, { + body: Bun.file(path), + method: "PUT", + }); + + expect(resp.status).toBe(200); + + const body = parseInt(await resp.text()); + expect(body).toBe(hugeTxt.length); + + server.stop(true); +}); + test("missing file throws the expected error", async () => { Bun.gc(true); // Run this 1000 times to check for GC bugs diff --git a/test/regression/issue/02499.test.ts b/test/regression/issue/02499.test.ts index 0e4666b36..f1ee1da80 100644 --- a/test/regression/issue/02499.test.ts +++ b/test/regression/issue/02499.test.ts @@ -12,8 +12,7 @@ it("onAborted() and onWritable are not called after receiving an empty response testDone(new Error("Test timed out, which means it failed")); }; - const body = new FormData(); - body.append("hey", "hi"); + const invalidJSON = Buffer.from("invalid json"); // We want to test that the server isn't keeping the connection open in a // zombie-like state when an error occurs due to an unhandled rejected promise @@ -69,7 +68,7 @@ it("onAborted() and onWritable are not called after receiving an empty response try { await fetch(`http://${hostname}:${port}/upload`, { - body, + body: invalidJSON, keepalive: false, method: "POST", timeout: true, @@ -91,4 +90,4 @@ it("onAborted() and onWritable are not called after receiving an empty response } timeout.onabort = () => {}; testDone(); -}); +}, 30_000); -- cgit v1.2.3 From e24d579a329d91350623e82b3ec5de79c7160a78 Mon Sep 17 00:00:00 2001 From: Ciro Spaciari Date: Wed, 31 May 2023 22:20:50 -0300 Subject: isolated version of Path compatible with nodejs (#3143) --- src/bun.js/node/types.zig | 7 ++- src/fs.zig | 114 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 87 insertions(+), 34 deletions(-) (limited to 'src') diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index da0459866..f090b3a12 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -1775,7 +1775,7 @@ pub const Path = struct { var path_slice: JSC.ZigString.Slice = args_ptr[0].toSlice(globalThis, heap_allocator); defer path_slice.deinit(); var path = path_slice.slice(); - var path_name = Fs.PathName.init(path); + var path_name = Fs.NodeJSPathName.init(path); var dir = JSC.ZigString.init(path_name.dir); const is_absolute = (isWindows and isZigStringAbsoluteWindows(dir)) or (!isWindows and path.len > 0 and path[0] == '/'); @@ -1788,9 +1788,8 @@ pub const Path = struct { dir = root; } } - // we use filename as base, and base as name because node.js/internals compatibilty - var base = JSC.ZigString.init(path_name.filename); - var name_ = JSC.ZigString.init(path_name.base); + var base = JSC.ZigString.init(path_name.base); + var name_ = JSC.ZigString.init(path_name.filename); var ext = JSC.ZigString.init(path_name.ext); dir.setOutputEncoding(); root.setOutputEncoding(); diff --git a/src/fs.zig b/src/fs.zig index e22b6b0b5..c0f3cd9dd 100644 --- a/src/fs.zig +++ b/src/fs.zig @@ -1190,6 +1190,78 @@ pub const FileSystem = struct { pub const Directory = struct { path: Path, contents: []string }; pub const File = struct { path: Path, contents: string }; +pub const NodeJSPathName = struct { + base: string, + dir: string, + /// includes the leading . + ext: string, + filename: string, + + pub fn init(_path: string) NodeJSPathName { + var path = _path; + var base = path; + // ext must be empty if not detected + var ext: string = ""; + var dir = path; + var is_absolute = true; + var _i = strings.lastIndexOfChar(path, '/'); + var first = true; + while (_i) |i| { + + // Stop if we found a non-trailing slash + if (i + 1 != path.len) { + base = path[i + 1 ..]; + dir = path[0..i]; + is_absolute = false; + break; + } + + // If the path starts with a slash and it's the only slash, it's absolute + if (i == 0 and first) { + base = path[1..]; + dir = &([_]u8{}); + break; + } + + first = false; + // Ignore trailing slashes + + path = path[0..i]; + + _i = strings.lastIndexOfChar(path, '/'); + } + + // clean trailing slashs + if (base.len > 1 and base[base.len - 1] == '/') { + base = base[0 .. base.len - 1]; + } + + // filename is base without extension + var filename = base; + + // if only one character ext = "" even if filename it's "." + if (filename.len > 1) { + // Strip off the extension + var _dot = strings.lastIndexOfChar(filename, '.'); + if (_dot) |dot| { + ext = filename[dot..]; + filename = filename[0..dot]; + } + } + + if (is_absolute) { + dir = &([_]u8{}); + } + + return NodeJSPathName{ + .dir = dir, + .base = base, + .ext = ext, + .filename = filename, + }; + } +}; + pub const PathName = struct { base: string, dir: string, @@ -1254,14 +1326,12 @@ pub const PathName = struct { pub fn init(_path: string) PathName { var path = _path; var base = path; - // ext must be empty if not detected - var ext: string = ""; + var ext = path; var dir = path; var is_absolute = true; + var _i = strings.lastIndexOfChar(path, '/'); - var first = true; while (_i) |i| { - // Stop if we found a non-trailing slash if (i + 1 != path.len) { base = path[i + 1 ..]; @@ -1270,48 +1340,32 @@ pub const PathName = struct { break; } - // If the path starts with a slash and it's the only slash, it's absolute - if (i == 0 and first) { - base = path[1..]; - dir = &([_]u8{}); - break; - } - - first = false; // Ignore trailing slashes - path = path[0..i]; _i = strings.lastIndexOfChar(path, '/'); } - // clean trailing slashs - if (base.len > 1 and base[base.len - 1] == '/') { - base = base[0 .. base.len - 1]; - } - - // filename is base with extension - var filename = base; - - // if only one character ext = "" even if base it's "." - if (base.len > 1) { - // Strip off the extension - var _dot = strings.lastIndexOfChar(base, '.'); - if (_dot) |dot| { - ext = base[dot..]; - base = base[0..dot]; - } + // Strip off the extension + var _dot = strings.lastIndexOfChar(base, '.'); + if (_dot) |dot| { + ext = base[dot..]; + base = base[0..dot]; } if (is_absolute) { dir = &([_]u8{}); } + if (base.len > 1 and base[base.len - 1] == '/') { + base = base[0 .. base.len - 1]; + } + return PathName{ .dir = dir, .base = base, .ext = ext, - .filename = filename, + .filename = if (dir.len > 0) _path[dir.len + 1 ..] else _path, }; } }; -- cgit v1.2.3 From 110d0752f333e4c32c9226e4a94e93f18837f9c7 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Wed, 31 May 2023 19:01:45 -0700 Subject: Add `"macro"` package.json exports condition --- src/options.zig | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'src') diff --git a/src/options.zig b/src/options.zig index d2752471d..a39c9fc6c 100644 --- a/src/options.zig +++ b/src/options.zig @@ -616,6 +616,7 @@ pub const Target = enum { array.set( Target.bun_macro, &[_]string{ + "macro", "bun", "worker", "module", @@ -624,13 +625,6 @@ pub const Target = enum { "browser", }, ); - // array.set(Target.bun_macro, [_]string{ "bun_macro", "browser", "default", },); - - // Original comment: - // The neutral target is for people that don't want esbuild to try to - // pick good defaults for their platform. In that case, the list of main - // fields is empty by default. You must explicitly configure it yourself. - // array.set(Target.neutral, &listc); break :brk array; }; -- cgit v1.2.3