Skip to content

Commit

Permalink
fix(middleware-sdk-s3): restore bucketEndpoint configurability (#5840)
Browse files Browse the repository at this point in the history
* fix(middleware-sdk-s3): restore bucketEndpoint configurability

* test: fix tests
  • Loading branch information
kuhe committed Feb 29, 2024
1 parent 1e2b1ae commit f05b540
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 8 deletions.
56 changes: 56 additions & 0 deletions packages/middleware-sdk-s3/src/bucket-endpoint-middleware.ts
@@ -0,0 +1,56 @@
import {
HandlerExecutionContext,
MetadataBearer,
RelativeMiddlewareOptions,
SerializeHandler,
SerializeHandlerArguments,
SerializeHandlerOutput,
SerializeMiddleware,
} from "@smithy/types";

interface PreviouslyResolved {
bucketEndpoint?: boolean;
}

/**
* @internal
*/
export function bucketEndpointMiddleware(options: PreviouslyResolved): SerializeMiddleware<any, any> {
return <Output extends MetadataBearer>(
next: SerializeHandler<any, Output>,
context: HandlerExecutionContext
): SerializeHandler<any, Output> =>
async (args: SerializeHandlerArguments<any>): Promise<SerializeHandlerOutput<Output>> => {
if (options.bucketEndpoint) {
const endpoint = context.endpointV2;
if (endpoint) {
const bucket: string | undefined = args.input.Bucket;
if (typeof bucket === "string") {
try {
const bucketEndpointUrl = new URL(bucket);
endpoint.url = bucketEndpointUrl;
} catch (e) {
const warning = `@aws-sdk/middleware-sdk-s3: bucketEndpoint=true was set but Bucket=${bucket} could not be parsed as URL.`;
if (context.logger?.constructor?.name === "NoOpLogger") {
console.warn(warning);
} else {
context.logger?.warn?.(warning);
}
throw e;
}
}
}
}
return next(args);
};
}

/**
* @internal
*/
export const bucketEndpointMiddlewareOptions: RelativeMiddlewareOptions = {
name: "bucketEndpointMiddleware",
override: true,
relation: "after",
toMiddleware: "endpointV2Middleware",
};
26 changes: 26 additions & 0 deletions packages/middleware-sdk-s3/src/middleware-sdk-s3.integ.spec.ts
Expand Up @@ -57,5 +57,31 @@ describe("middleware-sdk-s3", () => {

expect.hasAssertions();
});

it("allows using a bucket input value as the endpoint", async () => {
const client = new S3({
region: "us-west-2",
bucketEndpoint: true,
});

requireRequestsFrom(client).toMatch({
query: { "x-id": "PutObject" },
protocol: "https:",
hostname: "mybucket.com",
port: 8888,
path: "/my-bucket-path/my-key",
headers: {
host: "mybucket.com:8888",
},
});

await client.putObject({
Bucket: "https://mybucket.com:8888/my-bucket-path",
Key: "my-key",
Body: "abcd",
});

expect.hasAssertions();
});
});
});
6 changes: 6 additions & 0 deletions packages/middleware-sdk-s3/src/s3Configuration.ts
Expand Up @@ -32,6 +32,10 @@ export interface S3InputConfig {
* Identity provider for an S3 feature.
*/
s3ExpressIdentityProvider?: S3ExpressIdentityProvider;
/**
* Whether to use the bucket name as the endpoint for this client.
*/
bucketEndpoint?: boolean;
}

/**
Expand All @@ -54,6 +58,7 @@ export interface S3ResolvedConfig {
disableMultiregionAccessPoints: boolean;
followRegionRedirects: boolean;
s3ExpressIdentityProvider: S3ExpressIdentityProvider;
bucketEndpoint: boolean;
}

export const resolveS3Config = <T>(
Expand Down Expand Up @@ -81,5 +86,6 @@ export const resolveS3Config = <T>(
})
)
),
bucketEndpoint: input.bucketEndpoint ?? false,
};
};
6 changes: 3 additions & 3 deletions packages/middleware-sdk-s3/src/validate-bucket-name.spec.ts
Expand Up @@ -11,7 +11,7 @@ describe("validateBucketNameMiddleware", () => {
});

it("throws error if Bucket parameter contains '/'", async () => {
const handler = validateBucketNameMiddleware()(mockNextHandler, {} as any);
const handler = validateBucketNameMiddleware({} as any)(mockNextHandler, {} as any);
const bucket = "bucket/part/of/key";
let error;
try {
Expand All @@ -29,7 +29,7 @@ describe("validateBucketNameMiddleware", () => {
});

it("doesn't throw error if Bucket parameter has no '/'", async () => {
const handler = validateBucketNameMiddleware()(mockNextHandler, {} as any);
const handler = validateBucketNameMiddleware({} as any)(mockNextHandler, {} as any);
const args = {
input: {
Bucket: "bucket",
Expand All @@ -42,7 +42,7 @@ describe("validateBucketNameMiddleware", () => {

it("should not validate bucket name if the bucket name is an ARN", async () => {
mockValidateArn.mockReturnValue(true);
const handler = validateBucketNameMiddleware()(mockNextHandler, {} as any);
const handler = validateBucketNameMiddleware({} as any)(mockNextHandler, {} as any);
const args = {
input: {
Bucket: "arn:aws:s3:us-east-1:123456789012:accesspoint/myendpoint",
Expand Down
13 changes: 8 additions & 5 deletions packages/middleware-sdk-s3/src/validate-bucket-name.ts
Expand Up @@ -9,16 +9,19 @@ import {
Pluggable,
} from "@smithy/types";

import { bucketEndpointMiddleware, bucketEndpointMiddlewareOptions } from "./bucket-endpoint-middleware";
import { S3ResolvedConfig } from "./s3Configuration";

/**
* @internal
*/
export function validateBucketNameMiddleware(): InitializeMiddleware<any, any> {
export function validateBucketNameMiddleware({ bucketEndpoint }: S3ResolvedConfig): InitializeMiddleware<any, any> {
return <Output extends MetadataBearer>(next: InitializeHandler<any, Output>): InitializeHandler<any, Output> =>
async (args: InitializeHandlerArguments<any>): Promise<InitializeHandlerOutput<Output>> => {
const {
input: { Bucket },
} = args;
if (typeof Bucket === "string" && !validateArn(Bucket) && Bucket.indexOf("/") >= 0) {
if (!bucketEndpoint && typeof Bucket === "string" && !validateArn(Bucket) && Bucket.indexOf("/") >= 0) {
const err = new Error(`Bucket name shouldn't contain '/', received '${Bucket}'`);
err.name = "InvalidBucketName";
throw err;
Expand All @@ -40,9 +43,9 @@ export const validateBucketNameMiddlewareOptions: InitializeHandlerOptions = {
/**
* @internal
*/
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export const getValidateBucketNamePlugin = (unused: any): Pluggable<any, any> => ({
export const getValidateBucketNamePlugin = (options: S3ResolvedConfig): Pluggable<any, any> => ({
applyToStack: (clientStack) => {
clientStack.add(validateBucketNameMiddleware(), validateBucketNameMiddlewareOptions);
clientStack.add(validateBucketNameMiddleware(options), validateBucketNameMiddlewareOptions);
clientStack.addRelativeTo(bucketEndpointMiddleware(options), bucketEndpointMiddlewareOptions);
},
});
Expand Up @@ -116,6 +116,7 @@ export const initializeWithMaximalConfiguration = () => {
disableS3ExpressSessionAuth: false,
useGlobalEndpoint: false,
signingEscapePath: false,
bucketEndpoint: false,
};

const s3 = new S3Client(config);
Expand Down

0 comments on commit f05b540

Please sign in to comment.