FR: Restrict expected case type (camel vs snake) when parsing JSON

[davidfiala]

I didn't see any obvious way to restrict the expected type of proto field naming when parsing from JSON in javascript/typescript buf.

JsonWriteOptions allows you to specify an output type for fields as snake_case or camelCase.

However the corresponding JsonReadOptions lacks the ability to restrict to a specific casing format such as snake or camel.

This creates an ambiguity. And arguably a security risk. For instance, a code reviewer may not realize that a sensitive property was overridden further down in a JSON config.

In practice, it appears that whichever field is set last is the winner. Example code below.

From https://protobuf.dev/programming-guides/json/

Use proto field name instead of lowerCamelCase name: By default the protobuf JSON printer should convert the field name to lowerCamelCase and use that as the JSON name. An implementation may provide an option to use proto field name as the JSON name instead. Protobuf JSON parsers are required to accept both the converted lowerCamelCase name and the proto field name.

In a sense, protobuf-es is perfectly compliant with the behavior quoted above, if we accept the ambiguity. But I believe it would be safer to provide a restricted option, which wouldn't necessarily need to break existing users. Such an option remains compliant with the ambiguous quote above if the default remains to accept whichever field comes last during parsing.

// Strawman proposal:
export interface JsonReadOptions {
  // ...
  mandatoryProtoFieldName?: 'camel' | 'snake' | 'accept-last-parsed'; // default to 'accept-last-parsed' to make the behavior explicit
}

I'd love to get the team's thoughts. Thank you.

it('spaces', () => {
    const str = JSON.stringify({
      field_name_with_spaces: 'hello world',
    });

    const msg = fromJsonString(cfg.UnitTestConfigSchema, str);
    expect(msg.fieldNameWithSpaces).toBe('hello world');
  });

  it('camel', () => {
    const str = JSON.stringify({
      fieldNameWithSpaces: 'hello world',
    });

    const msg = fromJsonString(cfg.UnitTestConfigSchema, str);
    expect(msg.fieldNameWithSpaces).toBe('hello world');
  });

  it('ambiguous conflict order #1', () => {
    const str = JSON.stringify({
      field_name_with_spaces: 'option1',
      fieldNameWithSpaces: 'option2',
    });

    const msg = fromJsonString(cfg.UnitTestConfigSchema, str, {});
    expect(msg.fieldNameWithSpaces).toBe('fail on purpose'); // winner is fieldNameWithSpaces
  });

  it('ambiguous conflict order #2', () => {
    const str = JSON.stringify({
      fieldNameWithSpaces: 'option2',
      field_name_with_spaces: 'option1',
    });

    const msg = fromJsonString(cfg.UnitTestConfigSchema, str, {});
    expect(msg.fieldNameWithSpaces).toBe('fail on purpose'); // winner is field_name_with_spaces
  });

[timostamm]

Thanks for raising the issue, David.

We implement the options and the behavior because the spec requires it, but we don't necessarily agree with them. In my opinion, there should be only one way to serialize to JSON, and zero values should always be included. OpenAPI or TypeScript types generated from a Protobuf schema would be easier to use.

An opt-in for more strict parsing is a great idea. I think it's important to realize that JSON itself allows the same key to appear multiple times:

JSON.parse(`{"a": 1, "a": 2}`); // {a: 2}