aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/bun.js/bindings/BunString.cpp11
-rw-r--r--src/bun.js/bindings/bindings.zig9
-rw-r--r--src/install/install.zig91
-rw-r--r--src/logger.zig29
-rw-r--r--test/cli/install/bun-add.test.ts18
-rw-r--r--test/cli/install/bun-install.test.ts149
6 files changed, 243 insertions, 64 deletions
diff --git a/src/bun.js/bindings/BunString.cpp b/src/bun.js/bindings/BunString.cpp
index 8c5a1e66b..416d5d334 100644
--- a/src/bun.js/bindings/BunString.cpp
+++ b/src/bun.js/bindings/BunString.cpp
@@ -381,6 +381,17 @@ extern "C" BunString URL__getHref(BunString* input)
return Bun::toStringRef(url.string());
}
+extern "C" BunString URL__getHrefJoin(BunString* baseStr, BunString *relativeStr)
+{
+ auto base = Bun::toWTFString(*baseStr);
+ auto relative = Bun::toWTFString(*relativeStr);
+ auto url = WTF::URL(WTF::URL(base), relative);
+ if (!url.isValid() || url.isEmpty())
+ return { BunStringTag::Dead };
+
+ return Bun::toStringRef(url.string());
+}
+
extern "C" WTF::URL* URL__fromString(BunString* input)
{
auto&& str = Bun::toWTFString(*input);
diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig
index 870b78738..6729ab9ab 100644
--- a/src/bun.js/bindings/bindings.zig
+++ b/src/bun.js/bindings/bindings.zig
@@ -5480,12 +5480,21 @@ pub const URL = opaque {
extern fn URL__getHrefFromJS(JSValue, *JSC.JSGlobalObject) String;
extern fn URL__getHref(*String) String;
extern fn URL__getFileURLString(*String) String;
+ extern fn URL__getHrefJoin(*String, *String) String;
+
pub fn hrefFromString(str: bun.String) String {
JSC.markBinding(@src());
var input = str;
return URL__getHref(&input);
}
+ pub fn join(base: bun.String, relative: bun.String) String {
+ JSC.markBinding(@src());
+ var base_str = base;
+ var relative_str = relative;
+ return URL__getHrefJoin(&base_str, &relative_str);
+ }
+
pub fn fileURLFromString(str: bun.String) String {
JSC.markBinding(@src());
var input = str;
diff --git a/src/install/install.zig b/src/install/install.zig
index 768113d35..e8b95b820 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -249,61 +249,31 @@ const NetworkTask = struct {
allocator: std.mem.Allocator,
scope: *const Npm.Registry.Scope,
loaded_manifest: ?Npm.PackageManifest,
+ warn_on_error: bool,
) !void {
- const pathname: string = if (!strings.eqlComptime(scope.url.pathname, "/"))
- scope.url.pathname
- else
- @as(string, "");
+ this.url_buf = blk: {
+ const tmp = bun.JSC.URL.join(
+ bun.String.fromUTF8(scope.url.href),
+ bun.String.fromUTF8(name),
+ );
+ defer tmp.deref();
- if (pathname.len > 0) {
- if (scope.url.getPort()) |port_number| {
- this.url_buf = try std.fmt.allocPrint(
- allocator,
- "{s}://{s}:{d}/{s}/{s}",
- .{
- scope.url.displayProtocol(),
- scope.url.displayHostname(),
- port_number,
- strings.withoutTrailingSlash(pathname),
- name,
- },
- );
- } else {
- this.url_buf = try std.fmt.allocPrint(
- allocator,
- "{s}://{s}/{s}/{s}",
- .{
- scope.url.displayProtocol(),
- scope.url.displayHostname(),
- strings.withoutTrailingSlash(pathname),
- name,
- },
- );
- }
- } else {
- if (scope.url.getPort()) |port_number| {
- this.url_buf = try std.fmt.allocPrint(
- allocator,
- "{s}://{s}:{d}/{s}",
- .{
- scope.url.displayProtocol(),
- scope.url.displayHostname(),
- port_number,
- name,
- },
- );
- } else {
- this.url_buf = try std.fmt.allocPrint(
- allocator,
- "{s}://{s}/{s}",
- .{
- scope.url.displayProtocol(),
- scope.url.displayHostname(),
- name,
- },
- );
+ if (tmp.tag == .Dead) {
+ const msg = .{
+ .fmt = "Failed to join registry \"{s}\" and package \"{s}\" URLs",
+ .args = .{ scope.url.href, name },
+ };
+
+ if (warn_on_error)
+ this.package_manager.log.addWarningFmt(null, .{}, allocator, msg.fmt, msg.args) catch unreachable
+ else
+ this.package_manager.log.addErrorFmt(null, .{}, allocator, msg.fmt, msg.args) catch unreachable;
+
+ return error.InvalidURL;
}
- }
+ // This actually duplicates the string! So we defer deref the WTF managed one above.
+ break :blk try tmp.toOwnedSlice(allocator);
+ };
var last_modified: string = "";
var etag: string = "";
@@ -3075,6 +3045,7 @@ pub const PackageManager = struct {
this.allocator,
this.scopeForPackageName(name_str),
loaded_manifest,
+ dependency.behavior.isOptional() or dependency.behavior.isPeer(),
);
this.enqueueNetworkTask(network_task);
}
@@ -3432,7 +3403,21 @@ pub const PackageManager = struct {
i,
&dependency,
resolution,
- ) catch {};
+ ) catch |err| {
+ const note = .{
+ .fmt = "error occured while resolving {s}",
+ .args = .{
+ lockfile.str(&dependency.realname()),
+ },
+ };
+
+ if (dependency.behavior.isOptional() or dependency.behavior.isPeer())
+ this.log.addWarningWithNote(null, .{}, this.allocator, @errorName(err), note.fmt, note.args) catch unreachable
+ else
+ this.log.addZigErrorWithNote(this.allocator, err, note.fmt, note.args) catch unreachable;
+
+ continue;
+ };
}
this.drainDependencyList();
diff --git a/src/logger.zig b/src/logger.zig
index 7340efc3d..dfe60e2af 100644
--- a/src/logger.zig
+++ b/src/logger.zig
@@ -1027,6 +1027,20 @@ pub const Log = struct {
});
}
+ pub fn addZigErrorWithNote(log: *Log, allocator: std.mem.Allocator, err: anyerror, comptime noteFmt: string, args: anytype) !void {
+ @setCold(true);
+ log.errors += 1;
+
+ var notes = try allocator.alloc(Data, 1);
+ notes[0] = rangeData(null, Range.None, allocPrint(allocator, noteFmt, args) catch unreachable);
+
+ try log.addMsg(.{
+ .kind = .err,
+ .data = rangeData(null, Range.None, @errorName(err)),
+ .notes = notes,
+ });
+ }
+
pub fn addRangeWarning(log: *Log, source: ?*const Source, r: Range, text: string) !void {
@setCold(true);
if (!Kind.shouldPrint(.warn, log.level)) return;
@@ -1092,6 +1106,21 @@ pub const Log = struct {
});
}
+ pub fn addWarningWithNote(log: *Log, source: ?*const Source, l: Loc, allocator: std.mem.Allocator, warn: string, comptime note_fmt: string, note_args: anytype) !void {
+ @setCold(true);
+ if (!Kind.shouldPrint(.warn, log.level)) return;
+ log.warnings += 1;
+
+ var notes = try allocator.alloc(Data, 1);
+ notes[0] = rangeData(source, Range{ .loc = l }, allocPrint(allocator, note_fmt, note_args) catch unreachable);
+
+ try log.addMsg(.{
+ .kind = .warn,
+ .data = rangeData(null, Range.None, warn),
+ .notes = notes,
+ });
+ }
+
pub fn addRangeDebug(log: *Log, source: ?*const Source, r: Range, text: string) !void {
@setCold(true);
if (!Kind.shouldPrint(.debug, log.level)) return;
diff --git a/test/cli/install/bun-add.test.ts b/test/cli/install/bun-add.test.ts
index 589b7a87e..4461584e4 100644
--- a/test/cli/install/bun-add.test.ts
+++ b/test/cli/install/bun-add.test.ts
@@ -102,11 +102,10 @@ it("should reject missing package", async () => {
});
expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
- expect(err.replace(/^(.*?) v[^\n]+/, "$1").split(/\r?\n/)).toEqual([
- "bun add",
- `error: file:${add_path} failed to resolve`,
- "",
- ]);
+ expect(err.includes("bun add")).toBeTrue();
+ expect(err.includes("error: MissingPackageJSON")).toBeTrue();
+ expect(err.includes(`note: error occured while resolving file:${add_path}`)).toBeTrue();
+
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out).toBe("");
@@ -145,11 +144,10 @@ it("should reject invalid path without segfault", async () => {
});
expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
- expect(err.replace(/^(.*?) v[^\n]+/, "$1").split(/\r?\n/)).toEqual([
- "bun add",
- `error: file://${add_path} failed to resolve`,
- "",
- ]);
+ expect(err.includes("bun add")).toBeTrue();
+ expect(err.includes("error: MissingPackageJSON")).toBeTrue();
+ expect(err.includes(`note: error occured while resolving file://${add_path}`)).toBeTrue();
+
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out).toBe("");
diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts
index 28edac092..09da0bbd4 100644
--- a/test/cli/install/bun-install.test.ts
+++ b/test/cli/install/bun-install.test.ts
@@ -1,5 +1,5 @@
import { file, listen, Socket, spawn } from "bun";
-import { afterAll, afterEach, beforeAll, beforeEach, expect, it } from "bun:test";
+import { afterAll, afterEach, beforeAll, beforeEach, expect, it, describe, test } from "bun:test";
import { bunExe, bunEnv as env } from "harness";
import { access, mkdir, readlink, realpath, rm, writeFile } from "fs/promises";
import { join } from "path";
@@ -6006,3 +6006,150 @@ it("should handle `workspace:*` on both root & child", async () => {
expect(await file(join(package_dir, "node_modules", "baz", "package.json")).text()).toEqual(baz_package);
await access(join(package_dir, "bun.lockb"));
});
+
+describe("Registry URLs", () => {
+ // Some of the non failing URLs are invalid, but bun's URL parser ignores
+ // the validation error and returns a valid serialized URL anyway.
+ const registryURLs: [url: string, fails: boolean][] = [
+ ["asdfghjklqwertyuiop", true],
+ [" ", true],
+ ["::::::::::::::::", true],
+ ["https://ex ample.org/", true],
+ ["example", true],
+ ["https://example.com:demo", true],
+ ["http://[www.example.com]/", true],
+ ["c:", true],
+ ["c:a", true],
+ ["https://registry.npmjs.org/", false],
+ ["https://artifactory.xxx.yyy/artifactory/api/npm/my-npm/", false], // https://github.com/oven-sh/bun/issues/3899
+ ["", false],
+ ["https:example.org", false],
+ ["https://////example.com///", false],
+ ["https://example.com/https:example.org", false],
+ ["https://example.com/[]?[]#[]", false],
+ ["https://example/%?%#%", false],
+ ["c:/", false],
+ ["https://點看", false], // gets converted to punycode
+ ["https://xn--c1yn36f/", false],
+ ];
+
+ for (const entry of registryURLs) {
+ const regURL = entry[0];
+ const fails = entry[1];
+
+ it(`should ${fails ? "fail" : "handle"} joining registry and package URLs (${regURL})`, async () => {
+ await writeFile(join(package_dir, "bunfig.toml"), `[install]\ncache = false\nregistry = "${regURL}"`);
+
+ await writeFile(
+ join(package_dir, "package.json"),
+ JSON.stringify({
+ name: "foo",
+ version: "0.0.1",
+ dependencies: {
+ notapackage: "0.0.2",
+ },
+ }),
+ );
+
+ const { stdout, stderr, exited } = spawn({
+ cmd: [bunExe(), "install"],
+ cwd: package_dir,
+ stdout: null,
+ stdin: "pipe",
+ stderr: "pipe",
+ env,
+ });
+ expect(stdout).toBeDefined();
+ expect(await new Response(stdout).text()).toBeEmpty();
+
+ expect(stderr).toBeDefined();
+ const err = await new Response(stderr).text();
+
+ if (fails) {
+ expect(err.includes(`Failed to join registry \"${regURL}\" and package \"notapackage\" URLs`)).toBeTrue();
+ expect(err.includes("error: InvalidURL")).toBeTrue();
+ } else {
+ expect(err.includes("error: notapackage@0.0.2 failed to resolve")).toBeTrue();
+ }
+ // fails either way, since notapackage is, well, not a real package.
+ expect(await exited).not.toBe(0);
+ });
+ }
+
+ it("shouldn't fail joining invalid registry and package URLs for optional dependencies", async () => {
+ const regURL = "asdfghjklqwertyuiop";
+
+ await writeFile(join(package_dir, "bunfig.toml"), `[install]\ncache = false\nregistry = "${regURL}"`);
+
+ await writeFile(
+ join(package_dir, "package.json"),
+ JSON.stringify({
+ name: "foo",
+ version: "0.0.1",
+ optionalDependencies: {
+ notapackage: "0.0.2",
+ },
+ }),
+ );
+
+ const { stdout, stderr, exited } = spawn({
+ cmd: [bunExe(), "install"],
+ cwd: package_dir,
+ stdout: null,
+ stdin: "pipe",
+ stderr: "pipe",
+ env,
+ });
+ expect(stdout).toBeDefined();
+ expect(await new Response(stdout).text()).not.toBeEmpty();
+
+ expect(stderr).toBeDefined();
+ const err = await new Response(stderr).text();
+
+ expect(err.includes(`Failed to join registry \"${regURL}\" and package \"notapackage\" URLs`)).toBeTrue();
+ expect(err.includes("warn: InvalidURL")).toBeTrue();
+
+ expect(await exited).toBe(0);
+ });
+
+ // TODO: This test should fail if the param `warn_on_error` is true in
+ // `(install.zig).NetworkTask.forManifest()`. Unfortunately, that
+ // code never gets run for peer dependencies unless you do some package
+ // manifest magic. I doubt it'd ever fail, but having a dedicated
+ // test would be nice.
+ test.todo("shouldn't fail joining invalid registry and package URLs for peer dependencies", async () => {
+ const regURL = "asdfghjklqwertyuiop";
+
+ await writeFile(join(package_dir, "bunfig.toml"), `[install]\ncache = false\nregistry = "${regURL}"`);
+
+ await writeFile(
+ join(package_dir, "package.json"),
+ JSON.stringify({
+ name: "foo",
+ version: "0.0.1",
+ peerDependencies: {
+ notapackage: "0.0.2",
+ },
+ }),
+ );
+
+ const { stdout, stderr, exited } = spawn({
+ cmd: [bunExe(), "install"],
+ cwd: package_dir,
+ stdout: null,
+ stdin: "pipe",
+ stderr: "pipe",
+ env,
+ });
+ expect(stdout).toBeDefined();
+ expect(await new Response(stdout).text()).not.toBeEmpty();
+
+ expect(stderr).toBeDefined();
+ const err = await new Response(stderr).text();
+
+ expect(err.includes(`Failed to join registry \"${regURL}\" and package \"notapackage\" URLs`)).toBeTrue();
+ expect(err.includes("warn: InvalidURL")).toBeTrue();
+
+ expect(await exited).toBe(0);
+ });
+});
generated by cgit v1.2.3 (git 2.39.1) at 2025-06-26 20:21:50 +0000