From 6721c7f791159bd53189c74415948e6b58d0aaba Mon Sep 17 00:00:00 2001 From: siddsriv Date: Thu, 17 Oct 2024 01:43:49 +0000 Subject: [PATCH 1/3] chore(middleware-sdk-s3): add status code 400 for redirects --- .../src/region-redirect-middleware.spec.ts | 20 +++++++++++++++++++ .../src/region-redirect-middleware.ts | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts index b159333623ca..3b30b8827de2 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.spec.ts @@ -18,6 +18,18 @@ describe(regionRedirectMiddleware.name, () => { return null as any; }; + const next400 = (arg: any) => { + if (call === 0) { + call++; + throw Object.assign(new Error(), { + name: "IllegalLocationConstraintException", + $metadata: { httpStatusCode: 400 }, + $response: { headers: { "x-amz-bucket-region": redirectRegion } }, + }); + } + return null as any; + }; + beforeEach(() => { call = 0; }); @@ -30,6 +42,14 @@ describe(regionRedirectMiddleware.name, () => { expect(context.__s3RegionRedirect).toEqual(redirectRegion); }); + it("set S3 region redirect on context if receiving an error with status 400", async () => { + const middleware = regionRedirectMiddleware({ region, followRegionRedirects: true }); + const context = {} as HandlerExecutionContext; + const handler = middleware(next400, context); + await handler({ input: null }); + expect(context.__s3RegionRedirect).toEqual(redirectRegion); + }); + it("does not follow the redirect when followRegionRedirects is false", async () => { const middleware = regionRedirectMiddleware({ region, followRegionRedirects: false }); const context = {} as HandlerExecutionContext; diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts index 4ae0599e5e07..01986577e9d8 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts @@ -38,7 +38,7 @@ export function regionRedirectMiddleware(clientConfig: PreviouslyResolved): Init if ( clientConfig.followRegionRedirects && // err.name === "PermanentRedirect" && --> removing the error name check, as that allows for HEAD operations (which have the 301 status code, but not the same error name) to be covered for region redirection as well - err?.$metadata?.httpStatusCode === 301 + (err?.$metadata?.httpStatusCode === 301 || err?.$metadata?.httpStatusCode === 400) ) { try { const actualRegion = err.$response.headers["x-amz-bucket-region"]; From 74fd74f944b25ee5d4870660d1dcfea2b00b84d8 Mon Sep 17 00:00:00 2001 From: siddsriv Date: Thu, 17 Oct 2024 02:49:51 +0000 Subject: [PATCH 2/3] chore(middleware-sdk-s3): add error code check --- .../src/region-redirect-middleware.ts | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts index 01986577e9d8..abd78c9644de 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts @@ -35,22 +35,22 @@ export function regionRedirectMiddleware(clientConfig: PreviouslyResolved): Init try { return await next(args); } catch (err) { - if ( - clientConfig.followRegionRedirects && - // err.name === "PermanentRedirect" && --> removing the error name check, as that allows for HEAD operations (which have the 301 status code, but not the same error name) to be covered for region redirection as well - (err?.$metadata?.httpStatusCode === 301 || err?.$metadata?.httpStatusCode === 400) - ) { - try { - const actualRegion = err.$response.headers["x-amz-bucket-region"]; - context.logger?.debug(`Redirecting from ${await clientConfig.region()} to ${actualRegion}`); - context.__s3RegionRedirect = actualRegion; - } catch (e) { - throw new Error("Region redirect failed: " + e); + if (clientConfig.followRegionRedirects) { + if ( + err?.$metadata?.httpStatusCode === 301 || + (err?.$metadata?.httpStatusCode === 400 && err?.name === "IllegalLocationConstraintException") + ) { + try { + const actualRegion = err.$response.headers["x-amz-bucket-region"]; + context.logger?.debug(`Redirecting from ${await clientConfig.region()} to ${actualRegion}`); + context.__s3RegionRedirect = actualRegion; + } catch (e) { + throw new Error("Region redirect failed: " + e); + } + return next(args); } - return next(args); - } else { - throw err; } + throw err; } }; } From ae4961524448db39085f29d4286738ca8ed29470 Mon Sep 17 00:00:00 2001 From: siddsriv Date: Thu, 17 Oct 2024 02:51:18 +0000 Subject: [PATCH 3/3] chore(middleware-sdk-s3): re-add comment for PermanentRedirect --- packages/middleware-sdk-s3/src/region-redirect-middleware.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts index abd78c9644de..7a9329573a75 100644 --- a/packages/middleware-sdk-s3/src/region-redirect-middleware.ts +++ b/packages/middleware-sdk-s3/src/region-redirect-middleware.ts @@ -38,6 +38,7 @@ export function regionRedirectMiddleware(clientConfig: PreviouslyResolved): Init if (clientConfig.followRegionRedirects) { if ( err?.$metadata?.httpStatusCode === 301 || + // err.name === "PermanentRedirect" && --> removing the error name check, as that allows for HEAD operations (which have the 301 status code, but not the same error name) to be covered for region redirection as well (err?.$metadata?.httpStatusCode === 400 && err?.name === "IllegalLocationConstraintException") ) { try {