Skip to content

Commit c2c0db2

Browse files
committed
fix: address PR review comments
- parseBlockMap duplicate check now uses Object.hasOwn() so prototype- chain property names like `toString` and `constructor` aren't falsely rejected as duplicates. - Drop ':' from TAG_KEY_PATTERN. The block-map parser splits on the first ':', so a tag key containing ':' (e.g. `foo:bar`) couldn't be expressed in YAML form anyway. Aligning the JSON form keeps validation consistent across both input styles. Also rewords the validation error message to match the trimmed character set. - Rename the misleading "rejects JSON arrays" test to make clear it rejects non-string JSON values; add a separate case for a root-level JSON array (which falls through to the YAML parser); add a regression test for the prototype-chain dupe-check fix.
1 parent 7663c38 commit c2c0db2

4 files changed

Lines changed: 27 additions & 9 deletions

File tree

__tests__/tags.test.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,15 @@ describe("parseTags", () => {
5555
expect(() => parseTags("env: a\nenv: b")).toThrow(/duplicate tag key/);
5656
});
5757

58+
it("does not treat Object.prototype property names as duplicates", () => {
59+
// Regression: `if (key in out)` would falsely flag built-in property
60+
// names like `toString` or `constructor` as duplicates on first use.
61+
expect(parseTags("toString: bar\nconstructor: baz")).toEqual({
62+
toString: "bar",
63+
constructor: "baz",
64+
});
65+
});
66+
5867
it("rejects values longer than 256 chars", () => {
5968
const long = "x".repeat(257);
6069
expect(() => parseTags(`big: ${long}`)).toThrow(/exceeds 256/);
@@ -73,18 +82,27 @@ describe("parseTags", () => {
7382
expect(() => parseTags("{ not json")).toThrow(/not valid JSON/);
7483
});
7584

76-
it("rejects JSON arrays", () => {
77-
// Arrays don't start with `{`, so the parser falls through to YAML parsing.
78-
// This test ensures wrapping a literal `{` array-like in JSON fails clearly.
85+
it("rejects non-string JSON values (e.g. an array under a key)", () => {
7986
expect(() => parseTags('{"tags":["a","b"]}')).toThrow(/must be a string/);
8087
});
8188

89+
it("treats a root-level JSON array as YAML and rejects it as malformed", () => {
90+
// A literal `[…]` at the root doesn't start with `{`, so the parser
91+
// falls through to the block-map path, where it's rejected because the
92+
// first non-empty line lacks a "key: value" separator.
93+
expect(() => parseTags('["a","b"]')).toThrow(/expected "key: value"/);
94+
});
95+
8296
it("rejects JSON values that aren't strings", () => {
8397
expect(() => parseTags('{"replicas":3}')).toThrow(/must be a string/);
8498
});
8599

86100
it("rejects JSON keys with invalid characters", () => {
87101
expect(() => parseTags('{"1bad":"x"}')).toThrow(/start with a letter/);
88102
});
103+
104+
it("rejects JSON keys containing ':' (would conflict with the YAML separator)", () => {
105+
expect(() => parseTags('{"foo:bar":"value"}')).toThrow(/letters, numbers, and "_\.\/-"/);
106+
});
89107
});
90108
});

dist/index.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)