mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
bug(ENG-21479): Fix Valkey URL Parsing (#24458)
### What does this PR do? Fixes https://github.com/oven-sh/bun/issues/24385 ### How did you verify your code works? Confirmed that the test added in the first commit fails on mainline `bun` and is fixed in this PR. --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This commit is contained in:
@@ -274,16 +274,45 @@ pub const JSValkeyClient = struct {
|
|||||||
else
|
else
|
||||||
bun.String.static("valkey://localhost:6379");
|
bun.String.static("valkey://localhost:6379");
|
||||||
defer url_str.deref();
|
defer url_str.deref();
|
||||||
|
var fallback_url_buf: [2048]u8 = undefined;
|
||||||
|
|
||||||
// Parse and validate the URL using URL.zig's fromString which returns null for invalid URLs
|
// Parse and validate the URL using URL.zig's fromString which returns null for invalid URLs
|
||||||
const parsed_url = URL.fromString(url_str) orelse {
|
// TODO(markovejnovic): The following check for :// is a stop-gap. It is my expectation
|
||||||
if (url_str.tag != .StaticZigString) {
|
// that URL.fromString returns null if the protocol is not specified. This is not, in-fact,
|
||||||
const url_utf8 = url_str.toUTF8WithoutRef(this_allocator);
|
// the case right now and I do not understand why. It will take some work in JSC to
|
||||||
defer url_utf8.deinit();
|
// understand why this is happening, but since I need to uncork valkey, I'm adding this as
|
||||||
return globalObject.throwInvalidArguments("Invalid URL format: \"{s}\"", .{url_utf8.slice()});
|
// a stop-gap.
|
||||||
|
const parsed_url = get_url: {
|
||||||
|
const url_slice = url_str.toUTF8WithoutRef(this_allocator);
|
||||||
|
defer url_slice.deinit();
|
||||||
|
|
||||||
|
const url_byte_slice = url_slice.slice();
|
||||||
|
|
||||||
|
if (url_byte_slice.len == 0) {
|
||||||
|
return globalObject.throwInvalidArguments("Invalid URL format", .{});
|
||||||
}
|
}
|
||||||
// This should never happen since our default URL is valid
|
|
||||||
return globalObject.throwInvalidArguments("Invalid URL format", .{});
|
if (bun.strings.contains(url_byte_slice, "://")) {
|
||||||
|
break :get_url URL.fromString(url_str) orelse {
|
||||||
|
return globalObject.throwInvalidArguments("Invalid URL format", .{});
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
const corrected_url = get_url_slice: {
|
||||||
|
const written = std.fmt.bufPrintZ(
|
||||||
|
&fallback_url_buf,
|
||||||
|
"valkey://{s}",
|
||||||
|
.{url_byte_slice},
|
||||||
|
) catch {
|
||||||
|
return globalObject.throwInvalidArguments("URL is too long.", .{});
|
||||||
|
};
|
||||||
|
|
||||||
|
break :get_url_slice fallback_url_buf[0..written.len];
|
||||||
|
};
|
||||||
|
|
||||||
|
break :get_url URL.fromUTF8(corrected_url) orelse {
|
||||||
|
return globalObject.throwInvalidArguments("Invalid URL format", .{});
|
||||||
|
};
|
||||||
};
|
};
|
||||||
defer parsed_url.deinit();
|
defer parsed_url.deinit();
|
||||||
|
|
||||||
|
|||||||
11
test/_util/collection.ts
Normal file
11
test/_util/collection.ts
Normal file
@@ -0,0 +1,11 @@
|
|||||||
|
/**
|
||||||
|
* Computes the Cartesian product of multiple arrays.
|
||||||
|
*
|
||||||
|
* @param arrays An array of arrays for which to compute the Cartesian product.
|
||||||
|
* @returns An array containing all combinations of elements from the input arrays.
|
||||||
|
*/
|
||||||
|
export function cartesianProduct<T, U>(left: T[], right: U[]): [T, U][] {
|
||||||
|
return left.flatMap(leftItem =>
|
||||||
|
right.map(rightItem => [leftItem, rightItem] as [T, U])
|
||||||
|
);
|
||||||
|
}
|
||||||
37
test/regression/issue/24385.test.ts
Normal file
37
test/regression/issue/24385.test.ts
Normal file
@@ -0,0 +1,37 @@
|
|||||||
|
// https://github.com/oven-sh/bun/issues/24385
|
||||||
|
// Test for Redis client import regression
|
||||||
|
|
||||||
|
import { cartesianProduct } from "_util/collection";
|
||||||
|
import { expect, test } from "bun:test";
|
||||||
|
import { bunEnv, bunExe } from "harness";
|
||||||
|
|
||||||
|
test.concurrent.each(
|
||||||
|
cartesianProduct(
|
||||||
|
["REDIS_URL", "VALKEY_URL"],
|
||||||
|
[
|
||||||
|
"localhost:6379",
|
||||||
|
"redis+tls+unix:///tmp/redis.sock",
|
||||||
|
"redis+tls://localhost:6379",
|
||||||
|
"redis+unix:///tmp/redis.sock",
|
||||||
|
"redis://localhost:6379",
|
||||||
|
"rediss://localhost:6379",
|
||||||
|
"valkey://localhost:6379",
|
||||||
|
],
|
||||||
|
).map(([k, v]) => ({ key: k, value: v })),
|
||||||
|
)("Redis loads with $key=$value", async ({ key, value }) => {
|
||||||
|
const env = { ...bunEnv, [key]: value };
|
||||||
|
|
||||||
|
await using proc = Bun.spawn({
|
||||||
|
// We need to call redis.duplicate() since Bun lazily imports redis.
|
||||||
|
cmd: [bunExe(), "-e", 'import { redis } from "bun"; const d = redis.duplicate(); console.log("success");'],
|
||||||
|
env,
|
||||||
|
stderr: "pipe",
|
||||||
|
stdout: "pipe",
|
||||||
|
});
|
||||||
|
|
||||||
|
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||||
|
|
||||||
|
expect(stderr).not.toContain("Expected url protocol to be one of");
|
||||||
|
expect(stdout).toContain("success");
|
||||||
|
expect(exitCode).toBe(0);
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user