Skip to content

Commit

Permalink
fix(middleware-flexible-checksums): skip checksum validation if CRC64…
Browse files Browse the repository at this point in the history
…NVME dependency is absent
  • Loading branch information
trivikr committed Jan 21, 2025
1 parent c3f3d0a commit 2a3b5c1
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export const flexibleChecksumsResponseMiddleware =
await validateChecksumFromResponse(result.response as HttpResponse, {
config,
responseAlgorithms,
logger: context.logger,
});

if (isStreamingBody && collectedStream) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { HttpResponse } from "@smithy/protocol-http";
import { Logger } from "@smithy/types";
import { createChecksumStream } from "@smithy/util-stream";
import { afterEach, beforeEach, describe, expect, test as it, vi } from "vitest";

Expand Down Expand Up @@ -32,11 +33,13 @@ describe(validateChecksumFromResponse.name, () => {
} as HttpResponse;

const mockChecksum = "mockChecksum";
const mockResponseAlgorithms = [ChecksumAlgorithm.CRC32, ChecksumAlgorithm.CRC32C];
const mockResponseAlgorithms = [ChecksumAlgorithm.CRC32, ChecksumAlgorithm.CRC32C, ChecksumAlgorithm.CRC64NVME];
const mockLogger = { warn: vi.fn() } as unknown as Logger;

const mockOptions = {
config: mockConfig,
responseAlgorithms: mockResponseAlgorithms,
logger: mockLogger,
};

const mockChecksumAlgorithmFn = vi.fn();
Expand Down Expand Up @@ -66,7 +69,6 @@ describe(validateChecksumFromResponse.name, () => {

describe("skip validation", () => {
afterEach(() => {
expect(selectChecksumAlgorithmFunction).not.toHaveBeenCalled();
expect(getChecksum).not.toHaveBeenCalled();
});

Expand All @@ -75,19 +77,38 @@ describe(validateChecksumFromResponse.name, () => {
await validateChecksumFromResponse(mockResponse, { ...mockOptions, responseAlgorithms: emptyAlgorithmsList });
expect(getChecksumAlgorithmListForResponse).toHaveBeenCalledWith(emptyAlgorithmsList);
expect(getChecksumLocationName).not.toHaveBeenCalled();
expect(selectChecksumAlgorithmFunction).not.toHaveBeenCalled();
});

it("if updated algorithm list from response is empty", async () => {
vi.mocked(getChecksumAlgorithmListForResponse).mockImplementation(() => []);
await validateChecksumFromResponse(mockResponse, mockOptions);
expect(getChecksumAlgorithmListForResponse).toHaveBeenCalledWith(mockResponseAlgorithms);
expect(getChecksumLocationName).not.toHaveBeenCalled();
expect(selectChecksumAlgorithmFunction).not.toHaveBeenCalled();
});

it("if checksum is not present in header", async () => {
await validateChecksumFromResponse(mockResponse, mockOptions);
expect(getChecksumAlgorithmListForResponse).toHaveBeenCalledWith(mockResponseAlgorithms);
expect(getChecksumLocationName).toHaveBeenCalledTimes(mockResponseAlgorithms.length);
expect(selectChecksumAlgorithmFunction).not.toHaveBeenCalled();
});

it(`if checksum algorithm is ${ChecksumAlgorithm.CRC64NVME} and dependency is not available`, async () => {
const dependencyErrorMsg = "Dependency not available";
vi.mocked(selectChecksumAlgorithmFunction).mockImplementation(() => {
throw new Error(dependencyErrorMsg);
});
const responseWithChecksum = getMockResponseWithHeader(ChecksumAlgorithm.CRC64NVME, mockChecksum);
await validateChecksumFromResponse(responseWithChecksum, mockOptions);
expect(getChecksumAlgorithmListForResponse).toHaveBeenCalledWith(mockResponseAlgorithms);
expect(getChecksumLocationName).toHaveBeenCalledTimes(mockResponseAlgorithms.length);
expect(selectChecksumAlgorithmFunction).toHaveBeenCalledTimes(1);
expect(mockLogger.warn).toHaveBeenCalledTimes(1);
expect(mockLogger.warn).toHaveBeenCalledWith(
`Skipping ${ChecksumAlgorithm.CRC64NVME} checksum validation: ${dependencyErrorMsg}`
);
});
});

Expand Down Expand Up @@ -134,6 +155,17 @@ describe(validateChecksumFromResponse.name, () => {
expect(getChecksumLocationName).toHaveBeenNthCalledWith(2, mockResponseAlgorithms[1]);
validateCalls(isStream, mockResponseAlgorithms[1]);
});

it.each([false, true])("when checksum is populated for third algorithm when streaming: %s", async (isStream) => {
vi.mocked(isStreaming).mockReturnValue(isStream);
const responseWithChecksum = getMockResponseWithHeader(mockResponseAlgorithms[2], mockChecksum);
await validateChecksumFromResponse(responseWithChecksum, mockOptions);
expect(getChecksumLocationName).toHaveBeenCalledTimes(3);
expect(getChecksumLocationName).toHaveBeenNthCalledWith(1, mockResponseAlgorithms[0]);
expect(getChecksumLocationName).toHaveBeenNthCalledWith(2, mockResponseAlgorithms[1]);
expect(getChecksumLocationName).toHaveBeenNthCalledWith(3, mockResponseAlgorithms[2]);
validateCalls(isStream, mockResponseAlgorithms[2]);
});
});

it("throw error if checksum value is not accurate when not streaming", async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { HttpResponse } from "@smithy/protocol-http";
import { Checksum } from "@smithy/types";
import { Checksum, ChecksumConstructor, HashConstructor, Logger } from "@smithy/types";
import { createChecksumStream } from "@smithy/util-stream";

import { PreviouslyResolved } from "./configuration";
Expand All @@ -18,11 +18,13 @@ export interface ValidateChecksumFromResponseOptions {
* returned in the HTTP response.
*/
responseAlgorithms?: string[];

logger?: Logger;
}

export const validateChecksumFromResponse = async (
response: HttpResponse,
{ config, responseAlgorithms }: ValidateChecksumFromResponseOptions
{ config, responseAlgorithms, logger }: ValidateChecksumFromResponseOptions
) => {
// Verify checksum in response header.
const checksumAlgorithms = getChecksumAlgorithmListForResponse(responseAlgorithms);
Expand All @@ -31,7 +33,17 @@ export const validateChecksumFromResponse = async (
const responseHeader = getChecksumLocationName(algorithm);
const checksumFromResponse = responseHeaders[responseHeader];
if (checksumFromResponse) {
const checksumAlgorithmFn = selectChecksumAlgorithmFunction(algorithm as ChecksumAlgorithm, config);
let checksumAlgorithmFn: ChecksumConstructor | HashConstructor;
try {
checksumAlgorithmFn = selectChecksumAlgorithmFunction(algorithm as ChecksumAlgorithm, config);
} catch (error) {
if (algorithm === ChecksumAlgorithm.CRC64NVME) {
logger?.warn(`Skipping ${ChecksumAlgorithm.CRC64NVME} checksum validation: ${error.message}`);
continue;
}
throw error;
}

const { base64Encoder } = config;

if (isStreaming(responseBody)) {
Expand Down

0 comments on commit 2a3b5c1

Please sign in to comment.