Skip to content

Commit a4b5273

Browse files
deep copy when constructing object
1 parent 90fc356 commit a4b5273

File tree

2 files changed

+44
-3
lines changed

2 files changed

+44
-3
lines changed

src/appConfigurationImpl.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
333333
if (current[lastSegment] !== undefined) {
334334
throw new InvalidOperationError(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The key should not be part of another key.`);
335335
}
336-
// set value to the last segment
337-
current[lastSegment] = value;
336+
// Deep copy object values to avoid mutating the original objects in #configMap via shared references.
337+
// Using JSON.parse(JSON.stringify(...)) instead of structuredClone for compatibility with Node.js < 17.
338+
current[lastSegment] = typeof value === "object" && value !== null ? JSON.parse(JSON.stringify(value)) : value;
338339
}
339340
return data;
340341
}
@@ -492,7 +493,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
492493
// Use a Map to deduplicate configuration settings by key. When multiple selectors return settings with the same key,
493494
// the configuration setting loaded by the later selector in the iteration order will override the one from the earlier selector.
494495
const loadedSettings: Map<string, ConfigurationSetting> = new Map<string, ConfigurationSetting>();
495-
// deep copy selectors to avoid modification if current client fails
496+
// Deep copy selectors to avoid modification if current client fails.
497+
// Using JSON.parse(JSON.stringify(...)) instead of structuredClone for compatibility with Node.js < 17.
496498
const selectorsToUpdate: PagedSettingsWatcher[] = JSON.parse(
497499
JSON.stringify(selectors)
498500
);

test/load.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,18 @@ const mockedKVs = [{
101101
key: "keyWithEmptyTag",
102102
value: "valueWithEmptyTag",
103103
tags: {"emptyTag": ""}
104+
}, {
105+
key: "app6_markets",
106+
value: JSON.stringify({ gb: { url: "https://example.com/gb" }, dk: { url: "https://example.com/dk" } }),
107+
contentType: "application/json"
108+
}, {
109+
key: "app6_markets.markets.dk",
110+
value: JSON.stringify({ currency: "DKK" }),
111+
contentType: "application/json"
112+
}, {
113+
key: "app6_markets.markets.gb",
114+
value: JSON.stringify({ currency: "GBP" }),
115+
contentType: "application/json"
104116
}
105117
].map(createMockedKeyValue);
106118

@@ -579,6 +591,33 @@ describe("load", function () {
579591
}).to.throw("Invalid separator '%'. Supported values: '.', ',', ';', '-', '_', '__', '/', ':'.");
580592
});
581593

594+
it("should call constructConfigurationObject twice without throwing for JSON content-type keys with overlapping hierarchy", async () => {
595+
const connectionString = createMockedConnectionString();
596+
const settings = await load(connectionString, {
597+
selectors: [{
598+
keyFilter: "app6_*"
599+
}],
600+
trimKeyPrefixes: ["app6_"]
601+
});
602+
expect(settings).not.undefined;
603+
604+
// First call should succeed
605+
const data1 = settings.constructConfigurationObject({ separator: "." });
606+
expect(data1).not.undefined;
607+
expect(data1.markets.gb.url).eq("https://example.com/gb");
608+
expect(data1.markets.dk.url).eq("https://example.com/dk");
609+
expect(data1.markets.markets.dk.currency).eq("DKK");
610+
expect(data1.markets.markets.gb.currency).eq("GBP");
611+
612+
// Second call should also succeed (idempotent)
613+
const data2 = settings.constructConfigurationObject({ separator: "." });
614+
expect(data2).not.undefined;
615+
expect(data2.markets.gb.url).eq("https://example.com/gb");
616+
expect(data2.markets.dk.url).eq("https://example.com/dk");
617+
expect(data2.markets.markets.dk.currency).eq("DKK");
618+
expect(data2.markets.markets.gb.currency).eq("GBP");
619+
});
620+
582621
it("should load key values from snapshot", async () => {
583622
const snapshotName = "Test";
584623
const snapshotResponses = new Map([

0 commit comments

Comments
 (0)