Skip to content

Commit

Permalink
V2: Relax validation in BinaryWriter (#877)
Browse files Browse the repository at this point in the history
  • Loading branch information
@timostamm
timostamm authored Jun 4, 2024
1 parent 2e4a5f0 commit 11c5db0
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 42 deletions.
10 changes: 5 additions & 5 deletions packages/bundle-size/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ usually do. We repeat this for an increasing number of files.
<!--- TABLE-START -->
| code generator | files | bundle size | minified | compressed |
|-----------------|----------|------------------------:|-----------------------:|-------------------:|
| protobuf-es | 1 | 123,019 b | 63,933 b | 14,930 b |
| protobuf-es | 4 | 125,214 b | 65,443 b | 15,592 b |
| protobuf-es | 8 | 127,992 b | 67,214 b | 16,104 b |
| protobuf-es | 16 | 138,500 b | 75,195 b | 18,432 b |
| protobuf-es | 32 | 166,395 b | 97,210 b | 23,848 b |
| protobuf-es | 1 | 123,350 b | 64,136 b | 14,970 b |
| protobuf-es | 4 | 125,545 b | 65,646 b | 15,662 b |
| protobuf-es | 8 | 128,323 b | 67,417 b | 16,196 b |
| protobuf-es | 16 | 138,831 b | 75,398 b | 18,504 b |
| protobuf-es | 32 | 166,726 b | 97,413 b | 23,953 b |
| protobuf-javascript | 1 | 339,613 b | 255,820 b | 42,481 b |
| protobuf-javascript | 4 | 366,281 b | 271,092 b | 43,912 b |
| protobuf-javascript | 8 | 388,324 b | 283,409 b | 45,038 b |
Expand Down
12 changes: 6 additions & 6 deletions packages/bundle-size/chart.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
79 changes: 61 additions & 18 deletions packages/protobuf-test/src/wire/binary-encoding.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe("BinaryWriter", () => {
expect(user.firstName).toBe("Homer");
expect(user.active).toBe(true);
});
describe("float32", () => {
describe("float32()", () => {
it.each([
1024,
3.142,
Expand All @@ -41,13 +41,20 @@ describe("BinaryWriter", () => {
Number.NEGATIVE_INFINITY,
Number.NaN,
])("should encode %s", (val) => {
expect(new BinaryWriter().float(val).finish().length).toBeGreaterThan(0);
const bytes = new BinaryWriter().float(val).finish();
expect(bytes.length).toBeGreaterThan(0);
// @ts-expect-error test string
const bytesStr = new BinaryWriter().float(val.toString()).finish();
expect(bytesStr.length).toBeGreaterThan(0);
expect(bytesStr).toStrictEqual(bytes);
});
it.each([
{ val: "123", err: /^invalid float32: string/ },
{ val: true, err: /^invalid float32: bool/ },
{ val: null, err: "invalid float32: object" },
{ val: new Date(), err: "invalid float32: object" },
{ val: undefined, err: "invalid float32: undefined" },
{ val: true, err: "invalid float32: bool" },
])("should error for wrong type $val", ({ val, err }) => {
// @ts-expect-error TS2345
// @ts-expect-error test wrong type
expect(() => new BinaryWriter().float(val)).toThrow(err);
});
it.each([Number.MAX_VALUE, -Number.MAX_VALUE])(
Expand All @@ -56,40 +63,76 @@ describe("BinaryWriter", () => {
expect(() => new BinaryWriter().float(val)).toThrow(
/^invalid float32: .*/,
);
// @ts-expect-error test string
expect(() => new BinaryWriter().float(val.toString())).toThrow(
/^invalid float32: .*/,
);
},
);
});
describe("int32", () => {
// sfixed32, sint32, and int32 are signed 32-bit integers, just with different encoding
describe.each(["sfixed32", "sint32", "int32"] as const)("%s()", (type) => {
it.each([-0x80000000, 1024, 0x7fffffff])("should encode %s", (val) => {
expect(new BinaryWriter().int32(val).finish().length).toBeGreaterThan(0);
const bytes = new BinaryWriter()[type](val).finish();
expect(bytes.length).toBeGreaterThan(0);
// @ts-expect-error test string
const bytesStr = new BinaryWriter()[type](val.toString()).finish();
expect(bytesStr.length).toBeGreaterThan(0);
expect(bytesStr).toStrictEqual(bytes);
});
it.each([
{ val: "123", err: /^invalid int32: string/ },
{ val: true, err: /^invalid int32: bool/ },
{ val: null, err: "invalid int32: object" },
{ val: new Date(), err: "invalid int32: object" },
{ val: undefined, err: "invalid int32: undefined" },
{ val: true, err: "invalid int32: bool" },
])("should error for wrong type $val", ({ val, err }) => {
// @ts-expect-error TS2345
expect(() => new BinaryWriter().int32(val)).toThrow(err);
expect(() => new BinaryWriter()[type](val)).toThrow(err);
});
it.each([0x7fffffff + 1, -0x80000000 - 1])(
it.each([0x7fffffff + 1, -0x80000000 - 1, 3.142])(
"should error for value out of range %s",
(val) => {
expect(() => new BinaryWriter().int32(val)).toThrow(
expect(() => new BinaryWriter()[type](val)).toThrow(
/^invalid int32: .*/,
);
// @ts-expect-error test string
expect(() => new BinaryWriter()[type](val.toString())).toThrow(
/^invalid int32: .*/,
);
},
);
});
describe("uint32", () => {
// fixed32 and uint32 are unsigned 32-bit integers, just with different encoding
describe.each(["fixed32", "uint32"] as const)("%s()", (type) => {
it.each([0, 1024, 0xffffffff])("should encode %s", (val) => {
expect(new BinaryWriter().uint32(val).finish().length).toBeGreaterThan(0);
const bytes = new BinaryWriter()[type](val).finish();
expect(bytes.length).toBeGreaterThan(0);
// @ts-expect-error test string
const bytesStr = new BinaryWriter()[type](val.toString()).finish();
expect(bytesStr.length).toBeGreaterThan(0);
expect(bytesStr).toStrictEqual(bytes);
});
it.each([
{ val: "123", err: /^invalid uint32: string/ },
{ val: true, err: /^invalid uint32: bool/ },
])("should error for wrong type$val", ({ val, err }) => {
{ val: null, err: `invalid uint32: object` },
{ val: new Date(), err: `invalid uint32: object` },
{ val: undefined, err: `invalid uint32: undefined` },
{ val: true, err: `invalid uint32: bool` },
])("should error for wrong type $val", ({ val, err }) => {
// @ts-expect-error TS2345
expect(() => new BinaryWriter().uint32(val)).toThrow(err);
expect(() => new BinaryWriter()[type](val)).toThrow(err);
});
it.each([0xffffffff + 1, -1, 3.142])(
"should error for value out of range %s",
(val) => {
expect(() => new BinaryWriter()[type](val)).toThrow(
/^invalid uint32: .*/,
);
// @ts-expect-error test string
expect(() => new BinaryWriter()[type](val.toString())).toThrow(
/^invalid uint32: .*/,
);
},
);
});
});

Expand Down
8 changes: 4 additions & 4 deletions packages/protobuf/src/reflect/guard.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import type {
} from "./reflect-types.js";
import { unsafeLocal } from "./unsafe.js";
import type { DescField, DescMessage } from "../descriptors.js";
import { isMessage } from "../is-message.js";

export function isObject(arg: unknown): arg is Record<string, unknown> {
return arg !== null && typeof arg == "object" && !Array.isArray(arg);
Expand Down Expand Up @@ -102,8 +101,9 @@ export function isReflectMessage(
return (
isObject(arg) &&
unsafeLocal in arg &&
"message" in arg &&
isMessage(arg.message) &&
(messageDesc === undefined || arg.message.$typeName == messageDesc.typeName)
"desc" in arg &&
isObject(arg.desc) &&
arg.desc.kind === "message" &&
(messageDesc === undefined || arg.desc.typeName == messageDesc.typeName)
);
}
44 changes: 35 additions & 9 deletions packages/protobuf/src/wire/binary-encoding.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,29 +568,55 @@ export class BinaryReader {
}

/**
* Assert a valid signed protobuf 32-bit integer.
* Assert a valid signed protobuf 32-bit integer as a number or string.
*/
function assertInt32(arg: unknown): asserts arg is number {
if (typeof arg !== "number") throw new Error("invalid int32: " + typeof arg);
if (!Number.isInteger(arg) || arg > INT32_MAX || arg < INT32_MIN)
if (typeof arg == "string") {
arg = Number(arg);
} else if (typeof arg != "number") {
throw new Error("invalid int32: " + typeof arg);
}
if (
!Number.isInteger(arg) ||
(arg as number) > INT32_MAX ||
(arg as number) < INT32_MIN
)
throw new Error("invalid int32: " + arg);
}

/**
* Assert a valid unsigned protobuf 32-bit integer.
* Assert a valid unsigned protobuf 32-bit integer as a number or string.
*/
function assertUInt32(arg: unknown): asserts arg is number {
if (typeof arg !== "number") throw new Error("invalid uint32: " + typeof arg);
if (!Number.isInteger(arg) || arg > UINT32_MAX || arg < 0)
if (typeof arg == "string") {
arg = Number(arg);
} else if (typeof arg != "number") {
throw new Error("invalid uint32: " + typeof arg);
}
if (
!Number.isInteger(arg) ||
(arg as number) > UINT32_MAX ||
(arg as number) < 0
)
throw new Error("invalid uint32: " + arg);
}

/**
* Assert a valid protobuf float value.
* Assert a valid protobuf float value as a number or string.
*/
function assertFloat32(arg: unknown): asserts arg is number {
if (typeof arg !== "number")
if (typeof arg == "string") {
const o = arg;
arg = Number(arg);
if (isNaN(arg as number) && o !== "NaN") {
throw new Error("invalid float32: " + o);
}
} else if (typeof arg != "number") {
throw new Error("invalid float32: " + typeof arg);
if (Number.isFinite(arg) && (arg > FLOAT32_MAX || arg < FLOAT32_MIN))
}
if (
Number.isFinite(arg) &&
((arg as number) > FLOAT32_MAX || (arg as number) < FLOAT32_MIN)
)
throw new Error("invalid float32: " + arg);
}

0 comments on commit 11c5db0

Please sign in to comment.