Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
19 changes: 16 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
47 changes: 47 additions & 0 deletions test/additionalProperties.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' })
})
24 changes: 24 additions & 0 deletions test/patternProperties.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' })
})