Skip to content

Commit

Permalink
fix(middleware-signing): correct clock skew from error response (#3133)
Browse files Browse the repository at this point in the history
  • Loading branch information
AllanZhengYP committed Jan 5, 2022
1 parent f6a3ef5 commit 7a207a9
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 12 deletions.
13 changes: 13 additions & 0 deletions packages/middleware-serde/src/deserializerMiddleware.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,17 @@ describe("deserializerMiddleware", () => {
expect(mockDeserializer).toHaveBeenCalledTimes(1);
expect(mockDeserializer).toHaveBeenCalledWith(mockNextResponse.response, mockOptions);
});

it("injects $response reference to deserializing exceptions", async () => {
const exception = Object.assign(new Error("MockException"), mockNextResponse.response);
mockDeserializer.mockReset();
mockDeserializer.mockRejectedValueOnce(exception);
try {
await deserializerMiddleware(mockOptions, mockDeserializer)(mockNext, {})(mockArgs);
fail("DeserializerMiddleware should throw");
} catch (e) {
expect(e).toMatchObject(exception);
expect(e.$response).toEqual(mockNextResponse.response);
}
});
});
14 changes: 9 additions & 5 deletions packages/middleware-serde/src/deserializerMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@ export const deserializerMiddleware =
(next: DeserializeHandler<Input, Output>, context: HandlerExecutionContext): DeserializeHandler<Input, Output> =>
async (args: DeserializeHandlerArguments<Input>): Promise<DeserializeHandlerOutput<Output>> => {
const { response } = await next(args);
const parsed = await deserializer(response, options);
return {
response,
output: parsed as Output,
};
try {
const parsed = await deserializer(response, options);
return {
response,
output: parsed as Output,
};
} catch (error) {
throw Object.assign(error, { $response: response });
}
};
26 changes: 24 additions & 2 deletions packages/middleware-signing/src/middleware.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ describe(awsAuthMiddleware.name, () => {
});

describe("should update systemClockOffset if date header is present", () => {
it.each(["date", "Date"])("header[%s]", async (headerName) => {
it.each(["date", "Date"])("header '%s'", async (headerName) => {
const dateHeader = new Date().toString();
const options = { ...mockOptions };
const signingHandler = awsAuthMiddleware(options)(mockNext, {});

const mockResponseWithDateHeader = { response: { headers: { [headerName]: dateHeader } } };
const mockResponseWithDateHeader = { response: { headers: { [headerName]: dateHeader }, statusCode: 200 } };
mockNext.mockResolvedValue(mockResponseWithDateHeader);

const output = await signingHandler(mockSigningHandlerArgs);
Expand All @@ -88,6 +88,28 @@ describe(awsAuthMiddleware.name, () => {
expect(getUpdatedSystemClockOffset).toHaveBeenCalledTimes(1);
expect(getUpdatedSystemClockOffset).toHaveBeenCalledWith(dateHeader, mockSystemClockOffset);
});

it.each(["date", "Date"])("error response with header '%s'", async (headerName) => {
const serverTime = new Date().toString();
const options = { ...mockOptions };
const signingHandler = awsAuthMiddleware(options)(mockNext, {});

const mockError = Object.assign(new Error("error"), {
$response: { statusCode: 400, headers: { [headerName]: serverTime } },
});
mockNext.mockRejectedValue(mockError);

try {
await signingHandler(mockSigningHandlerArgs);
fail(`should throw ${mockError}`);
} catch (error) {
expect(error).toStrictEqual(mockError);
}

expect(options.systemClockOffset).toBe(mockUpdatedSystemClockOffset);
expect(getUpdatedSystemClockOffset).toHaveBeenCalledTimes(1);
expect(getUpdatedSystemClockOffset).toHaveBeenCalledWith(serverTime, mockSystemClockOffset);
});
});

it("should update systemClockOffset if error contains ServerTime", async () => {
Expand Down
13 changes: 8 additions & 5 deletions packages/middleware-signing/src/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { HttpRequest } from "@aws-sdk/protocol-http";
import { HttpRequest, HttpResponse } from "@aws-sdk/protocol-http";
import {
FinalizeHandler,
FinalizeHandlerArguments,
Expand Down Expand Up @@ -29,21 +29,24 @@ export const awsAuthMiddleware =
signingService: context["signing_service"],
}),
}).catch((error) => {
if (error.ServerTime) {
options.systemClockOffset = getUpdatedSystemClockOffset(error.ServerTime, options.systemClockOffset);
const serverTime: string | undefined = error.ServerTime ?? getDateHeader(error.$response);
if (serverTime) {
options.systemClockOffset = getUpdatedSystemClockOffset(serverTime, options.systemClockOffset);
}
throw error;
});

const { headers } = output.response as any;
const dateHeader = headers && (headers.date || headers.Date);
const dateHeader = getDateHeader(output.response);
if (dateHeader) {
options.systemClockOffset = getUpdatedSystemClockOffset(dateHeader, options.systemClockOffset);
}

return output;
};

const getDateHeader = (response: unknown): string | undefined =>
HttpResponse.isInstance(response) ? response.headers?.date ?? response.headers?.Date : undefined;

export const awsAuthMiddlewareOptions: RelativeMiddlewareOptions = {
name: "awsAuthMiddleware",
tags: ["SIGNATURE", "AWSAUTH"],
Expand Down
6 changes: 6 additions & 0 deletions packages/types/src/shapes.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { HttpResponse } from "./http";
import { MetadataBearer } from "./response";

/**
Expand Down Expand Up @@ -45,6 +46,11 @@ export interface SmithyException {
* Indicates that an error MAY be retried by the client.
*/
readonly $retryable?: RetryableTrait;

/**
* Reference to low-level HTTP response object.
*/
readonly $response?: HttpResponse;
}

export type SdkError = Error & Partial<SmithyException> & Partial<MetadataBearer>;

0 comments on commit 7a207a9

Please sign in to comment.