Skip to content

Commit

Permalink
[FIX] Added validation of name option in ConsumerConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
aricart committed Sep 14, 2022
1 parent b629214 commit 19f7cdc
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 13 deletions.
12 changes: 11 additions & 1 deletion nats-base-client/jsmconsumer_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ import {
} from "./types.ts";
import { BaseApiClient } from "./jsbaseclient_api.ts";
import { ListerFieldFilter, ListerImpl } from "./jslister.ts";
import { validateDurableName, validateStreamName } from "./jsutil.ts";
import {
validateDurableName,
validateStreamName,
validName,
} from "./jsutil.ts";
import { NatsConnectionImpl } from "./nats.ts";
import { Feature } from "./semver.ts";

Expand Down Expand Up @@ -69,6 +73,12 @@ export class ConsumerAPIImpl extends BaseApiClient implements ConsumerAPI {
if (name && !newAPI) {
throw new Error(`consumer 'name' requires server ${min}`);
}
if (name) {
const m = validName(name);
if (m.length) {
throw new Error(`consumer 'name' ${m}`);
}
}

let subj;
let consumerName = "";
Expand Down
20 changes: 15 additions & 5 deletions nats-base-client/jsutil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,24 @@ export function validateName(context: string, name = "") {
if (name === "") {
throw Error(`${context} name required`);
}
const m = validName(name);
if (m.length) {
throw new Error(`invalid ${context} name - ${context} name ${m}`);
}
}

export function validName(name = ""): string {
if (name === "") {
throw Error(`name required`);
}
const bad = [".", "*", ">"];
bad.forEach((v) => {
for (let i = 0; i < bad.length; i++) {
const v = bad[i];
if (name.indexOf(v) !== -1) {
throw Error(
`invalid ${context} name - ${context} name cannot contain '${v}'`,
);
return `cannot contain '${v}'`;
}
});
}
return "";
}

export function defaultConsumer(
Expand Down
62 changes: 55 additions & 7 deletions tests/jsm_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1420,15 +1420,11 @@ Deno.test("jsm - consumers with name and durable_name", async () => {
return;
}

// change the version of the server to force legacy apis
const jsm = await nc.jetstreamManager();
await jsm.streams.add({
name: "A",
subjects: ["foo", "bar"],
});
const { stream } = await initStream(nc);

// should be ok
await jsm.consumers.add("A", {
await jsm.consumers.add(stream, {
name: "x",
durable_name: "x",
ack_policy: AckPolicy.None,
Expand All @@ -1437,7 +1433,7 @@ Deno.test("jsm - consumers with name and durable_name", async () => {
// should fail from the server
await assertRejects(
async () => {
await jsm.consumers.add("A", {
await jsm.consumers.add(stream, {
name: "y",
durable_name: "z",
ack_policy: AckPolicy.None,
Expand All @@ -1449,3 +1445,55 @@ Deno.test("jsm - consumers with name and durable_name", async () => {

await cleanup(ns, nc);
});

Deno.test("jsm - consumer name is validated", async () => {
const { ns, nc } = await setup(
jetstreamServerConf({}, true),
);

if (await notCompatible(ns, nc, "2.9.0")) {
return;
}
const { stream } = await initStream(nc);
const jsm = await nc.jetstreamManager();

function test(
n: string,
conf: Partial<ConsumerConfig> = {},
): Promise<unknown> {
const opts = Object.assign({ name: n, ack_policy: AckPolicy.None }, conf);
return jsm.consumers.add(stream, opts);
}

await assertRejects(
async () => {
await test("hello.world");
},
Error,
"consumer 'name' cannot contain '.'",
);

await assertRejects(
async () => {
await test("hello>world");
},
Error,
"consumer 'name' cannot contain '>'",
);
await assertRejects(
async () => {
await test("one*two");
},
Error,
"consumer 'name' cannot contain '*'",
);
await assertRejects(
async () => {
await test(".");
},
Error,
"consumer 'name' cannot contain '.'",
);

await cleanup(ns, nc);
});

0 comments on commit 19f7cdc

Please sign in to comment.