diff --git a/packages/protobuf-test/src/json.test.ts b/packages/protobuf-test/src/json.test.ts index 4dc76bf89..c12eeb32a 100644 --- a/packages/protobuf-test/src/json.test.ts +++ b/packages/protobuf-test/src/json.test.ts @@ -1068,6 +1068,62 @@ void suite("parsing duplicate keys", () => { { message: /oneof_int32_field from JSON: set multiple times/ }, ); }); + void test("rejects a duplicate field when the JSON name comes first", () => { + assert.throws( + () => + fromJson(proto3_ts.Proto3MessageSchema, { + singularStringField: "a", + singular_string_field: "b", + }), + { message: /singular_string_field from JSON: set multiple times/ }, + ); + }); + void test("reports an invalid first occurrence before the duplicate", () => { + // The first occurrence is decoded before the duplicate is rejected. + assert.throws( + () => + fromJson(proto3_ts.Proto3MessageSchema, { + singularStringField: 123, + singular_string_field: "b", + }), + (err: unknown) => + err instanceof Error && + /singular_string_field from JSON/.test(err.message) && + !/set multiple times/.test(err.message), + ); + }); + void test("accepts one member in each of several oneofs", () => { + const msg = fromJson(OneofMessageSchema, { + e: "ONEOF_ENUM_A", + value: 1, + foo: { name: "a" }, + }); + assert.strictEqual(msg.enum.case, "e"); + assert.strictEqual(msg.scalar.case, "value"); + assert.strictEqual(msg.message.case, "foo"); + }); + void test("rejects two members of a oneof that is not the first one set", () => { + // A conflict is detected in any oneof, not only in the first one set. + assert.throws( + () => + fromJson(OneofMessageSchema, { + e: "ONEOF_ENUM_A", + value: 1, + error: "x", + }), + { message: /oneof set multiple times by value and error/ }, + ); + assert.throws( + () => + fromJson(OneofMessageSchema, { + value: 1, + foo: { name: "a" }, + e: "ONEOF_ENUM_A", + bar: { a: 1 }, + }), + { message: /oneof set multiple times by foo and bar/ }, + ); + }); void suite("merge", () => { void test("rejects duplicate keys within a single document", () => { const target = create(proto3_ts.Proto3MessageSchema); diff --git a/packages/protobuf/src/from-json.ts b/packages/protobuf/src/from-json.ts index 4bcf764b1..ed39428be 100644 --- a/packages/protobuf/src/from-json.ts +++ b/packages/protobuf/src/from-json.ts @@ -279,6 +279,9 @@ interface CompiledFieldEntry { // A oneof member that is a scalar field skips JSON null, see conformance // test Required.Proto3.JsonInput.OneofFieldNull{First,Second}. oneofScalarNullSkip: boolean; + // Whether the JSON name differs from the proto name. Only then can a JSON + // object set the field twice. + aliased: boolean; } function compileMessage(desc: DescMessage): CompiledJsonReader { @@ -313,8 +316,12 @@ function compileMessage(desc: DescMessage): CompiledJsonReader { `cannot decode ${descString} from JSON: ${formatVal(json)}`, ); } - const oneofSeen = new Map(); - const fieldSeen = new Set(); + // Only allocated when needed: most objects never set a field twice, and + // most messages set at most one oneof. + let fieldSeen: Set | undefined; + let firstOneof: DescOneof | undefined; + let firstOneofField: DescField | undefined; + let oneofSeen: Map | undefined; const jsonKeys = Object.keys(json); for (let i = 0; i < jsonKeys.length; i++) { const jsonKey = jsonKeys[i]; @@ -322,25 +329,42 @@ function compileMessage(desc: DescMessage): CompiledJsonReader { const entry = fieldsByJsonKey.get(jsonKey); if (entry !== undefined) { const field = entry.field; - if (fieldSeen.has(field)) { - // The same field may be set by its proto name and its JSON name, or by - // a duplicate or unicode-escaped key that JSON.parse already collapsed. - // Checked before the null-skip below so that a null entry still counts. - throw new FieldError(field, "set multiple times"); + if ( + entry.aliased && + Object.prototype.hasOwnProperty.call( + json, + jsonKey === field.name ? field.jsonName : field.name, + ) + ) { + // The object holds both keys of this field; if both set it, the + // second one is an error. Checked before the null-skip below so + // that a null entry still counts. + if (fieldSeen?.has(field)) { + throw new FieldError(field, "set multiple times"); + } + fieldSeen ??= new Set(); + fieldSeen.add(field); } - fieldSeen.add(field); if (entry.oneofScalarNullSkip && jsonValue === null) { continue; } - if (entry.oneof) { - const seen = oneofSeen.get(entry.oneof); + const oneof = entry.oneof; + if (oneof !== undefined) { + const seen = + oneof === firstOneof ? firstOneofField : oneofSeen?.get(oneof); if (seen !== undefined) { throw new FieldError( - entry.oneof, + oneof, `oneof set multiple times by ${seen.name} and ${field.name}`, ); } - oneofSeen.set(entry.oneof, field); + if (firstOneof === undefined) { + firstOneof = oneof; + firstOneofField = field; + } else { + oneofSeen ??= new Map(); + oneofSeen.set(oneof, field); + } } entry.read(message, jsonValue, ctx); } else { @@ -378,6 +402,7 @@ function compileMessage(desc: DescMessage): CompiledJsonReader { oneof: field.oneof, oneofScalarNullSkip: field.oneof !== undefined && field.fieldKind == "scalar", + aliased: field.jsonName !== field.name, }; fieldsByJsonKey.set(field.name, entry).set(field.jsonName, entry); }