Skip to content

Commit

Permalink
fix casting to StableId bug in identifier encoding (#21507)
Browse files Browse the repository at this point in the history
## Description

This PR fixes a bug where we are incorrectly casting a cursor value to
`StableId`.
  • Loading branch information
daesunp authored Jun 18, 2024
1 parent 40e1942 commit cdcf65c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
type ITreeCursorSynchronous,
type TreeNodeSchemaIdentifier,
forEachField,
type Value,
} from "../../../core/index.js";
import { brand, fail } from "../../../util/index.js";

Expand All @@ -23,7 +24,7 @@ import {
encodeValue,
} from "./compressedEncode.js";
import type { EncodedChunkShape, EncodedFieldShape, EncodedValueShape } from "./format.js";
import type { StableId } from "@fluidframework/id-compressor";
import { isStableId } from "@fluidframework/id-compressor/internal";

export class NodeShape extends Shape<EncodedChunkShape> implements NodeEncoder {
// TODO: Ensure uniform chunks, encoding and identifier generation sort fields the same.
Expand All @@ -39,6 +40,19 @@ export class NodeShape extends Shape<EncodedChunkShape> implements NodeEncoder {
this.explicitKeys = new Set(this.fields.map((f) => f.key));
}

private getValueToEncode(cursor: ITreeCursorSynchronous, cache: EncoderCache): Value {
if (this.value === 0) {
assert(typeof cursor.value === "string", "identifier must be type string");
if (isStableId(cursor.value)) {
const sessionSpaceCompressedId = cache.idCompressor.tryRecompress(cursor.value);
if (sessionSpaceCompressedId !== undefined) {
return cache.idCompressor.normalizeToOpSpace(sessionSpaceCompressedId);
}
}
}
return cursor.value;
}

public encodeNode(
cursor: ITreeCursorSynchronous,
cache: EncoderCache,
Expand All @@ -49,19 +63,7 @@ export class NodeShape extends Shape<EncodedChunkShape> implements NodeEncoder {
} else {
assert(cursor.type === this.type, 0x741 /* type must match shape */);
}

if (this.value === 0) {
const sessionSpaceCompressedId = cache.idCompressor.tryRecompress(
cursor.value as StableId,
);
const opSpaceCompressedId =
sessionSpaceCompressedId !== undefined
? cache.idCompressor.normalizeToOpSpace(sessionSpaceCompressedId)
: cursor.value;
encodeValue(opSpaceCompressedId, this.value, outputBuffer);
} else {
encodeValue(cursor.value, this.value, outputBuffer);
}
encodeValue(this.getValueToEncode(cursor, cache), this.value, outputBuffer);
for (const field of this.fields) {
cursor.enterField(brand(field.key));
field.shape.encodeField(cursor, cache, outputBuffer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { strict as assert } from "assert";
import type { SessionId, StableId } from "@fluidframework/id-compressor";
import type { SessionId } from "@fluidframework/id-compressor";
import { createIdCompressor } from "@fluidframework/id-compressor/internal";

import {
Expand Down Expand Up @@ -81,7 +81,7 @@ const schemaWithIdentifier = schemaFactory.object("parent", {
identifier: schemaFactory.identifier,
});

function getIdentifierEncodingContext(id: StableId) {
function getIdentifierEncodingContext(id: string) {
const config = new TreeConfiguration(schemaWithIdentifier, () => ({
identifier: id,
}));
Expand Down Expand Up @@ -253,7 +253,7 @@ describe("End to end chunked encoding", () => {
assert.equal(identifierValue, testIdCompressor.recompress(id));
});

it("is the uncompressed value when it is an unknown/invalid identifier", () => {
it("is the uncompressed value when it is an unknown identifier", () => {
// generate an id from a different id compressor.
const nodeKeyManager = new MockNodeKeyManager();
const id = nodeKeyManager.stabilizeNodeKey(nodeKeyManager.generateLocalNodeKey());
Expand All @@ -280,5 +280,30 @@ describe("End to end chunked encoding", () => {
// Check that the identifierValue is the original uncompressed id.
assert.equal(identifierValue, id);
});

it("is the uncompressed value when it is not a UUID", () => {
const id = "invalidUUID";
const { encoderContext, checkout } = getIdentifierEncodingContext(id);

const forestSummarizer = new ForestSummarizer(
checkout.forest,
new RevisionTagCodec(testIdCompressor),
fieldBatchCodec,
encoderContext,
options,
testIdCompressor,
);

function stringifier(content: unknown) {
return JSON.stringify(content);
}
const { summary } = forestSummarizer.getAttachSummary(stringifier);
const tree = summary.tree.ForestTree;
assert(tree.type === SummaryType.Blob);
const treeContent = JSON.parse(tree.content as string);
const identifierValue = treeContent.fields.data[0][1];
// Check that the identifierValue is the original uncompressed id.
assert.equal(identifierValue, id);
});
});
});

0 comments on commit cdcf65c

Please sign in to comment.