fix: assure bucket endpoint middleware inserted before host-header-middleware (#574)

* fix: use host name from request instead of endpont provider

* fix: assure bucketEndpointMw applies before hostHeaderMw

Author: AllanZhengYP

packages/middleware-bucket-endpoint/package.json

@@ -20,6 +20,7 @@
   "devDependencies": {
     "@types/jest": "^24.0.12",
     "jest": "^24.7.1",
-    "typescript": "~3.4.0"
+    "typescript": "~3.4.0",
+    "@aws-sdk/middleware-stack": "^0.1.0-preview.6"
   }
 }

packages/middleware-bucket-endpoint/src/bucketEndpointMiddleware.spec.ts

@@ -1,5 +1,9 @@
-import { bucketEndpointMiddleware } from "./bucketEndpointMiddleware";
 import { HttpRequest } from "@aws-sdk/protocol-http";
+import { MiddlewareStack } from "@aws-sdk/middleware-stack";
+import {
+  bucketEndpointMiddleware,
+  bucketEndpointMiddlewareOptions
+} from "./bucketEndpointMiddleware";
 import { resolveBucketEndpointConfig } from "./configurations";
 
 describe("bucketEndpointMiddleware", () => {
@@ -129,4 +133,29 @@ describe("bucketEndpointMiddleware", () => {
     expect(hostname).toBe("bucket.s3-accelerate.dualstack.amazonaws.com");
     expect(path).toBe("/");
   });
+
+  it("should be inserted before 'hostheaderMiddleware' if exists", async () => {
+    const stack = new MiddlewareStack();
+    const mockHostheaderMiddleware = (next: any) => (args: any) => {
+      args.request.arr.push("two");
+      return next(args);
+    };
+    const mockbucketEndpointMiddleware = (next: any) => (args: any) => {
+      args.request.arr.push("one");
+      return next(args);
+    };
+    stack.add(mockHostheaderMiddleware, {
+      ...bucketEndpointMiddlewareOptions,
+      name: bucketEndpointMiddlewareOptions.toMiddleware
+    });
+    stack.addRelativeTo(
+      mockbucketEndpointMiddleware,
+      bucketEndpointMiddlewareOptions
+    );
+    const handler = stack.resolve(next, {} as any);
+    expect.assertions(2);
+    await handler({ request: { arr: [] }, input: {} } as any);
+    expect(next.mock.calls.length).toBe(1);
+    expect(next.mock.calls[0][0].request.arr).toEqual(["one", "two"]);
+  });
 });

packages/middleware-bucket-endpoint/src/bucketEndpointMiddleware.ts

@@ -7,7 +7,8 @@ import {
   BuildHandlerOutput,
   BuildMiddleware,
   MetadataBearer,
-  Pluggable
+  Pluggable,
+  RelativeLocation
 } from "@aws-sdk/types";
 import { HttpRequest } from "@aws-sdk/protocol-http";
 
@@ -51,17 +52,20 @@ export function bucketEndpointMiddleware(
   };
 }
 
-export const bucketEndpointMiddlewareOptions: BuildHandlerOptions = {
+export const bucketEndpointMiddlewareOptions: BuildHandlerOptions &
+  RelativeLocation<any, any> = {
   step: "build",
   tags: ["BUCKET_ENDPOINT"],
-  name: "bucketEndpointMiddleware"
+  name: "bucketEndpointMiddleware",
+  relation: "before",
+  toMiddleware: "hostHeaderMiddleware"
 };
 
 export const getBucketEndpointPlugin = (
   options: BucketEndpointResolvedConfig
 ): Pluggable<any, any> => ({
   applyToStack: clientStack => {
-    clientStack.add(
+    clientStack.addRelativeTo(
       bucketEndpointMiddleware(options),
       bucketEndpointMiddlewareOptions
     );

packages/middleware-host-header/jest.config.js

@@ -0,0 +1,5 @@
+const base = require("../../jest.config.base.js");
+
+module.exports = {
+  ...base
+};

packages/middleware-host-header/src/index.spec.ts

@@ -0,0 +1,62 @@
+import { hostHeaderMiddleware } from "./index";
+import { HttpRequest } from "@aws-sdk/protocol-http";
+describe("hostHeaderMiddleware", () => {
+  const mockNextHandler = jest.fn();
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it("should set host header if not already set", async () => {
+    expect.assertions(2);
+    const middleware = hostHeaderMiddleware({ requestHandler: {} as any });
+    const handler = middleware(mockNextHandler, {} as any);
+    await handler({
+      input: {},
+      request: new HttpRequest({ hostname: "foo.amazonaws.com" })
+    });
+    expect(mockNextHandler.mock.calls.length).toEqual(1);
+    expect(mockNextHandler.mock.calls[0][0].request.headers.host).toBe(
+      "foo.amazonaws.com"
+    );
+  });
+
+  it("should not set host header if already set", async () => {
+    expect.assertions(2);
+    const middleware = hostHeaderMiddleware({ requestHandler: {} as any });
+    const handler = middleware(mockNextHandler, {} as any);
+    await handler({
+      input: {},
+      request: new HttpRequest({
+        hostname: "foo.amazonaws.com",
+        headers: { host: "random host" }
+      })
+    });
+    expect(mockNextHandler.mock.calls.length).toEqual(1);
+    expect(mockNextHandler.mock.calls[0][0].request.headers.host).toBe(
+      "random host"
+    );
+  });
+
+  it("should set :authority header for H2 requests", async () => {
+    expect.assertions(3);
+    const middleware = hostHeaderMiddleware({
+      requestHandler: { metadata: { handlerProtocol: "h2" } }
+    } as any);
+    const handler = middleware(mockNextHandler, {} as any);
+    await handler({
+      input: {},
+      request: new HttpRequest({
+        hostname: "foo.amazonaws.com",
+        headers: { host: "random host" }
+      })
+    });
+    expect(mockNextHandler.mock.calls.length).toEqual(1);
+    expect(
+      mockNextHandler.mock.calls[0][0].request.headers.host
+    ).not.toBeDefined();
+    expect(
+      mockNextHandler.mock.calls[0][0].request.headers[":authority"]
+    ).toEqual("");
+  });
+});

packages/middleware-host-header/src/index.ts

@@ -2,8 +2,6 @@ import { HttpRequest } from "@aws-sdk/protocol-http";
 import {
   RequestHandler,
   BuildMiddleware,
-  Provider,
-  Endpoint,
   BuildHandlerOptions,
   AbsoluteLocation,
   Pluggable
@@ -12,11 +10,9 @@ import {
 export interface HostHeaderInputConfig {}
 interface PreviouslyResolved {
   requestHandler: RequestHandler<any, any>;
-  endpoint: Provider<Endpoint>;
 }
 export interface HostHeaderResolvedConfig {
   requestHandler: RequestHandler<any, any>;
-  endpoint: Provider<Endpoint>;
 }
 export function resolveHostHeaderConfig<T>(
   input: T & PreviouslyResolved & HostHeaderInputConfig
@@ -40,7 +36,7 @@ export const hostHeaderMiddleware = <
     request.headers[":authority"] = "";
     //non-H2 request and 'host' header is not set, set the 'host' header to request's hostname.
   } else if (!request.headers["host"]) {
-    request.headers["host"] = (await options.endpoint()).hostname;
+    request.headers["host"] = request.hostname;
   }
   return next(args);
 };