diff --git a/package-lock.json b/package-lock.json index 6bd8f99f..9aa77a9d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,8 @@ "dependencies": { "@azure/app-configuration": "^1.6.1", "@azure/identity": "^4.2.1", - "@azure/keyvault-secrets": "^4.7.0" + "@azure/keyvault-secrets": "^4.7.0", + "jsonc-parser": "^3.3.1" }, "devDependencies": { "@rollup/plugin-typescript": "^11.1.2", @@ -1229,9 +1230,9 @@ } }, "node_modules/brace-expansion": { - "version": "1.1.11", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", - "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", "dev": true, "dependencies": { "balanced-match": "^1.0.0", @@ -2488,6 +2489,11 @@ "integrity": "sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==", "dev": true }, + "node_modules/jsonc-parser": { + "version": "3.3.1", + "resolved": "https://registry.npmjs.org/jsonc-parser/-/jsonc-parser-3.3.1.tgz", + "integrity": "sha512-HUgH65KyejrUFPvHFPbqOY0rsFip3Bo5wb4ngvdi1EpCYWUQDC5V+Y7mZws+DLkr4M//zQJoanu1SP+87Dv1oQ==" + }, "node_modules/jsonwebtoken": { "version": "9.0.2", "resolved": "https://registry.npmjs.org/jsonwebtoken/-/jsonwebtoken-9.0.2.tgz", @@ -2794,11 +2800,10 @@ } }, "node_modules/mocha/node_modules/brace-expansion": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", - "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz", + "integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==", "dev": true, - "license": "MIT", "dependencies": { "balanced-match": "^1.0.0" } @@ -3222,9 +3227,9 @@ } }, "node_modules/rimraf/node_modules/brace-expansion": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", - "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz", + "integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==", "dev": true, "dependencies": { "balanced-match": "^1.0.0" diff --git a/package.json b/package.json index 4ac941b2..57e76127 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "dependencies": { "@azure/app-configuration": "^1.6.1", "@azure/identity": "^4.2.1", - "@azure/keyvault-secrets": "^4.7.0" + "@azure/keyvault-secrets": "^4.7.0", + "jsonc-parser": "^3.3.1" } } diff --git a/rollup.config.mjs b/rollup.config.mjs index 16224a2f..0df0e168 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -11,6 +11,7 @@ export default [ "@azure/identity", "crypto", "dns/promises", + "jsonc-parser", "@microsoft/feature-management" ], input: "src/index.ts", diff --git a/src/JsonKeyValueAdapter.ts b/src/JsonKeyValueAdapter.ts index 84266069..75ecaffc 100644 --- a/src/JsonKeyValueAdapter.ts +++ b/src/JsonKeyValueAdapter.ts @@ -2,6 +2,7 @@ // Licensed under the MIT license. import { ConfigurationSetting, featureFlagContentType, secretReferenceContentType } from "@azure/app-configuration"; +import { stripComments } from "jsonc-parser"; import { parseContentType, isJsonContentType } from "./common/contentType.js"; import { IKeyValueAdapter } from "./IKeyValueAdapter.js"; @@ -25,13 +26,19 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter { async processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]> { let parsedValue: unknown; if (setting.value !== undefined) { - try { - parsedValue = JSON.parse(setting.value); - } catch (error) { - parsedValue = setting.value; + const parseResult = this.#tryParseJson(setting.value); + if (parseResult.success) { + parsedValue = parseResult.result; + } else { + // Try parsing with comments stripped + const parseWithoutCommentsResult = this.#tryParseJson(stripComments(setting.value)); + if (parseWithoutCommentsResult.success) { + parsedValue = parseWithoutCommentsResult.result; + } else { + // If still not valid JSON, return the original value + parsedValue = setting.value; + } } - } else { - parsedValue = setting.value; } return [setting.key, parsedValue]; } @@ -39,4 +46,15 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter { async onChangeDetected(): Promise { return; } + + #tryParseJson(value: string): { success: true; result: unknown } | { success: false } { + try { + return { success: true, result: JSON.parse(value) }; + } catch (error) { + if (error instanceof SyntaxError) { + return { success: false }; + } + throw error; + } + } } diff --git a/test/json.test.ts b/test/json.test.ts index a2b57907..fbb5f03e 100644 --- a/test/json.test.ts +++ b/test/json.test.ts @@ -60,7 +60,7 @@ describe("json", function () { createMockedJsonKeyValue("json.settings.object", "{}"), createMockedJsonKeyValue("json.settings.array", "[]"), createMockedJsonKeyValue("json.settings.number", "8"), - createMockedJsonKeyValue("json.settings.string", "string"), + createMockedJsonKeyValue("json.settings.string", "\"string\""), createMockedJsonKeyValue("json.settings.false", "false"), createMockedJsonKeyValue("json.settings.true", "true"), createMockedJsonKeyValue("json.settings.null", "null"), @@ -88,4 +88,87 @@ describe("json", function () { expect(settings.get("json.settings.emptyString")).eq("", "is empty string"); expect(settings.get("json.settings.illegalString")).eq("[unclosed", "is illegal string"); }); + + it("should load json values with comments", async () => { + // Test various comment styles and positions + const mixedCommentStylesValue = `{ + // Single line comment at start + "ApiSettings": { + "BaseUrl": "https://api.example.com", // Inline single line + /* Multi-line comment + spanning multiple lines */ + "ApiKey": "secret-key", + "Endpoints": [ + // Comment before array element + "/users", + /* Comment between elements */ + "/orders", + "/products" // Comment after element + ] + }, + // Test edge cases + "StringWithSlashes": "This is not a // comment", + "StringWithStars": "This is not a /* comment */", + "UrlValue": "https://example.com/path", // This is a real comment + "EmptyComment": "value", // + /**/ + "AfterEmptyComment": "value2" + /* Final multi-line comment */ + }`; + + // Test invalid JSON with comments + const invalidJsonWithCommentsValue = `// This is a comment + { invalid json structure + // Another comment + missing quotes and braces`; + + // Test only comments (should be invalid JSON) + const onlyCommentsValue = ` + // Just comments + /* No actual content */ + `; + + const keyValues = [ + createMockedJsonKeyValue("MixedCommentStyles", mixedCommentStylesValue), + createMockedJsonKeyValue("InvalidJsonWithComments", invalidJsonWithCommentsValue), + createMockedJsonKeyValue("OnlyComments", onlyCommentsValue) + ]; + + mockAppConfigurationClientListConfigurationSettings([keyValues]); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString); + expect(settings).not.undefined; + + // Verify mixed comment styles are properly parsed + const mixedConfig = settings.get("MixedCommentStyles"); + expect(mixedConfig).not.undefined; + expect(mixedConfig.ApiSettings).not.undefined; + expect(mixedConfig.ApiSettings.BaseUrl).eq("https://api.example.com"); + expect(mixedConfig.ApiSettings.ApiKey).eq("secret-key"); + expect(mixedConfig.ApiSettings.Endpoints).not.undefined; + expect(Array.isArray(mixedConfig.ApiSettings.Endpoints)).eq(true); + expect(mixedConfig.ApiSettings.Endpoints[0]).eq("/users"); + expect(mixedConfig.ApiSettings.Endpoints[1]).eq("/orders"); + expect(mixedConfig.ApiSettings.Endpoints[2]).eq("/products"); + + // Verify edge cases where comment-like text appears in strings + expect(mixedConfig.StringWithSlashes).eq("This is not a // comment"); + expect(mixedConfig.StringWithStars).eq("This is not a /* comment */"); + expect(mixedConfig.UrlValue).eq("https://example.com/path"); + expect(mixedConfig.EmptyComment).eq("value"); + expect(mixedConfig.AfterEmptyComment).eq("value2"); + + // Invalid JSON should fall back to string value + const invalidConfig = settings.get("InvalidJsonWithComments"); + expect(invalidConfig).not.undefined; + expect(typeof invalidConfig).eq("string"); + expect(invalidConfig).eq(invalidJsonWithCommentsValue); + + // Only comments should be treated as string value (invalid JSON) + const onlyCommentsConfig = settings.get("OnlyComments"); + expect(onlyCommentsConfig).not.undefined; + expect(typeof onlyCommentsConfig).eq("string"); + expect(onlyCommentsConfig).eq(onlyCommentsValue); + }); });