From 7a207a9d3173631f62f8b90ee1fbd7f68342133a Mon Sep 17 00:00:00 2001 From: AllanZhengYP Date: Wed, 5 Jan 2022 09:34:50 -0800 Subject: [PATCH] fix(middleware-signing): correct clock skew from error response (#3133) --- .../src/deserializerMiddleware.spec.ts | 13 ++++++++++ .../src/deserializerMiddleware.ts | 14 ++++++---- .../middleware-signing/src/middleware.spec.ts | 26 +++++++++++++++++-- packages/middleware-signing/src/middleware.ts | 13 ++++++---- packages/types/src/shapes.ts | 6 +++++ 5 files changed, 60 insertions(+), 12 deletions(-) diff --git a/packages/middleware-serde/src/deserializerMiddleware.spec.ts b/packages/middleware-serde/src/deserializerMiddleware.spec.ts index 88043c3a0d6b..69e261901f17 100644 --- a/packages/middleware-serde/src/deserializerMiddleware.spec.ts +++ b/packages/middleware-serde/src/deserializerMiddleware.spec.ts @@ -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); + } + }); }); diff --git a/packages/middleware-serde/src/deserializerMiddleware.ts b/packages/middleware-serde/src/deserializerMiddleware.ts index 4e7a52728b7e..34661d0bf606 100644 --- a/packages/middleware-serde/src/deserializerMiddleware.ts +++ b/packages/middleware-serde/src/deserializerMiddleware.ts @@ -15,9 +15,13 @@ export const deserializerMiddleware = (next: DeserializeHandler, context: HandlerExecutionContext): DeserializeHandler => async (args: DeserializeHandlerArguments): Promise> => { 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 }); + } }; diff --git a/packages/middleware-signing/src/middleware.spec.ts b/packages/middleware-signing/src/middleware.spec.ts index af8a689cb170..e9b8ae8b1d67 100644 --- a/packages/middleware-signing/src/middleware.spec.ts +++ b/packages/middleware-signing/src/middleware.spec.ts @@ -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); @@ -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 () => { diff --git a/packages/middleware-signing/src/middleware.ts b/packages/middleware-signing/src/middleware.ts index 75ee4bf74fe8..8f007b19215a 100644 --- a/packages/middleware-signing/src/middleware.ts +++ b/packages/middleware-signing/src/middleware.ts @@ -1,4 +1,4 @@ -import { HttpRequest } from "@aws-sdk/protocol-http"; +import { HttpRequest, HttpResponse } from "@aws-sdk/protocol-http"; import { FinalizeHandler, FinalizeHandlerArguments, @@ -29,14 +29,14 @@ 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); } @@ -44,6 +44,9 @@ export const awsAuthMiddleware = 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"], diff --git a/packages/types/src/shapes.ts b/packages/types/src/shapes.ts index 7f5c537781e8..81334e87c8e0 100644 --- a/packages/types/src/shapes.ts +++ b/packages/types/src/shapes.ts @@ -1,3 +1,4 @@ +import { HttpResponse } from "./http"; import { MetadataBearer } from "./response"; /** @@ -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 & Partial;