Merge branch 'main' into RequestResponseBrowserStreaming

.github/ISSUE_TEMPLATE/bug-report.yml

@@ -12,6 +12,14 @@ body:
       description: What is the problem? A clear and concise description of the bug.
     validations:
       required: true
+  - type: checkboxes
+    id: regression
+    attributes:
+      label: Regression Issue
+      description: What is a regression? If it worked in a previous version but doesn't in the latest version, it's considered a regression. In this case, please provide specific version number in the report.
+      options:
+        - label: Select this option if this issue appears to be a regression.
+          required: false
   - type: textarea
     id: expected
     attributes:

.github/workflows/ci.yml

@@ -167,7 +167,7 @@ jobs:
   check-docs:
     runs-on: ubuntu-20.04 # latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
         with:
           submodules: true
       - name: Check docs
@@ -179,7 +179,7 @@ jobs:
     runs-on: ubuntu-20.04 # latest
     steps:
     - name: Checkout Source
-      uses: actions/checkout@v2
+      uses: actions/checkout@v4
       with:
         submodules: true
         fetch-depth: 0
@@ -191,7 +191,7 @@ jobs:
   check-lockfile-version:
     runs-on: ubuntu-20.04 # latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
         with:
           fetch-depth: 0
       - name: Check for edits to package-lock.json

.github/workflows/issue-regression-labeler.yml

@@ -0,0 +1,32 @@
+# Apply potential regression label on issues
+name: issue-regression-label
+on:
+  issues:
+    types: [opened, edited]
+jobs:
+  add-regression-label:
+    runs-on: ubuntu-latest
+    permissions:
+      issues: write
+    steps:
+    - name: Fetch template body
+      id: check_regression
+      uses: actions/github-script@v7
+      env: 
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        TEMPLATE_BODY: ${{ github.event.issue.body }}
+      with:
+        script: |
+          const regressionPattern = /\[x\] Select this option if this issue appears to be a regression\./i;
+          const template = `${process.env.TEMPLATE_BODY}`
+          const match = regressionPattern.test(template);
+          core.setOutput('is_regression', match);
+    - name: Manage regression label
+      env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      run: |
+        if [ "${{ steps.check_regression.outputs.is_regression }}" == "true" ]; then
+          gh issue edit ${{ github.event.issue.number }} --add-label "potential-regression" -R ${{ github.repository }}
+        else
+          gh issue edit ${{ github.event.issue.number }} --remove-label "potential-regression" -R ${{ github.repository }}
+        fi

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.1)
+cmake_minimum_required(VERSION 3.9)
 project(aws-crt-nodejs C)
 option(BUILD_DEPS "Builds aws common runtime dependencies as part of build, only do this if you don't want to control your dependency chain." ON)
 
@@ -45,7 +45,7 @@ if (BUILD_DEPS)
     if (UNIX AND NOT APPLE)
         include(AwsPrebuildDependency)
         # s2n-tls uses libcrypto during its configuration, so we need to prebuild aws-lc.
-        prebuild_dependency(
+        aws_prebuild_dependency(
             DEPENDENCY_NAME AWSLC
             SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/crt/aws-lc
             CMAKE_ARGUMENTS
@@ -54,8 +54,14 @@ if (BUILD_DEPS)
                 -DBUILD_LIBSSL=OFF
                 -DBUILD_TESTING=OFF
         )
-        set(UNSAFE_TREAT_WARNINGS_AS_ERRORS OFF)
-        add_subdirectory(crt/s2n)
+        # prebuild s2n-tls.
+        aws_prebuild_dependency(
+            DEPENDENCY_NAME S2N
+            SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/crt/s2n
+            CMAKE_ARGUMENTS
+                -DUNSAFE_TREAT_WARNINGS_AS_ERRORS=OFF
+                -DBUILD_TESTING=OFF
+        )
     endif()
 
     add_subdirectory(crt/aws-c-sdkutils)
@@ -74,10 +80,6 @@ else()
     set(IN_SOURCE_BUILD OFF)
 endif()
 
-if (POLICY CMP0069)
-    cmake_policy(SET CMP0069 NEW) # Enable LTO/IPO if available in the compiler, see AwsCFlags
-endif()
-
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_PREFIX_PATH}/${CMAKE_INSTALL_LIBDIR}/cmake")
 
 include(AwsCFlags)

lib/browser/crypto.ts

@@ -12,25 +12,35 @@
  */
 
 import * as Crypto from "crypto-js";
-import { fromUtf8 } from "@aws-sdk/util-utf8-browser";
 import { Hashable } from "../common/crypto";
 
 export { Hashable } from "../common/crypto";
 
 /**
- * Object that allows for continuous MD5 hashing of data.
- *
- * @category Crypto
+ * CryptoJS does not provide easy access to underlying bytes.
+ * As a workaround just dump it to a string and then reinterpret chars as individual bytes.
+ * Note: we are using Latin1 here because its a static sized 8 bit encoding so each char maps directly to a byte value.
+ * TODO: long term we would probably want to move to WebCrypto for SHA's and some other 3p for crc's and md5.
+ * @param hash 
+ * @returns 
  */
-export class Md5Hash {
-    private hash?: Crypto.WordArray;
+function wordArrayToUint8Array(hash: Crypto.WordArray) {
+    return Uint8Array.from(hash.toString(Crypto.enc.Latin1).split('').map(c => c.charCodeAt(0)));;
+}
+
+class BaseHash {
+    private hasher : any;
+
+    constructor(hasher: any) {
+        this.hasher = hasher;
+    }
 
     /**
      * Hashes additional data
      * @param data Additional data to hash
      */
     update(data: Hashable) {
-        this.hash = Crypto.MD5(data.toString(), this.hash ? this.hash.toString() : undefined);
+        this.hasher.update(data.toString());
     }
 
     /**
@@ -41,10 +51,20 @@ export class Md5Hash {
      * @returns the final hash digest
      */
     finalize(truncate_to?: number): DataView {
-        const digest = this.hash ? this.hash.toString() : '';
-        const truncated = digest.substring(0, truncate_to ? truncate_to : digest.length);
-        const bytes = fromUtf8(truncated);
-        return new DataView(bytes.buffer);
+        const hashBuffer = wordArrayToUint8Array(this.hasher.finalize()) ;
+        const truncated = hashBuffer.slice(0, truncate_to ? truncate_to : hashBuffer.length);
+        return new DataView(truncated.buffer);;
+    }
+}
+
+/**
+ * Object that allows for continuous MD5 hashing of data.
+ *
+ * @category Crypto
+ */
+export class Md5Hash extends BaseHash {
+    constructor() {
+        super(Crypto.algo.MD5.create());
     }
 }
 
@@ -71,29 +91,9 @@ export function hash_md5(data: Hashable, truncate_to?: number): DataView {
  *
  * @category Crypto
  */
-export class Sha256Hash {
-    private hash?: Crypto.WordArray;
-
-    /**
-     * Hashes additional data
-     * @param data Additional data to hash
-     */
-    update(data: Hashable) {
-        this.hash = Crypto.SHA256(data.toString(), this.hash ? this.hash.toString() : undefined);
-    }
-
-    /**
-     * Completes the hash computation and returns the final hash digest.
-     *
-     * @param truncate_to The maximum number of bytes to receive. Leave as undefined or 0 to receive the entire digest.
-     *
-     * @returns the final hash digest
-     */
-    finalize(truncate_to?: number): DataView {
-        const digest = this.hash ? this.hash.toString() : '';
-        const truncated = digest.substring(0, truncate_to ? truncate_to : digest.length);
-        const bytes = fromUtf8(truncated);
-        return new DataView(bytes.buffer);
+export class Sha256Hash extends BaseHash {
+    constructor() {
+        super(Crypto.algo.SHA256.create());
     }
 }
 
@@ -109,40 +109,19 @@ export class Sha256Hash {
  * @category Crypto
  */
 export function hash_sha256(data: Hashable, truncate_to?: number): DataView {
-    const digest = Crypto.SHA256(data.toString()).toString();
-    const truncated = digest.substring(0, truncate_to ? truncate_to : digest.length);
-    const bytes = fromUtf8(truncated);
-    return new DataView(bytes.buffer);
+    const sha256 = new Sha256Hash();
+    sha256.update(data);
+    return sha256.finalize(truncate_to);
 }
 
 /**
  * Object that allows for continuous SHA1 hashing of data.
  *
  * @category Crypto
  */
- export class Sha1Hash {
-    private hash?: Crypto.WordArray;
-
-    /**
-     * Hashes additional data
-     * @param data Additional data to hash
-     */
-    update(data: Hashable) {
-        this.hash = Crypto.SHA1(data.toString(), this.hash ? this.hash.toString() : undefined);
-    }
-
-    /**
-     * Completes the hash computation and returns the final hash digest.
-     *
-     * @param truncate_to The maximum number of bytes to receive. Leave as undefined or 0 to receive the entire digest.
-     *
-     * @returns the final hash digest
-     */
-    finalize(truncate_to?: number): DataView {
-        const digest = this.hash ? this.hash.toString() : '';
-        const truncated = digest.substring(0, truncate_to ? truncate_to : digest.length);
-        const bytes = fromUtf8(truncated);
-        return new DataView(bytes.buffer);
+ export class Sha1Hash extends BaseHash {
+    constructor() {
+        super(Crypto.algo.SHA1.create());
     }
 }
 
@@ -158,49 +137,24 @@ export function hash_sha256(data: Hashable, truncate_to?: number): DataView {
  * @category Crypto
  */
 export function hash_sha1(data: Hashable, truncate_to?: number): DataView {
-    const digest = Crypto.SHA1(data.toString()).toString();
-    const truncated = digest.substring(0, truncate_to ? truncate_to : digest.length);
-    const bytes = fromUtf8(truncated);
-    return new DataView(bytes.buffer);
+    const sha1 = new Sha1Hash();
+    sha1.update(data);
+    return sha1.finalize(truncate_to);
 }
 
 /**
  * Object that allows for continuous hashing of data with an hmac secret.
  *
  * @category Crypto
  */
-export class Sha256Hmac {
-    private hmac: any;
-
+export class Sha256Hmac extends BaseHash {
     /**
      * Constructor for the Sha256Hmac class type
      * @param secret secret key to seed the hmac process with
      */
     constructor(secret: Hashable) {
         // @ts-ignore types file doesn't have this signature of create()
-        this.hmac = Crypto.algo.HMAC.create(Crypto.algo.SHA256, secret);
-    }
-
-    /**
-     * Hashes additional data
-     * @param data Additional data to hash
-     */
-    update(data: Hashable) {
-        this.hmac.update(data.toString());
-    }
-
-    /**
-     * Completes the hash computation and returns the final hmac digest.
-     *
-     * @param truncate_to The maximum number of bytes to receive. Leave as undefined or 0 to receive the entire digest.
-     *
-     * @returns the final hmac digest
-     */
-    finalize(truncate_to?: number): DataView {
-        const digest = this.hmac.finalize();
-        const truncated = digest.toString().substring(0, truncate_to ? truncate_to : digest.length);
-        const bytes = fromUtf8(truncated);
-        return new DataView(bytes.buffer);
+        super(Crypto.algo.HMAC.create(Crypto.algo.SHA256, secret));
     }
 }
 

lib/browser/mqtt.ts

@@ -36,7 +36,7 @@ import {
     OnConnectionFailedResult,
     OnConnectionClosedResult
 } from "../common/mqtt";
-import { normalize_payload } from "../common/mqtt_shared";
+import {normalize_payload, normalize_payload_to_buffer} from "../common/mqtt_shared";
 
 export {
     QoS, Payload, MqttRequest, MqttSubscribeRequest, MqttWill, OnMessageCallback, MqttConnectionConnected, MqttConnectionDisconnected,
@@ -310,7 +310,7 @@ export class MqttClientConnection extends BufferedEventEmitter {
 
         const will = this.config.will ? {
             topic: this.config.will.topic,
-            payload: normalize_payload(this.config.will.payload),
+            payload: normalize_payload_to_buffer(this.config.will.payload),
             qos: this.config.will.qos,
             retain: this.config.will.retain,
         } : undefined;
@@ -576,7 +576,18 @@ export class MqttClientConnection extends BufferedEventEmitter {
                     return this.on_error(error);
                 }
                 const sub = (packet as mqtt.ISubscriptionGrant[])[0];
-                resolve({ topic: sub.topic, qos: sub.qos });
+
+                /*
+                 * 128 is not modeled in QoS, either on our side nor mqtt-js's side.
+                 * We have always passed this 128 to the user and it is not reasonable to extend
+                 * our output type with 128 since it's also our input type and we don't want anyone
+                 * to pass 128 to us.
+                 *
+                 * The 5 client solves this by making the output type a completely separate enum.
+                 *
+                 * By doing this cast, we make the type checker ignore this edge case.
+                 */
+                resolve({ topic: sub.topic, qos: sub.qos as QoS });
             });
         });
     }

lib/browser/mqtt5.ts

@@ -898,4 +898,4 @@ export class Mqtt5Client extends BufferedEventEmitter implements mqtt5.IMqtt5Cli
             this.emit(Mqtt5Client.MESSAGE_RECEIVED, messageReceivedEvent);
         }, 0);
     }
-}
+}