Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/common-falcons-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hunkdiff": patch
---

Reject review event frames whose event ID names a different generation or revision than the frame's own fields.
56 changes: 56 additions & 0 deletions src/session/reviewEventProtocol.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,62 @@ describe("review event envelope parsing", () => {
expect(parseReviewEventBegin(begin)).toEqual(begin);
});

test("accepts a well-formed single frame", () => {
const single = {
eventId: begin.eventId,
generation: begin.generation,
stateRevision: begin.stateRevision,
payload: { hello: "world" },
};

expect(parseReviewEventFrame(single)).toEqual(single);
});

test("refuses a frame whose id names another generation or revision", () => {
const otherGeneration = "generation:test:4";
const single = {
eventId: begin.eventId,
generation: begin.generation,
stateRevision: begin.stateRevision,
payload: {},
};

expect(parseReviewEventFrame({ ...single, generation: otherGeneration })).toBeUndefined();
expect(parseReviewEventFrame({ ...single, stateRevision: 9 })).toBeUndefined();
expect(parseReviewEventBegin({ ...begin, generation: otherGeneration })).toBeUndefined();
expect(parseReviewEventBegin({ ...begin, stateRevision: 9 })).toBeUndefined();

const chunk: ReviewEventChunkV1 = {
eventId: begin.eventId,
generation: otherGeneration,
offset: 0,
byteLength: 10,
encoding: "base64",
data: "",
contentDigest: begin.contentDigest,
contentSize: 10,
eof: true,
};
expect(parseReviewEventChunk(chunk)).toBeUndefined();
expect(parseReviewEventChunk({ ...chunk, generation: begin.generation })).toEqual({
...chunk,
generation: begin.generation,
});

const end: ReviewEventEndV1 = {
eventId: begin.eventId,
generation: otherGeneration,
contentSize: 10,
contentDigest: begin.contentDigest,
chunkCount: 1,
};
expect(parseReviewEventEnd(end)).toBeUndefined();
expect(parseReviewEventEnd({ ...end, generation: begin.generation })).toEqual({
...end,
generation: begin.generation,
});
});

test("refuses an extra field, an unknown encoding, and a non-canonical digest", () => {
expect(parseReviewEventBegin({ ...begin, extra: 1 })).toBeUndefined();
expect(parseReviewEventBegin({ ...begin, encoding: "hex" })).toBeUndefined();
Expand Down
22 changes: 13 additions & 9 deletions src/session/reviewEventProtocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,6 @@ export function parseReviewEventId(
return { type, address: { generation: match[2]!, stateRevision } };
}

/** Whether one value could be an event id a client is echoing back at us. */
export function isReviewEventId(value: unknown): value is string {
return parseReviewEventId(value) !== undefined;
}

// -- Frame payloads ---------------------------------------------------------------------

/** One whole event, small enough to send in a single frame. */
Expand Down Expand Up @@ -208,13 +203,22 @@ function isChunkCount(value: unknown): value is number {
return isCount(value) && (value as number) >= 1 && (value as number) <= MAX_REVIEW_EVENT_CHUNKS;
}

/** Whether one frame's id names the same publication position its own fields state. */
function eventIdMatchesFrame(
record: Record<string, unknown>,
fields: readonly (keyof ReviewPublicationAddress)[],
): boolean {
const parsed = parseReviewEventId(record.eventId);
return parsed !== undefined && fields.every((field) => parsed.address[field] === record[field]);
}

/** Parse one single-frame event body. */
export function parseReviewEventFrame(value: unknown): ReviewEventFrameV1 | undefined {
const record = asRecord(value);
if (
!record ||
!hasExactKeys(record, ["eventId", "generation", "stateRevision", "payload"]) ||
!isReviewEventId(record.eventId) ||
!eventIdMatchesFrame(record, ["generation", "stateRevision"]) ||
parseReviewGeneration(record.generation) === undefined ||
!isCount(record.stateRevision)
) {
Expand All @@ -237,7 +241,7 @@ export function parseReviewEventBegin(value: unknown): ReviewEventBeginV1 | unde
"contentDigest",
"chunkCount",
]) ||
!isReviewEventId(record.eventId) ||
!eventIdMatchesFrame(record, ["generation", "stateRevision"]) ||
parseReviewGeneration(record.generation) === undefined ||
!isCount(record.stateRevision) ||
record.encoding !== "base64" ||
Expand Down Expand Up @@ -266,7 +270,7 @@ export function parseReviewEventChunk(value: unknown): ReviewEventChunkV1 | unde
"contentSize",
"eof",
]) ||
!isReviewEventId(record.eventId) ||
!eventIdMatchesFrame(record, ["generation"]) ||
parseReviewGeneration(record.generation) === undefined ||
!isCount(record.offset) ||
!isCount(record.byteLength) ||
Expand Down Expand Up @@ -294,7 +298,7 @@ export function parseReviewEventEnd(value: unknown): ReviewEventEndV1 | undefine
"contentDigest",
"chunkCount",
]) ||
!isReviewEventId(record.eventId) ||
!eventIdMatchesFrame(record, ["generation"]) ||
parseReviewGeneration(record.generation) === undefined ||
!isPayloadSize(record.contentSize) ||
!isReviewSha256Digest(record.contentDigest) ||
Expand Down
Loading