Merge branch 'JsMqtt-9-5' into JsMqtt-10-ManualAck

Author: Bret Ambrose

lib/browser/mqtt5.spec.ts

@@ -335,40 +335,17 @@ function createOperationFailureClient() : mqtt5.IMqtt5Client {
     return new mqtt5.Mqtt5Client(createWsIotCoreClientConfig());
 }
 
-/* Variant of the shared version that doesn't expect the stop call to throw */
-async function testDisconnectValidationFailure(client : mqtt5.Mqtt5Client, sessionExpiry: number) {
-    let connectionSuccess = once(client, mqtt5.Mqtt5Client.CONNECTION_SUCCESS);
-    let disconnection = once(client, mqtt5.Mqtt5Client.DISCONNECTION);
-
-    client.start();
-
-    await connectionSuccess;
-
-    client.stop({
-        reasonCode: mqtt5.DisconnectReasonCode.NormalDisconnection,
-        sessionExpiryIntervalSeconds: sessionExpiry
-    });
-
-    // even though validation fails, we should still see the connection drop (on failure result handler)
-    await disconnection;
-
-    let stopped = once(client, mqtt5.Mqtt5Client.STOPPED);
-
-    client.stop();
-    await stopped;
-
-    client.close();
-}
-
 test_utils.conditional_test(test_utils.ClientEnvironmentalConfig.hasIotCoreEnvironment())('Disconnection failure - session expiry underflow', async () => {
     await retry.networkTimeoutRetryWrapper( async () => {
-        await testDisconnectValidationFailure(createOperationFailureClient() as mqtt5.Mqtt5Client, -5);
+        // @ts-ignore
+        await test_utils.testDisconnectValidationFailure(createOperationFailureClient() as mqtt5.Mqtt5Client, -5);
     })
 });
 
 test_utils.conditional_test(test_utils.ClientEnvironmentalConfig.hasIotCoreEnvironment())('Disconnection failure - session expiry overflow', async () => {
     await retry.networkTimeoutRetryWrapper( async () => {
-        await testDisconnectValidationFailure(createOperationFailureClient() as mqtt5.Mqtt5Client, 4294967296);
+        // @ts-ignore
+        await test_utils.testDisconnectValidationFailure(createOperationFailureClient() as mqtt5.Mqtt5Client, 4294967296);
     })
 });
 

lib/browser/mqtt5.ts

@@ -21,6 +21,7 @@ import {CrtError} from "./error";
 import * as ws from "./ws";
 import * as mqtt_shared from "../common/mqtt_shared";
 import * as auth from "./auth";
+import * as validate from "./mqtt_internal/validate";
 
 export * from "../common/mqtt5";
 export * from '../common/mqtt5_packet';
@@ -374,6 +375,13 @@ export class Mqtt5Client extends BufferedEventEmitter implements mqtt5.IMqtt5Cli
      * @param disconnectPacket (optional) properties of a DISCONNECT packet to send as part of the shutdown process
      */
     stop(disconnectPacket?: mqtt5_packet.DisconnectPacket) {
+        // maintain backwards comaptibility with previous implementation which let validation errors
+        // trump the stop request
+        if (disconnectPacket) {
+            disconnectPacket.type = mqtt5_packet.PacketType.Disconnect;
+            validate.validateInitialOutboundPacket(disconnectPacket, internal_mqtt_client.ProtocolMode.Mqtt5);
+        }
+
         this.internalClient.stop(disconnectPacket);
     }
 

lib/browser/mqtt_internal/client.ts

@@ -246,30 +246,6 @@ function buildClientConfigLogString(prefix: string, config: ClientConfig) : stri
     return result;
 }
 
-function validateUnsignedInteger(value: number, fieldName: string) {
-    if (!Number.isInteger(value) || value < 0) {
-        throw new CrtError(`Field "${fieldName}" with value "${value}" is not a valid unsigned integer`);
-    }
-}
-
-function validateOptionalUnsignedInteger(value: number | undefined, fieldName: string) {
-    if (value === undefined) {
-        return;
-    }
-
-    validateUnsignedInteger(value, fieldName);
-}
-
-function validateOptionalPositiveU32(value: number | undefined, fieldName: string) {
-    if (value === undefined) {
-        return;
-    }
-
-    if (!Number.isInteger(value) || value <= 0 || value > (256 * 256 * 256 * 256 - 1)) {
-        throw new CrtError(`Field "${fieldName}" with value "${value}" is not a valid positive 32 bit integer`);
-    }
-}
-
 function validateConnectOptions(options : ConnectOptions, mode : ProtocolMode) {
     if (options.connectPacketTransformer && (typeof options.connectPacketTransformer !== "function")) {
         throw new CrtError("connectPacketTransformer must be a function");
@@ -291,7 +267,7 @@ function validateConnectOptions(options : ConnectOptions, mode : ProtocolMode) {
         validate.validateU16(options.receiveMaximum, "receiveMaximum");
     }
 
-    validateOptionalPositiveU32(options.maximumPacketSizeBytes, "maximumPacketSizeBytes");
+    validate.validateOptionalPositiveU32(options.maximumPacketSizeBytes, "maximumPacketSizeBytes");
     validate.validateOptionalU32(options.willDelayIntervalSeconds, "willDelayIntervalSeconds");
 
     if (options.will !== undefined) {
@@ -312,21 +288,21 @@ function validateConfig(config: ClientConfig) {
 
     validateConnectOptions(config.connectOptions, config.protocolVersion);
 
-    validateOptionalUnsignedInteger(config.pingTimeoutMillis, "pingTimeoutMillis");
+    validate.validateOptionalUnsignedInteger(config.pingTimeoutMillis, "pingTimeoutMillis");
 
     if (!config.connectionFactory || (typeof config.connectionFactory !== "function")) {
         throw new CrtError("connectionFactory must be a valid function");
     }
 
-    validateUnsignedInteger(config.connectTimeoutMillis, "connectTimeoutMillis");
+    validate.validateUnsignedInteger(config.connectTimeoutMillis, "connectTimeoutMillis");
 
     if (config.retryJitterMode !== undefined && mqtt5.RetryJitterType[config.retryJitterMode] == undefined) {
         throw new CrtError("Invalid value for retryJitterMode");
     }
 
-    validateOptionalUnsignedInteger(config.minReconnectDelayMs, "minReconnectDelayMs");
-    validateOptionalUnsignedInteger(config.maxReconnectDelayMs, "maxReconnectDelayMs");
-    validateOptionalUnsignedInteger(config.resetConnectionFailureCountMillis, "resetConnectionFailureCountMillis");
+    validate.validateOptionalUnsignedInteger(config.minReconnectDelayMs, "minReconnectDelayMs");
+    validate.validateOptionalUnsignedInteger(config.maxReconnectDelayMs, "maxReconnectDelayMs");
+    validate.validateOptionalUnsignedInteger(config.resetConnectionFailureCountMillis, "resetConnectionFailureCountMillis");
 
     if (config.resubscribeMode !== undefined && ResubscribeModeType[config.resubscribeMode] == undefined) {
         throw new CrtError("Invalid value for resubscribeMode");

lib/browser/mqtt_internal/validate.ts

@@ -170,6 +170,30 @@ function validateOptionalVli(value: number | undefined, fieldName: string) {
     }
 }
 
+export function validateUnsignedInteger(value: number, fieldName: string) {
+    if (!Number.isInteger(value) || value < 0) {
+        throw new CrtError(`Field "${fieldName}" with value "${value}" is not a valid unsigned integer`);
+    }
+}
+
+export function validateOptionalUnsignedInteger(value: number | undefined, fieldName: string) {
+    if (value === undefined) {
+        return;
+    }
+
+    validateUnsignedInteger(value, fieldName);
+}
+
+export function validateOptionalPositiveU32(value: number | undefined, fieldName: string) {
+    if (value === undefined) {
+        return;
+    }
+
+    if (!Number.isInteger(value) || value <= 0 || value > (256 * 256 * 256 * 256 - 1)) {
+        throw new CrtError(`Field "${fieldName}" with value "${value}" is not a valid positive 32 bit integer`);
+    }
+}
+
 // we don't validate length here because we don't know encoding length without doing a utf-8 conversion.
 // This means we validate string length in the internal validators which checks ArrayBuffer lengths.
 function validateString(value: any, fieldName: string) {