bufbuild / protobuf-es

Protocol Buffers for ECMAScript. The only JavaScript Protobuf library that is fully-compliant with Protobuf conformance tests.
Apache License 2.0
1.16k stars 69 forks source link

Fix assignability of wrong message type in create #972

Closed timostamm closed 2 months ago

timostamm commented 2 months ago

The function create() takes an optional initializer object as the second argument. In the object, all fields of the message are optional.

Because of a confusion in the type, it also accepts a message of a different type, as long as the type is assignable (any non-optional property in the target is also present in the source with the same name and type). This results in broken behavior with field presence, where passing a message with unset fields will populate fields in the target with the default value.

Here is an example that demonstrates the issue:

syntax = "proto2";

message A {
  optional string str = 1;
}

message B {
  optional string str = 1;
}
import { create } from "@bufbuild/protobuf";
import { ASchema, BSchema } from "./gen/example_pb";

const a = create(ASchema);
isFieldSet(a, ASchema.str); // false
a.str; // ""

const b = create(BSchema, a); // Type A is assignable to the init argument for B.
isFieldSet(b, BSchema.str); // true 💥
b.str; // ""

We thought that we prevent this situation with the $typeName property, but there is a bug in the internal MessageInit type that bypasses the type branding.

This PR fixes the bug. This is a breaking change because it introduces a compile-time error for a situation that previously compiled without complaints. Since the situation should be very rare, and most likely a bug in itself, we decided to make this change and release it as a bugfix.