Fix validation for FieldMask (de-)serialization to/from JSON (#1323)

Co-authored-by: Dan Hudlow <hudlow@buf.build>
Co-authored-by: Timo Stamm <ts@timostamm.de>

Author: 3 people

packages/bundle-size/README.md

@@ -16,10 +16,10 @@ usually do. We repeat this for an increasing number of files.
 
 | code generator      | files | bundle size |  minified | compressed |
 | ------------------- | ----: | ----------: | --------: | ---------: |
-| Protobuf-ES         |     1 |   133,010 b |  68,782 b |   15,811 b |
+| Protobuf-ES         |     1 |   133,010 b |  68,782 b |   15,803 b |
 | Protobuf-ES         |     4 |   135,199 b |  70,289 b |   16,433 b |
 | Protobuf-ES         |     8 |   137,961 b |  72,060 b |   16,996 b |
-| Protobuf-ES         |    16 |   148,411 b |  80,041 b |   19,301 b |
+| Protobuf-ES         |    16 |   148,411 b |  80,041 b |   19,349 b |
 | Protobuf-ES         |    32 |   176,202 b | 102,059 b |   24,783 b |
 | protobuf-javascript |     1 |   304,887 b | 234,842 b |   35,819 b |
 | protobuf-javascript |     4 |   330,904 b | 249,814 b |   37,197 b |

packages/bundle-size/chart.svg

@@ -17,6 +17,7 @@ import * as assert from "node:assert";
 import {
   safeObjectProperty,
   protoCamelCase,
+  protoSnakeCase,
   qualifiedName,
 } from "@bufbuild/protobuf/reflect";
 import type { DescFile } from "@bufbuild/protobuf";
@@ -155,4 +156,21 @@ void suite("protoCamelCase", () => {
       assert.strictEqual(protoCamelCase(name), protocJsonName);
     });
   }
+  void suite("irreversible", () => {
+    testIrreversible("foo_", "foo");
+    testIrreversible("foo__", "foo");
+    testIrreversible("foo__bar", "fooBar");
+    testIrreversible("foo_1", "foo1");
+    function testIrreversible(snakeCase: string, expectedCamelCase: string) {
+      void test(`"${snakeCase}" is irreversible`, () => {
+        assert.strictEqual(protoCamelCase(snakeCase), expectedCamelCase);
+        assert.notStrictEqual(protoSnakeCase(expectedCamelCase), snakeCase);
+      });
+    }
+  });
+});
+
+void test("protoSnakeCase", () => {
+  assert.strictEqual(protoSnakeCase("fooBar"), "foo_bar");
+  assert.strictEqual(protoSnakeCase("fooUTF8"), "foo_u_t_f8");
 });

packages/protobuf-test/src/reflect/names.test.ts

@@ -32,6 +32,7 @@ import type {
 import { reflect } from "./reflect/reflect.js";
 import { FieldError, isFieldError } from "./reflect/error.js";
 import { formatVal } from "./reflect/reflect-check.js";
+import { protoSnakeCase } from "./reflect/names.js";
 import type { ScalarValue } from "./reflect/scalar.js";
 import type { EnumJsonType, EnumShape, MessageShape } from "./types.js";
 import { base64Decode } from "./wire/base64-encoding.js";
@@ -755,16 +756,14 @@ function fieldMaskFromJson(fieldMask: FieldMask, json: JsonValue) {
   if (json === "") {
     return;
   }
-  function camelToSnake(str: string) {
-    if (str.includes("_")) {
+  fieldMask.paths = json.split(",").map((path) => {
+    if (path.includes("_")) {
       throw new Error(
         `cannot decode message ${fieldMask.$typeName} from JSON: path names must be lowerCamelCase`,
       );
     }
-    const sc = str.replace(/[A-Z]/g, (letter) => "_" + letter.toLowerCase());
-    return sc[0] === "_" ? sc.substring(1) : sc;
-  }
-  fieldMask.paths = json.split(",").map(camelToSnake);
+    return protoSnakeCase(path);
+  });
 }
 
 function structFromJson(struct: Struct, json: JsonValue) {

packages/protobuf/src/from-json.ts

@@ -47,6 +47,8 @@ export function qualifiedName(desc: AnyDesc) {
  * used by protoc to convert a field name to a JSON name.
  *
  * See https://protobuf.com/docs/language-spec#default-json-names
+ *
+ * The function protoSnakeCase provides the reverse.
  */
 export function protoCamelCase(snakeCase: string): string {
   let capNext = false;
@@ -82,6 +84,19 @@ export function protoCamelCase(snakeCase: string): string {
   return b.join("");
 }
 
+/**
+ * Converts protoCamelCase to snake_case.
+ *
+ * This function is the reverse of function protoCamelCase. Note that some names
+ * are not reversible - for example, "foo__bar" -> "fooBar" -> "foo_bar".
+ */
+export function protoSnakeCase(lowerCamelCase: string): string {
+  return lowerCamelCase.replace(
+    /[A-Z]/g,
+    (letter) => "_" + letter.toLowerCase(),
+  );
+}
+
 /**
  * Names that cannot be used for object properties because they are reserved
  * by built-in JavaScript properties.

packages/protobuf/src/reflect/names.ts

@@ -20,7 +20,7 @@ import {
   ScalarType,
 } from "./descriptors.js";
 import type { JsonObject, JsonValue } from "./json-value.js";
-import { protoCamelCase } from "./reflect/names.js";
+import { protoCamelCase, protoSnakeCase } from "./reflect/names.js";
 import { reflect } from "./reflect/reflect.js";
 import type { Registry } from "./registry.js";
 import type {
@@ -488,11 +488,9 @@ function durationToJson(val: Duration) {
 function fieldMaskToJson(val: FieldMask) {
   return val.paths
     .map((p) => {
-      if (p.match(/_[0-9]?_/g) || p.match(/[A-Z]/g)) {
+      if (protoSnakeCase(protoCamelCase(p)) !== p) {
         throw new Error(
-          `cannot encode message ${val.$typeName} to JSON: lowerCamelCase of path name "` +
-            p +
-            '" is irreversible',
+          `cannot encode message ${val.$typeName} to JSON: lowerCamelCase of path name "${p}" is irreversible`,
         );
       }
       return protoCamelCase(p);