Skip to content

Commit

Permalink
fix: assure bucket endpoint middleware inserted before host-header-mi…
Browse files Browse the repository at this point in the history
…ddleware (#574)

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

* fix: assure bucketEndpointMw applies before hostHeaderMw
  • Loading branch information
AllanZhengYP authored and trivikr committed Jan 3, 2020
1 parent ae04135 commit c9c4127
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 11 deletions.
3 changes: 2 additions & 1 deletion packages/middleware-bucket-endpoint/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -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"]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import {
BuildHandlerOutput,
BuildMiddleware,
MetadataBearer,
Pluggable
Pluggable,
RelativeLocation
} from "@aws-sdk/types";
import { HttpRequest } from "@aws-sdk/protocol-http";

Expand Down Expand Up @@ -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
);
Expand Down
5 changes: 5 additions & 0 deletions packages/middleware-host-header/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const base = require("../../jest.config.base.js");

module.exports = {
...base
};
62 changes: 62 additions & 0 deletions packages/middleware-host-header/src/index.spec.ts
Original file line number Diff line number Diff line change
@@ -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("");
});
});
6 changes: 1 addition & 5 deletions packages/middleware-host-header/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import { HttpRequest } from "@aws-sdk/protocol-http";
import {
RequestHandler,
BuildMiddleware,
Provider,
Endpoint,
BuildHandlerOptions,
AbsoluteLocation,
Pluggable
Expand All @@ -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
Expand All @@ -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);
};
Expand Down

0 comments on commit c9c4127

Please sign in to comment.