Skip to content

fix(deploy/functions): improve list param parsing robustness #8895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
135 changes: 135 additions & 0 deletions src/deploy/functions/params.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,3 +299,138 @@
await expect(params.resolveParams(paramsToResolve, fakeConfig, {})).to.eventually.be.rejected;
});
});

describe("ParamValue.asList()", () => {
describe("basic usage", () => {
it("should parse JSON-style array with double quotes", () => {
const paramValue = new params.ParamValue('["1", "2", "3"]', false, { list: true });
expect(paramValue.asList()).to.deep.equal(["1", "2", "3"]);
});

it("should split comma-separated values", () => {
const paramValue = new params.ParamValue("a,b,c", false, { list: true });
expect(paramValue.asList()).to.deep.equal(["a", "b", "c"]);
});

it("should handle empty array", () => {
const paramValue = new params.ParamValue("[]", false, { list: true });
expect(paramValue.asList()).to.deep.equal([]);
});

it("should handle a single value", () => {
const paramValue = new params.ParamValue("foo", false, { list: true });
expect(paramValue.asList()).to.deep.equal(["foo"]);
});

it("should handle whitespace around values", () => {
const paramValue = new params.ParamValue(" a , b , c ", false, { list: true });
expect(paramValue.asList()).to.deep.equal([" a ", " b ", " c "]);
});
});

describe("delimiter handling", () => {
it("should use custom delimiter", () => {
const paramValue = new params.ParamValue("a;b;c", false, { list: true });
paramValue.setDelimiter(";");
expect(paramValue.asList()).to.deep.equal(["a", "b", "c"]);
});
});

describe("robustness", () => {
it("should handle values with commas inside JSON array", () => {
const paramValue = new params.ParamValue('["hello, world", "foo, bar"]', false, { list: true });

Check failure on line 341 in src/deploy/functions/params.spec.ts

View workflow job for this annotation

GitHub Actions / unit (22)

Replace `·list:·true` with `⏎········list:·true,⏎·····`
expect(paramValue.asList()).to.deep.equal(["hello, world", "foo, bar"]);
});

it("should fall back to delimiter splitting for malformed JSON", () => {
const paramValue = new params.ParamValue("[a, b, c]", false, { list: true });
expect(paramValue.asList()).to.deep.equal(["a", " b", " c"]);
});

it("should handle special characters", () => {
const paramValue = new params.ParamValue('["@#$%", "^&*()"]', false, { list: true });
expect(paramValue.asList()).to.deep.equal(["@#$%", "^&*()"]);
});

it("should work with fromList static method", () => {
const originalList = ["apple", "banana", "cherry"];
const paramValue = params.ParamValue.fromList(originalList);
expect(paramValue.asList()).to.deep.equal(originalList);
});

it("should handle square brackets in the middle of the string", () => {
const paramValue = new params.ParamValue('["]", "[", "]"]', false, { list: true });
expect(paramValue.asList()).to.deep.equal(["]", "[", "]"]);
});

it("should handle quotes, apostrophes, and backticks in the middle of the string", () => {
const paramValue = new params.ParamValue('["a\\"b", "c`d", "e\'f"]', false, { list: true });
expect(paramValue.asList()).to.deep.equal(["a\"b", "c`d", "e'f"]);

Check failure on line 368 in src/deploy/functions/params.spec.ts

View workflow job for this annotation

GitHub Actions / unit (22)

Replace `"a\"b"` with `'a"b'`
});

it("should handle empty string", () => {
const paramValue = new params.ParamValue("", false, { list: true });
expect(paramValue.asList()).to.deep.equal([""]);
});

it("should handle whitespace-only string", () => {
const paramValue = new params.ParamValue(" ", false, { list: true });
expect(paramValue.asList()).to.deep.equal([" "]);
});

it("should handle JSON array with numbers", () => {
const paramValue = new params.ParamValue("[1, 2, 3]", false, { list: true });
expect(paramValue.asList()).to.deep.equal([1, 2, 3]);
});

it("should handle JSON array with booleans", () => {
const paramValue = new params.ParamValue("[true, false, true]", false, { list: true });
expect(paramValue.asList()).to.deep.equal([true, false, true]);
});

it("should handle JSON array with nulls", () => {
const paramValue = new params.ParamValue('["a", null, "b"]', false, { list: true });
expect(paramValue.asList()).to.deep.equal(["a", null, "b"]);
});

it("should handle trailing delimiter", () => {
const paramValue = new params.ParamValue("a,b,c,", false, { list: true });
expect(paramValue.asList()).to.deep.equal(["a", "b", "c", ""]);
});

it("should handle leading delimiter", () => {
const paramValue = new params.ParamValue(",a,b,c", false, { list: true });
expect(paramValue.asList()).to.deep.equal(["", "a", "b", "c"]);
});

it("should handle consecutive delimiters", () => {
const paramValue = new params.ParamValue("a,,b", false, { list: true });
expect(paramValue.asList()).to.deep.equal(["a", "", "b"]);
});

it("should handle only delimiters", () => {
const paramValue = new params.ParamValue(",,,", false, { list: true });
expect(paramValue.asList()).to.deep.equal(["", "", "", ""]);
});

it("should handle JSON array with empty strings", () => {
const paramValue = new params.ParamValue('["", "a", ""]', false, { list: true });
expect(paramValue.asList()).to.deep.equal(["", "a", ""]);
});

it("should handle JSON array with whitespace strings", () => {
const paramValue = new params.ParamValue('[" ", "a", " "]', false, { list: true });
expect(paramValue.asList()).to.deep.equal([" ", "a", " "]);
});

it("should handle JSON array with nested arrays", () => {
const paramValue = new params.ParamValue('[["a", "b"], ["c"]]', false, { list: true });
expect(paramValue.asList()).to.deep.equal([["a", "b"], ["c"]]);
});

it("should handle JSON array with objects", () => {
const paramValue = new params.ParamValue('[{"foo":1}, {"bar":2}]', false, { list: true });
expect(paramValue.asList()).to.deep.equal([{ foo: 1 }, { bar: 2 }]);
});
});
});
18 changes: 17 additions & 1 deletion src/deploy/functions/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,23 @@ export class ParamValue {
}

asList(): string[] {
return this.rawValue.split(this.delimiter);
let modifiedValue = this.rawValue;

// Handle something like "["a", "b", "c"]"
if (modifiedValue.startsWith("[") && modifiedValue.endsWith("]")) {
try {
return JSON.parse(modifiedValue);
} catch (e) {
// Remove the brackets
// Handles scenarios like "[a, b, c]"
modifiedValue = modifiedValue.replace(/[\[\]]/g, "");

// Not a valid JSON array, fall through to splitting by delimiter.
}
}

// Continue to handle something like "a,b,c"
return modifiedValue.split(this.delimiter);
}

asNumber(): number {
Expand Down
Loading