Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/runtime/valkey_jsc/js_valkey_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,16 @@ fn from_js(global: &JSGlobalObject, value: JSValue) -> JsResult<Option<JSArgumen
return JSArgument::from_js_maybe_file(global, str.to_js(), true);
}

JSArgument::from_js_maybe_file(global, value, false)
let arg = JSArgument::from_js_maybe_file(global, value, false)?;
if let Some(JSArgument::Blob(ref blob)) = arg {
// `shared_view()` is empty for file/S3 stores, which would serialize as "".
if blob.needs_to_read_file() || blob.is_s3() {
return Err(global.throw_invalid_arguments(format_args!(
"RedisClient cannot serialize a file- or S3-backed Blob; pass await blob.bytes() instead"
)));
}
}
Ok(arg)
}

/// Shim around `protocol::valkey_error_to_js` that:
Expand Down
130 changes: 130 additions & 0 deletions test/js/valkey/unit/buffer-operations.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { RedisClient, type TCPSocketListener } from "bun";
import { beforeEach, describe, expect, test } from "bun:test";
import { tempDir } from "harness";
import { join } from "node:path";
import { ConnectionType, createClient, ctx, isEnabled } from "../test-utils";

describe.skipIf(!isEnabled)("Valkey: Buffer Operations", () => {
Expand Down Expand Up @@ -113,3 +116,130 @@ describe.skipIf(!isEnabled)("Valkey: Buffer Operations", () => {
function expectAssert(value: unknown): asserts value {
expect(value).toBeTruthy();
}

// Argument-serialization tests that run without a real server. A Blob whose
// bytes live off-heap (Bun.file / S3) has an empty in-memory view; before the
// fix, that view was silently written to the wire as a zero-length bulk string.
describe("Valkey: Blob argument serialization", () => {
const CRLF = "\r\n";
const HELLO = `%1${CRLF}$5${CRLF}proto${CRLF}:3${CRLF}`;

type PerSocket = { buf: string; hello: boolean };

function createRecorder(): { server: TCPSocketListener<PerSocket>; wire: string[][] } {
const wire: string[][] = [];
const server = Bun.listen<PerSocket>({
hostname: "127.0.0.1",
port: 0,
socket: {
open(s) {
s.data = { buf: "", hello: false };
},
data(s, d) {
const st = s.data;
st.buf += d.toString("latin1");
let out = "";
for (;;) {
const m = /^\*(\d+)\r\n/.exec(st.buf);
if (!m) break;
let i = m[0].length;
const argv: string[] = [];
let ok = true;
for (let k = 0; k < +m[1]; k++) {
const mm = /^\$(\d+)\r\n/.exec(st.buf.slice(i));
if (!mm) {
ok = false;
break;
}
const L = +mm[1];
if (st.buf.length < i + mm[0].length + L + 2) {
ok = false;
break;
}
argv.push(st.buf.substr(i + mm[0].length, L));
i += mm[0].length + L + 2;
}
if (!ok) break;
st.buf = st.buf.slice(i);
if (!st.hello) {
st.hello = true;
out += HELLO;
continue;
}
wire.push(argv);
out += `+OK${CRLF}`;
}
if (out) s.write(out);
},
close() {},
error() {},
},
});
return { server, wire };
}

test("in-memory Blob arguments reach the wire with their bytes intact", async () => {
const { server, wire } = createRecorder();
const client = new RedisClient(`redis://127.0.0.1:${server.port}`, { autoReconnect: false });
try {
await client.connect();
await client.set("mem", new Blob(["blobdata"]));
await client.set("sliced", new Blob(["0123456789"]).slice(2, 5));
await client.set(new Blob(["keyblob"]), "v");
} finally {
client.close();
server.stop(true);
}
expect(wire).toEqual([
["SET", "mem", "blobdata"],
["SET", "sliced", "234"],
["SET", "keyblob", "v"],
]);
});

test("file-backed Blob arguments are rejected instead of serialized as empty", async () => {
using dir = tempDir("valkey-file-blob", { "src.txt": "FILEDATA123" });
const fpath = join(String(dir), "src.txt");

const { server, wire } = createRecorder();
const client = new RedisClient(`redis://127.0.0.1:${server.port}`, { autoReconnect: false });
try {
await client.connect();

const expected = expect.objectContaining({
code: "ERR_INVALID_ARG_TYPE",
message: expect.stringMatching(/file- or S3-backed Blob/),
});
// value position
expect(() => client.set("file", Bun.file(fpath))).toThrow(expected);
// key position
expect(() => client.set(Bun.file(fpath), "as-key")).toThrow(expected);
// single-part Blob around a file-backed Blob shares the file store
expect(() => client.set("wrapped", new Blob([Bun.file(fpath)]))).toThrow(expected);
// reading the file once does not make the Blob itself in-memory
const bf = Bun.file(fpath);
await bf.text();
expect(() => client.set("fileRead", bf)).toThrow(expected);
// nonexistent path is still a file-backed Blob
expect(() => client.set("missing", Bun.file(join(String(dir), "no.txt")))).toThrow(expected);
// S3-backed Blob (no network: the throw happens in argument conversion)
const s3 = new Bun.S3Client({
accessKeyId: "x",
secretAccessKey: "y",
bucket: "b",
endpoint: "http://127.0.0.1:1",
});
expect(() => client.set("s3", s3.file("some-key"))).toThrow(expected);
// other prototype methods route through the same converter
expect(() => client.getBuffer(Bun.file(fpath))).toThrow(expected);
expect(() => client.append("k", Bun.file(fpath))).toThrow(expected);

// nothing above should have produced a frame
await client.set("after", "ok");
} finally {
client.close();
server.stop(true);
}
expect(wire).toEqual([["SET", "after", "ok"]]);
});
});
Loading