From 8efe10e46aa6b024b3391a95ddeabc9c506e8c85 Mon Sep 17 00:00:00 2001 From: intech Date: Sat, 10 Oct 2026 17:54:35 +0400 Subject: [PATCH] Avoid allocating duplicate-key tracking per message in fromJson Every JSON object decoded by fromJson allocated a Set of seen fields and a Map of seen oneofs, only to detect duplicates. A field can only be set twice in one object through two distinct keys, its proto name and its JSON name, and most messages set at most one oneof. A field whose JSON name differs from its proto name now looks up its other key in the object; the set is only created when an object holds both keys of some field. The first oneof that is set is kept in two locals, and the map is only created when a second oneof is set. The order in which keys are read and errors are raised is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JCd4TPygQp4GPxMsTMm2Bi --- packages/protobuf-test/src/json.test.ts | 56 +++++++++++++++++++++++++ packages/protobuf/src/from-json.ts | 49 ++++++++++++++++------ 2 files changed, 93 insertions(+), 12 deletions(-) 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); }