diff --git a/index.js b/index.js index 96ca8ff0..3789dd15 100644 --- a/index.js +++ b/index.js @@ -386,9 +386,22 @@ function buildInnerObject (context, location, objVar) { const localUid = context.uid++ let addComma = '' - if (requiredProperties.length > 0) { - // If we have required properties, we know that at least one property will be serialized. - // We can avoid the runtime check for the comma. + // The skip-leading-comma optimization is only safe when at least one + // declared property is also required: that guarantees the first emitted + // entry has no preceding separator. `propertiesKeys` is sorted with + // required keys first, so `requiredProperties.includes(propertiesKeys[0])` + // is the cheapest way to express the condition. + // + // Falls through to the generic runtime-`addComma` branch when: + // * `requiredProperties` is empty (no anchor needed), or + // * `propertiesKeys` is empty (record-style schema with only + // `additionalProperties`), or + // * `required` only names keys that are not in `properties` (so no + // declared property is guaranteed to emit first). + // Otherwise the additionalProperties / patternProperties serializer + // would write a separator with no preceding property, producing + // `{ ,"k":v,... }`. + if (propertiesKeys.length > 0 && requiredProperties.includes(propertiesKeys[0])) { // The first property is required, so we don't need a comma. // For the subsequent properties, we can blindly add a comma. diff --git a/test/additionalProperties.test.js b/test/additionalProperties.test.js index a42e176b..4a44a8c3 100644 --- a/test/additionalProperties.test.js +++ b/test/additionalProperties.test.js @@ -330,3 +330,50 @@ test('function and symbol references are not serialized as undefined', (t) => { const obj = { str: 'x', test: 'test', meth: () => 'x', sym: Symbol('x') } t.assert.equal(stringify(obj), '{"str":"x","test":"test"}') }) + +test('required + additionalProperties without declared properties produces valid JSON', (t) => { + // Regression: when `required` is non-empty but `properties` is absent + // (e.g. a record-style schema with only `additionalProperties`), the + // serializer used to emit a stray leading comma: + // {"obj":{,"a":1,"b":2}} + // because the "skip first comma" optimization assumed a declared + // property would anchor it. With no `properties`, the additionalProperties + // branch would write the separator before its first entry. + t.plan(2) + const stringify = build({ + type: 'object', + properties: { + obj: { + type: 'object', + propertyNames: { type: 'string', enum: ['a', 'b'] }, + additionalProperties: { type: 'number' }, + required: ['a', 'b'] + } + } + }) + + const out = stringify({ obj: { a: 1, b: 2 } }) + t.assert.equal(out, '{"obj":{"a":1,"b":2}}') + t.assert.deepStrictEqual(JSON.parse(out), { obj: { a: 1, b: 2 } }) +}) + +test('required key not in properties + additionalProperties produces valid JSON', (t) => { + // Regression: declared `properties` is non-empty but `required` only lists + // keys that are not in `properties`. After sorting, propertiesKeys[0] is + // not required, so the "first declared property anchors the comma" premise + // does not hold. The serializer used to emit '{,"str":"x"}' for input + // missing the (non-required) declared `num`. + t.plan(2) + const stringify = build({ + type: 'object', + properties: { + num: { type: 'number' } + }, + additionalProperties: true, + required: ['str'] + }) + + const out = stringify({ str: 'x' }) + t.assert.equal(out, '{"str":"x"}') + t.assert.deepStrictEqual(JSON.parse(out), { str: 'x' }) +}) diff --git a/test/patternProperties.test.js b/test/patternProperties.test.js index d80a7f1b..13633d4f 100644 --- a/test/patternProperties.test.js +++ b/test/patternProperties.test.js @@ -166,3 +166,27 @@ test('patternProperties - fail on invalid regex, handled by ajv', (t) => { } }), new Error('schema is invalid: data/patternProperties must match format "regex"')) }) + +test('required key not in properties + patternProperties produces valid JSON', (t) => { + // Regression: symmetric case to the additionalProperties variant. `required` + // names a key not in `properties`, so after sorting propertiesKeys[0] is not + // required and the "first declared property anchors the comma" premise does + // not hold. The patternProperties branch shares the same code path, so the + // same stray-leading-comma bug would surface here without the tightened + // guard in `index.js`. + t.plan(2) + const stringify = build({ + type: 'object', + properties: { + num: { type: 'number' } + }, + patternProperties: { + '^s_': { type: 'string' } + }, + required: ['s_x'] + }) + + const out = stringify({ s_x: 'x' }) + t.assert.equal(out, '{"s_x":"x"}') + t.assert.deepStrictEqual(JSON.parse(out), { s_x: 'x' }) +})