Get artifactory to work (#5744)

* Get artifactory to work

* Cleanup url normalization a ltitle more

* Clean up tests

* prettier

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

6 files changed, 163 insertions, 69 deletions

diff --git a/docs/guides/install/jfrog-artifactory.md b/docs/guides/install/jfrog-artifactory.md
new file mode 100644
index 000000000..e4872982b
--- /dev/null
+++ b/docs/guides/install/jfrog-artifactory.md
@@ -0,0 +1,28 @@
+---
+name: Using bun install with Artifactory
+---
+
+[JFrog Artifactory](https://jfrog.com/artifactory/) is a package management system for npm, Docker, Maven, NuGet, Ruby, Helm, and more. It allows you to host your own private npm registry, npm packages, and other types of packages as well.
+
+To use it with `bun install`, add a `bunfig.toml` file to your project with the following contents:
+
+---
+
+### Configure with bunfig.toml
+
+Make sure to replace `MY_SUBDOMAIN` with your JFrog Artifactory subdomain, such as `jarred1234` and MY_TOKEN with your JFrog Artifactory token.
+
+```toml#bunfig.toml
+[install.registry]
+url = "https://MY_SUBDOMAIN.jfrog.io/artifactory/api/npm/npm/_auth=MY_TOKEN"
+# Bun v1.0.3+ supports using an environment variable here
+# url = "$NPM_CONFIG_REGISTRY"
+```
+
+---
+
+### Configure with `$NPM_CONFIG_REGISTRY`
+
+Like with npm, you can use the `NPM_CONFIG_REGISTRY` environment variable to configure JFrog Artifactory with bun install.
+
+---
diff --git a/src/bunfig.zig b/src/bunfig.zig
index bb52e3053..63a6c389d 100644
--- a/src/bunfig.zig
+++ b/src/bunfig.zig
@@ -55,59 +55,68 @@ pub const Bunfig = struct {
             return error.@"Invalid Bunfig";
         }
 
-        fn parseRegistry(this: *Parser, expr: js_ast.Expr) !Api.NpmRegistry {
+        fn parseRegistryURLString(this: *Parser, str: *js_ast.E.String) !Api.NpmRegistry {
+            const url = URL.parse(str.data);
+            var registry = std.mem.zeroes(Api.NpmRegistry);
+
+            // Token
+            if (url.username.len == 0 and url.password.len > 0) {
+                registry.token = url.password;
+                registry.url = try std.fmt.allocPrint(this.allocator, "{s}://{}/{s}/", .{ url.displayProtocol(), url.displayHost(), std.mem.trim(u8, url.pathname, "/") });
+            } else if (url.username.len > 0 and url.password.len > 0) {
+                registry.username = url.username;
+                registry.password = url.password;
+
+                registry.url = try std.fmt.allocPrint(this.allocator, "{s}://{}/{s}/", .{ url.displayProtocol(), url.displayHost(), std.mem.trim(u8, url.pathname, "/") });
+            } else {
+                // Do not include a trailing slash. There might be parameters at the end.
+                registry.url = url.href;
+            }
+
+            return registry;
+        }
+
+        fn parseRegistryObject(this: *Parser, obj: *js_ast.E.Object) !Api.NpmRegistry {
             var registry = std.mem.zeroes(Api.NpmRegistry);
 
+            if (obj.get("url")) |url| {
+                try this.expect(url, .e_string);
+                const href = url.data.e_string.data;
+                // Do not include a trailing slash. There might be parameters at the end.
+                registry.url = href;
+            }
+
+            if (obj.get("username")) |username| {
+                try this.expect(username, .e_string);
+                registry.username = username.data.e_string.data;
+            }
+
+            if (obj.get("password")) |password| {
+                try this.expect(password, .e_string);
+                registry.password = password.data.e_string.data;
+            }
+
+            if (obj.get("token")) |token| {
+                try this.expect(token, .e_string);
+                registry.token = token.data.e_string.data;
+            }
+
+            return registry;
+        }
+
+        fn parseRegistry(this: *Parser, expr: js_ast.Expr) !Api.NpmRegistry {
             switch (expr.data) {
                 .e_string => |str| {
-                    const url = URL.parse(str.data);
-                    // Token
-                    if (url.username.len == 0 and url.password.len > 0) {
-                        registry.token = url.password;
-                        registry.url = try std.fmt.allocPrint(this.allocator, "{s}://{s}/{s}/", .{ url.displayProtocol(), url.displayHostname(), std.mem.trim(u8, url.pathname, "/") });
-                    } else if (url.username.len > 0 and url.password.len > 0) {
-                        registry.username = url.username;
-                        registry.password = url.password;
-                        registry.url = try std.fmt.allocPrint(this.allocator, "{s}://{s}/{s}/", .{ url.displayProtocol(), url.displayHostname(), std.mem.trim(u8, url.pathname, "/") });
-                    } else {
-                        if (strings.hasSuffixComptime(url.href, "/")) {
-                            registry.url = url.href;
-                        } else {
-                            registry.url = try std.fmt.allocPrint(this.allocator, "{s}/", .{url.href});
-                        }
-                    }
+                    return this.parseRegistryURLString(str);
                 },
                 .e_object => |obj| {
-                    if (obj.get("url")) |url| {
-                        try this.expect(url, .e_string);
-                        if (strings.hasSuffixComptime(url.data.e_string.data, "/")) {
-                            registry.url = url.data.e_string.data;
-                        } else {
-                            registry.url = try std.fmt.allocPrint(this.allocator, "{s}/", .{url.data.e_string.data});
-                        }
-                    }
-
-                    if (obj.get("username")) |username| {
-                        try this.expect(username, .e_string);
-                        registry.username = username.data.e_string.data;
-                    }
-
-                    if (obj.get("password")) |password| {
-                        try this.expect(password, .e_string);
-                        registry.password = password.data.e_string.data;
-                    }
-
-                    if (obj.get("token")) |token| {
-                        try this.expect(token, .e_string);
-                        registry.token = token.data.e_string.data;
-                    }
+                    return this.parseRegistryObject(obj);
                 },
                 else => {
                     try this.addError(expr.loc, "Expected registry to be a URL string or an object");
+                    return std.mem.zeroes(Api.NpmRegistry);
                 },
             }
-
-            return registry;
         }
 
         fn loadLogLevel(this: *Parser, expr: js_ast.Expr) !void {
diff --git a/src/install/install.zig b/src/install/install.zig
index c7a2816a5..3757a6980 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -273,8 +273,8 @@ const NetworkTask = struct {
 
             if (tmp.tag == .Dead) {
                 const msg = .{
-                    .fmt = "Failed to join registry \"{s}\" and package \"{s}\" URLs",
-                    .args = .{ scope.url.href, name },
+                    .fmt = "Failed to join registry {} and package {} URLs",
+                    .args = .{ strings.QuotedFormatter{ .text = scope.url.href }, strings.QuotedFormatter{ .text = name } },
                 };
 
                 if (warn_on_error)
@@ -3806,10 +3806,10 @@ pub const PackageManager = struct {
                             switch (response.status_code) {
                                 404 => {
                                     if (comptime log_level != .silent) {
-                                        const fmt = "\n<r><red>error<r>: package <b>\"{s}\"<r> not found <d>{s}{s} 404<r>\n";
+                                        const fmt = "\n<r><red>error<r>: package <b>\"{s}\"<r> not found <d>{}{s} 404<r>\n";
                                         const args = .{
                                             name.slice(),
-                                            task.http.url.displayHostname(),
+                                            task.http.url.displayHost(),
                                             task.http.url.pathname,
                                         };
 
@@ -3823,10 +3823,10 @@ pub const PackageManager = struct {
                                 },
                                 401 => {
                                     if (comptime log_level != .silent) {
-                                        const fmt = "\n<r><red>error<r>: unauthorized <b>\"{s}\"<r> <d>{s}{s} 401<r>\n";
+                                        const fmt = "\n<r><red>error<r>: unauthorized <b>\"{s}\"<r> <d>{}{s} 401<r>\n";
                                         const args = .{
                                             name.slice(),
-                                            task.http.url.displayHostname(),
+                                            task.http.url.displayHost(),
                                             task.http.url.pathname,
                                         };
 
diff --git a/src/install/npm.zig b/src/install/npm.zig
index fec545b0c..24e631836 100644
--- a/src/install/npm.zig
+++ b/src/install/npm.zig
@@ -81,6 +81,7 @@ pub const Registry = struct {
 
             var url = URL.parse(registry.url);
             var auth: string = "";
+            var needs_normalize = false;
 
             if (registry.token.len == 0) {
                 outer: {
@@ -90,10 +91,12 @@ pub const Registry = struct {
                             url.pathname = pathname;
                             url.path = pathname;
                         }
-
+                        var needs_to_check_slash = true;
                         while (strings.lastIndexOfChar(pathname, ':')) |colon| {
                             var segment = pathname[colon + 1 ..];
                             pathname = pathname[0..colon];
+                            needs_to_check_slash = false;
+                            needs_normalize = true;
                             if (pathname.len > 1 and pathname[pathname.len - 1] == '/') {
                                 pathname = pathname[0 .. pathname.len - 1];
                             }
@@ -124,6 +127,47 @@ pub const Registry = struct {
                                 continue;
                             }
                         }
+
+                        // In this case, there is only one.
+                        if (needs_to_check_slash) {
+                            if (strings.lastIndexOfChar(pathname, '/')) |last_slash| {
+                                var remain = pathname[last_slash + 1 ..];
+                                if (strings.indexOfChar(remain, '=')) |eql_i| {
+                                    const segment = remain[0..eql_i];
+                                    var value = remain[eql_i + 1 ..];
+
+                                    // https://github.com/yarnpkg/yarn/blob/6db39cf0ff684ce4e7de29669046afb8103fce3d/src/registries/npm-registry.js#L364
+                                    // Bearer Token
+                                    if (strings.eqlComptime(segment, "_authToken")) {
+                                        registry.token = value;
+                                        pathname = pathname[0 .. last_slash + 1];
+                                        needs_normalize = true;
+                                        break :outer;
+                                    }
+
+                                    if (strings.eqlComptime(segment, "_auth")) {
+                                        auth = value;
+                                        pathname = pathname[0 .. last_slash + 1];
+                                        needs_normalize = true;
+                                        break :outer;
+                                    }
+
+                                    if (strings.eqlComptime(segment, "username")) {
+                                        registry.username = value;
+                                        pathname = pathname[0 .. last_slash + 1];
+                                        needs_normalize = true;
+                                        break :outer;
+                                    }
+
+                                    if (strings.eqlComptime(segment, "_password")) {
+                                        registry.password = value;
+                                        pathname = pathname[0 .. last_slash + 1];
+                                        needs_normalize = true;
+                                        break :outer;
+                                    }
+                                }
+                            }
+                        }
                     }
 
                     registry.username = env.getAuto(registry.username);
@@ -144,6 +188,16 @@ pub const Registry = struct {
 
             registry.token = env.getAuto(registry.token);
 
+            if (needs_normalize) {
+                url = URL.parse(
+                    try std.fmt.allocPrint(allocator, "{s}://{}/{s}/", .{
+                        url.displayProtocol(),
+                        url.displayHost(),
+                        strings.trim(url.pathname, "/"),
+                    }),
+                );
+            }
+
             return Scope{ .name = name, .url = url, .token = registry.token, .auth = auth };
         }
     };
diff --git a/test/cli/install/bun-add.test.ts b/test/cli/install/bun-add.test.ts
index c00c0ba7b..14ae44f73 100644
--- a/test/cli/install/bun-add.test.ts
+++ b/test/cli/install/bun-add.test.ts
@@ -20,7 +20,11 @@ import {
 beforeAll(dummyBeforeAll);
 afterAll(dummyAfterAll);
 
+let port: string;
 let add_dir: string;
+beforeAll(() => {
+  port = new URL(root_url).port;
+});
 
 beforeEach(async () => {
   add_dir = await mkdtemp(join(await realpath(tmpdir()), "bun-add.test"));
@@ -102,9 +106,9 @@ it("should reject missing package", async () => {
   });
   expect(stderr).toBeDefined();
   const err = await new Response(stderr).text();
-  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(err).toContain("bun add");
+  expect(err).toContain("error: MissingPackageJSON");
+  expect(err).toContain(`note: error occured while resolving file:${add_path}`);
 
   expect(stdout).toBeDefined();
   const out = await new Response(stdout).text();
@@ -144,9 +148,9 @@ it("should reject invalid path without segfault", async () => {
   });
   expect(stderr).toBeDefined();
   const err = await new Response(stderr).text();
-  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(err).toContain("bun add");
+  expect(err).toContain("error: MissingPackageJSON");
+  expect(err).toContain(`note: error occured while resolving file://${add_path}`);
 
   expect(stdout).toBeDefined();
   const out = await new Response(stdout).text();
@@ -189,7 +193,7 @@ it("should handle semver-like names", async () => {
   });
   expect(stderr).toBeDefined();
   const err = await new Response(stderr).text();
-  expect(err.split(/\r?\n/)).toContain('error: package "1.2.3" not found localhost/1.2.3 404');
+  expect(err.split(/\r?\n/)).toContain(`error: package "1.2.3" not found localhost:${port}/1.2.3 404`);
   expect(stdout).toBeDefined();
   expect(await new Response(stdout).text()).toBe("");
   expect(await exited).toBe(1);
@@ -232,7 +236,7 @@ it("should handle @scoped names", async () => {
   });
   expect(stderr).toBeDefined();
   const err = await new Response(stderr).text();
-  expect(err.split(/\r?\n/)).toContain('error: package "@bar/baz" not found localhost/@bar%2fbaz 404');
+  expect(err.split(/\r?\n/)).toContain(`error: package "@bar/baz" not found localhost:${port}/@bar%2fbaz 404`);
   expect(stdout).toBeDefined();
   expect(await new Response(stdout).text()).toBe("");
   expect(await exited).toBe(1);
@@ -1512,7 +1516,7 @@ async function installRedirectsToAdd(saveFlagFirst: boolean) {
     " 1 packages installed",
   ]);
   expect(await exited).toBe(0);
-  expect((await file(join(package_dir, "package.json")).text()).includes("bun-add.test"));
+  expect(await file(join(package_dir, "package.json")).text()).toInclude("bun-add.test");
 }
 
 it("should add dependency alongside peerDependencies", async () => {
diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts
index 25cd0aca7..354909745 100644
--- a/test/cli/install/bun-install.test.ts
+++ b/test/cli/install/bun-install.test.ts
@@ -96,7 +96,7 @@ it("should handle missing package", async () => {
   });
   expect(stderr).toBeDefined();
   const err = await new Response(stderr).text();
-  expect(err.split(/\r?\n/)).toContain('error: package "foo" not found localhost/foo 404');
+  expect(err.split(/\r?\n/)).toContain(`error: package "foo" not found localhost:${new URL(root_url).port}/foo 404`);
   expect(stdout).toBeDefined();
   expect(await new Response(stdout).text()).toBeEmpty();
   expect(await exited).toBe(1);
@@ -6022,13 +6022,13 @@ describe("Registry URLs", () => {
     ["https://registry.npmjs.org/", false],
     ["https://artifactory.xxx.yyy/artifactory/api/npm/my-npm/", false], // https://github.com/oven-sh/bun/issues/3899
     ["https://artifactory.xxx.yyy/artifactory/api/npm/my-npm", false], // https://github.com/oven-sh/bun/issues/5368
-    ["", true],
+    // ["", true],
     ["https:example.org", false],
     ["https://////example.com///", false],
     ["https://example.com/https:example.org", false],
     ["https://example.com/[]?[]#[]", false],
     ["https://example/%?%#%", false],
-    ["c:", false],
+    ["c:", true],
     ["c:/", false],
     ["https://點看", false], // gets converted to punycode
     ["https://xn--c1yn36f/", false],
@@ -6067,11 +6067,10 @@ describe("Registry URLs", () => {
       const err = await new Response(stderr).text();
 
       if (fails) {
-        const url = regURL.at(-1) === "/" ? regURL : regURL + "/";
-        expect(err.includes(`Failed to join registry \"${url}\" and package \"notapackage\" URLs`)).toBeTrue();
-        expect(err.includes("error: InvalidURL")).toBeTrue();
+        expect(err).toContain(`Failed to join registry "${regURL}" and package "notapackage" URLs`);
+        expect(err).toContain("error: InvalidURL");
       } else {
-        expect(err.includes("error: notapackage@0.0.2 failed to resolve")).toBeTrue();
+        expect(err).toContain("error: notapackage@0.0.2 failed to resolve");
       }
       // fails either way, since notapackage is, well, not a real package.
       expect(await exited).not.toBe(0);
@@ -6108,8 +6107,8 @@ describe("Registry URLs", () => {
     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(err).toContain(`Failed to join registry "${regURL}" and package "notapackage" URLs`);
+    expect(err).toContain("warn: InvalidURL");
 
     expect(await exited).toBe(0);
   });
@@ -6149,8 +6148,8 @@ describe("Registry URLs", () => {
     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(err).toContain(`Failed to join registry "${regURL}" and package "notapackage" URLs`);
+    expect(err).toContain("warn: InvalidURL");
 
     expect(await exited).toBe(0);
   });