Skip to content

Commit

Permalink
Validate Input to rehydrateDetachedContainerFromSnapshot (microsoft#1…
Browse files Browse the repository at this point in the history
…7706)

Validates that the string passed to
rehydrateDetachedContainerFromSnapshot is of the correct format, rather
than assuming via cast which can lead to random failures. Also add a
unit test to verify the validation

---------

Co-authored-by: Tony Murphy <[email protected]>
  • Loading branch information
anthony-murphy and anthony-murphy authored Oct 16, 2023
1 parent a79478d commit eebb3b2
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 6 deletions.
15 changes: 11 additions & 4 deletions packages/loader/container-loader/src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ const savedContainerEvent = "saved";

const packageNotFactoryError = "Code package does not implement IRuntimeFactory";

const hasBlobsSummaryTree = ".hasAttachmentBlobs";

/**
* @internal
*/
Expand Down Expand Up @@ -473,7 +475,11 @@ export class Container
container.mc.logger,
{ eventName: "RehydrateDetachedFromSnapshot" },
async (_event) => {
const deserializedSummary = JSON.parse(snapshot) as ISummaryTree;
const deserializedSummary = JSON.parse(snapshot);
if (!isCombinedAppAndProtocolSummary(deserializedSummary, hasBlobsSummaryTree)) {
throw new UsageError("Cannot rehydrate detached container. Incorrect format");
}

await container.rehydrateDetachedFromSnapshot(deserializedSummary);
return container;
},
Expand Down Expand Up @@ -1178,7 +1184,7 @@ export class Container
const combinedSummary = combineAppAndProtocolSummary(appSummary, protocolSummary);

if (this.detachedBlobStorage && this.detachedBlobStorage.size > 0) {
combinedSummary.tree[".hasAttachmentBlobs"] = {
combinedSummary.tree[hasBlobsSummaryTree] = {
type: SummaryType.Blob,
content: "true",
};
Expand Down Expand Up @@ -1815,12 +1821,13 @@ export class Container
}

private async rehydrateDetachedFromSnapshot(detachedContainerSnapshot: ISummaryTree) {
if (detachedContainerSnapshot.tree[".hasAttachmentBlobs"] !== undefined) {
if (detachedContainerSnapshot.tree[hasBlobsSummaryTree] !== undefined) {
assert(
!!this.detachedBlobStorage && this.detachedBlobStorage.size > 0,
0x250 /* "serialized container with attachment blobs must be rehydrated with detached blob storage" */,
);
delete detachedContainerSnapshot.tree[".hasAttachmentBlobs"];
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete detachedContainerSnapshot.tree[hasBlobsSummaryTree];
}

const snapshotTree = getSnapshotTreeFromSerializedContainer(detachedContainerSnapshot);
Expand Down
130 changes: 130 additions & 0 deletions packages/loader/container-loader/src/test/loader.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/*!
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
* Licensed under the MIT License.
*/

import assert from "assert";
import { v4 as uuid } from "uuid";
import { isFluidError } from "@fluidframework/telemetry-utils";
import { FluidErrorTypes } from "@fluidframework/core-interfaces";
import { ICreateBlobResponse, SummaryType } from "@fluidframework/protocol-definitions";
import { IRuntime } from "@fluidframework/container-definitions";
import { stringToBuffer } from "@fluid-internal/client-utils";
import { IDetachedBlobStorage, Loader } from "../loader";

const failProxy = <T extends object>() => {
const proxy = new Proxy<T>({} as any as T, {
get: (_, p) => {
throw Error(`${p.toString()} not implemented`);
},
});
return proxy;
};

const failSometimeProxy = <T extends object>(handler: Partial<T>) => {
const proxy = new Proxy<T>(handler as T, {
get: (t, p, r) => {
if (p in handler) {
return Reflect.get(t, p, r);
}
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return failProxy();
},
});
return proxy;
};

const codeLoader = {
load: async () => {
return {
details: {
package: "none",
},
module: {
fluidExport: {
IRuntimeFactory: {
get IRuntimeFactory() {
return this;
},
async instantiateRuntime(context, existing) {
return failSometimeProxy<IRuntime>({
createSummary: () => ({
tree: {},
type: SummaryType.Tree,
}),
});
},
},
},
},
};
},
};

describe("loader unit test", () => {
it("rehydrateDetachedContainerFromSnapshot with invalid format", async () => {
const loader = new Loader({
codeLoader: failProxy(),
documentServiceFactory: failProxy(),
urlResolver: failProxy(),
});

try {
await loader.rehydrateDetachedContainerFromSnapshot(`{"foo":"bar"}`);
assert.fail("should fail");
} catch (e) {
assert.strict(isFluidError(e), `should be a Fluid error: ${e}`);
assert.strictEqual(e.errorType, FluidErrorTypes.usageError, "should be a usage error");
}
});

it("rehydrateDetachedContainerFromSnapshot with valid format", async () => {
const loader = new Loader({
codeLoader,
documentServiceFactory: failProxy(),
urlResolver: failProxy(),
});
const detached = await loader.createDetachedContainer({ package: "none" });
const summary = detached.serialize();
assert.strictEqual(
summary,
'{"type":1,"tree":{".protocol":{"tree":{"attributes":{"content":"{\\"minimumSequenceNumber\\":0,\\"sequenceNumber\\":0}","type":2},"quorumMembers":{"content":"[]","type":2},"quorumProposals":{"content":"[]","type":2},"quorumValues":{"content":"[[\\"code\\",{\\"key\\":\\"code\\",\\"value\\":{\\"package\\":\\"none\\"},\\"approvalSequenceNumber\\":0,\\"commitSequenceNumber\\":0,\\"sequenceNumber\\":0}]]","type":2}},"type":1},".app":{"tree":{},"type":1}}}',
"summary does not match expected format",
);
await loader.rehydrateDetachedContainerFromSnapshot(summary);
});

it("rehydrateDetachedContainerFromSnapshot with valid format and attachment blobs", async () => {
const blobs = new Map<string, ArrayBufferLike>();
const detachedBlobStorage: IDetachedBlobStorage = {
createBlob: async (file) => {
const response: ICreateBlobResponse = {
id: uuid(),
};
blobs.set(response.id, file);
return response;
},
getBlobIds: () => [...blobs.keys()],
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
readBlob: async (id) => blobs.get(id)!,
get size() {
return blobs.size;
},
};
const loader = new Loader({
codeLoader,
documentServiceFactory: failProxy(),
urlResolver: failProxy(),
detachedBlobStorage,
});
const detached = await loader.createDetachedContainer({ package: "none" });
await detachedBlobStorage.createBlob(stringToBuffer("whatever", "utf8"));
const summary = detached.serialize();
assert.strictEqual(
summary,
'{"type":1,"tree":{".protocol":{"tree":{"attributes":{"content":"{\\"minimumSequenceNumber\\":0,\\"sequenceNumber\\":0}","type":2},"quorumMembers":{"content":"[]","type":2},"quorumProposals":{"content":"[]","type":2},"quorumValues":{"content":"[[\\"code\\",{\\"key\\":\\"code\\",\\"value\\":{\\"package\\":\\"none\\"},\\"approvalSequenceNumber\\":0,\\"commitSequenceNumber\\":0,\\"sequenceNumber\\":0}]]","type":2}},"type":1},".app":{"tree":{},"type":1},".hasAttachmentBlobs":{"type":2,"content":"true"}}}',
"summary does not match expected format",
);
await loader.rehydrateDetachedContainerFromSnapshot(summary);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export interface IProgress {
}

// @internal
export function isCombinedAppAndProtocolSummary(summary: ISummaryTree | undefined): summary is CombinedAppAndProtocolSummary;
export function isCombinedAppAndProtocolSummary(summary: ISummaryTree | undefined, ...optionalRootTrees: string[]): summary is CombinedAppAndProtocolSummary;

// @public (undocumented)
export function isOnline(): OnlineStatus;
Expand Down
3 changes: 2 additions & 1 deletion packages/loader/driver-utils/src/summaryForCreateNew.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface CombinedAppAndProtocolSummary extends ISummaryTree {
*/
export function isCombinedAppAndProtocolSummary(
summary: ISummaryTree | undefined,
...optionalRootTrees: string[]
): summary is CombinedAppAndProtocolSummary {
if (
summary?.tree === undefined ||
Expand All @@ -38,7 +39,7 @@ export function isCombinedAppAndProtocolSummary(
) {
return false;
}
const treeKeys = Object.keys(summary.tree);
const treeKeys = Object.keys(summary.tree).filter((t) => !optionalRootTrees.includes(t));
if (treeKeys.length !== 2) {
return false;
}
Expand Down

0 comments on commit eebb3b2

Please sign in to comment.