fix(middleware-sdk-s3): improve error message on stream upload (#3571)

* fix(middleware-sdk-s3): improve error message on stream upload

* feat(middleware-sdk-s3): add warning when calling PutObject with unknown content length

* fix(middleware-sdk-s3): correct middleware name

Author: kuhe

Diff for: .gitignore

@@ -36,6 +36,7 @@ codegen/sdk-codegen/smithy-build.json
 .gradle
 */out/
 */*/out/
+workspace
 
 coverage
 dist

Diff for: clients/client-s3/src/commands/PutObjectCommand.ts

@@ -1,6 +1,7 @@
 // smithy-typescript generated code
 import { getBucketEndpointPlugin } from "@aws-sdk/middleware-bucket-endpoint";
 import { getFlexibleChecksumsPlugin } from "@aws-sdk/middleware-flexible-checksums";
+import { getCheckContentLengthHeaderPlugin } from "@aws-sdk/middleware-sdk-s3";
 import { getSerdePlugin } from "@aws-sdk/middleware-serde";
 import { getSsecPlugin } from "@aws-sdk/middleware-ssec";
 import { HttpRequest as __HttpRequest, HttpResponse as __HttpResponse } from "@aws-sdk/protocol-http";
@@ -173,6 +174,7 @@ export class PutObjectCommand extends $Command<PutObjectCommandInput, PutObjectC
     options?: __HttpHandlerOptions
   ): Handler<PutObjectCommandInput, PutObjectCommandOutput> {
     this.middlewareStack.use(getSerdePlugin(configuration, this.serialize, this.deserialize));
+    this.middlewareStack.use(getCheckContentLengthHeaderPlugin(configuration));
     this.middlewareStack.use(getSsecPlugin(configuration));
     this.middlewareStack.use(getBucketEndpointPlugin(configuration));
     this.middlewareStack.use(

Diff for: codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3Config.java

@@ -162,6 +162,11 @@ public List<RuntimeClientPlugin> getClientPlugins() {
                              HAS_MIDDLEWARE)
                         .servicePredicate((m, s) -> testServiceId(s))
                         .build(),
+                RuntimeClientPlugin.builder()
+                        .withConventions(AwsDependency.S3_MIDDLEWARE.dependency, "CheckContentLengthHeader",
+                                HAS_MIDDLEWARE)
+                        .operationPredicate((m, s, o) -> testServiceId(s) && o.getId().getName(s).equals("PutObject"))
+                        .build(),
                 RuntimeClientPlugin.builder()
                         .withConventions(AwsDependency.S3_MIDDLEWARE.dependency, "UseRegionalEndpoint",
                                         HAS_MIDDLEWARE)

Diff for: packages/middleware-sdk-s3/src/check-content-length-header.spec.ts

@@ -0,0 +1,90 @@
+import { checkContentLengthHeader } from "./check-content-length-header";
+
+describe("checkContentLengthHeaderMiddleware", () => {
+  const mockNextHandler = jest.fn();
+
+  let spy;
+
+  beforeEach(() => {
+    spy = jest.spyOn(console, "warn");
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it("warns if uploading a payload of unknown length", async () => {
+    const handler = checkContentLengthHeader()(mockNextHandler, {});
+
+    await handler({
+      request: {
+        method: null,
+        protocol: null,
+        hostname: null,
+        path: null,
+        query: {},
+        headers: {},
+      },
+      input: {},
+    });
+
+    expect(spy).toHaveBeenCalledWith(
+      "Are you using a Stream of unknown length as the Body of a PutObject request? Consider using Upload instead from @aws-sdk/lib-storage."
+    );
+  });
+
+  it("uses the context logger if available", async () => {
+    const context = {
+      logger: {
+        called: false,
+        calledWith: "",
+        warn(msg) {
+          this.called = true;
+          this.calledWith = msg;
+        },
+        debug() {},
+        info() {},
+        error() {},
+      },
+    };
+    const handler = checkContentLengthHeader()(mockNextHandler, context);
+
+    await handler({
+      request: {
+        method: null,
+        protocol: null,
+        hostname: null,
+        path: null,
+        query: {},
+        headers: {},
+      },
+      input: {},
+    });
+
+    expect(spy).not.toHaveBeenCalled();
+    expect(context.logger.called).toBe(true);
+    expect(context.logger.calledWith).toEqual(
+      "Are you using a Stream of unknown length as the Body of a PutObject request? Consider using Upload instead from @aws-sdk/lib-storage."
+    );
+  });
+
+  it("does not warn if uploading a payload of known length", async () => {
+    const handler = checkContentLengthHeader()(mockNextHandler, {});
+
+    await handler({
+      request: {
+        method: null,
+        protocol: null,
+        hostname: null,
+        path: null,
+        query: {},
+        headers: {
+          "content-length": "5",
+        },
+      },
+      input: {},
+    });
+
+    expect(spy).not.toHaveBeenCalled();
+  });
+});

Diff for: packages/middleware-sdk-s3/src/check-content-length-header.ts

@@ -0,0 +1,62 @@
+import { HttpRequest } from "@aws-sdk/protocol-http";
+import {
+  FinalizeHandler,
+  FinalizeHandlerArguments,
+  FinalizeHandlerOutput,
+  FinalizeRequestHandlerOptions,
+  FinalizeRequestMiddleware,
+  HandlerExecutionContext,
+  MetadataBearer,
+  Pluggable,
+} from "@aws-sdk/types";
+
+const CONTENT_LENGTH_HEADER = "content-length";
+
+/**
+ * @internal
+ *
+ * Log a warning if the input to PutObject is detected to be a Stream of unknown ContentLength and
+ * recommend the usage of the @aws-sdk/lib-storage Upload class.
+ */
+export function checkContentLengthHeader(): FinalizeRequestMiddleware<any, any> {
+  return <Output extends MetadataBearer>(
+      next: FinalizeHandler<any, Output>,
+      context: HandlerExecutionContext
+    ): FinalizeHandler<any, Output> =>
+    async (args: FinalizeHandlerArguments<any>): Promise<FinalizeHandlerOutput<Output>> => {
+      const { request } = args;
+
+      if (HttpRequest.isInstance(request)) {
+        if (!request.headers[CONTENT_LENGTH_HEADER]) {
+          const message = `Are you using a Stream of unknown length as the Body of a PutObject request? Consider using Upload instead from @aws-sdk/lib-storage.`;
+          if (typeof context?.logger?.warn === "function") {
+            context.logger.warn(message);
+          } else {
+            console.warn(message);
+          }
+        }
+      }
+
+      return next({ ...args });
+    };
+}
+
+/**
+ * @internal
+ */
+export const checkContentLengthHeaderMiddlewareOptions: FinalizeRequestHandlerOptions = {
+  step: "finalizeRequest",
+  tags: ["CHECK_CONTENT_LENGTH_HEADER"],
+  name: "getCheckContentLengthHeaderPlugin",
+  override: true,
+};
+
+/**
+ * @internal
+ */
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+export const getCheckContentLengthHeaderPlugin = (unused: any): Pluggable<any, any> => ({
+  applyToStack: (clientStack) => {
+    clientStack.add(checkContentLengthHeader(), checkContentLengthHeaderMiddlewareOptions);
+  },
+});

Diff for: packages/middleware-sdk-s3/src/index.ts

@@ -1,3 +1,4 @@
+export * from "./check-content-length-header";
 export * from "./throw-200-exceptions";
 export * from "./use-regional-endpoint";
 export * from "./validate-bucket-name";