Merge pull request #2392 from jerch/fix_2389

fix several bugs in Params

src/common/parser/EscapeSequenceParser.ts

@@ -543,20 +543,16 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
           break;
         case ParserAction.PARAM:
           // inner loop: digits (0x30 - 0x39) and ; (0x3b) and : (0x3a)
-          let isSub = false;
           do {
             switch (code) {
               case 0x3b:
                 params.addParam(0);  // ZDM
-                isSub = false;
                 break;
               case 0x3a:
                 params.addSubParam(-1);
-                isSub = true;
                 break;
               default:  // 0x30 - 0x39
-                if (isSub) params.addSubParamDigit(code - 48);
-                else params.addParamDigit(code - 48);
+                params.addDigit(code - 48);
             }
           } while (++i < length && (code = data[i]) > 0x2f && code < 0x3c);
           i--;

src/common/parser/Params.test.ts

@@ -16,27 +16,29 @@ class TestParams extends Params {
 }
 
 /** `Params` parser shim */
-function parse(params: Params, s: string): void {
+function parse(params: Params, s: string | string[]): void {
   params.reset();
   params.addParam(0);
-  let isSub = false;
-  for (let i = 0; i < s.length; ++i) {
-    let code = s.charCodeAt(i);
-    do {
-      switch (code) {
-        case 0x3b:
-          params.addParam(0);
-          isSub = false;
-          break;
-        case 0x3a:
-          params.addSubParam(-1);
-          isSub = true;
-          break;
-        default:  // 0x30 - 0x39
-          if (isSub) params.addSubParamDigit(code - 48);
-          else params.addParamDigit(code - 48);
-      }
-    } while (++i < s.length && (code = s.charCodeAt(i)) > 0x2f && code < 0x3c);
+  if (typeof s === 'string') {
+    s = [s];
+  }
+  for (const chunk of s) {
+    for (let i = 0; i < chunk.length; ++i) {
+      let code = chunk.charCodeAt(i);
+      do {
+        switch (code) {
+          case 0x3b:
+            params.addParam(0);
+            break;
+          case 0x3a:
+            params.addSubParam(-1);
+            break;
+          default:  // 0x30 - 0x39
+            params.addDigit(code - 48);
+        }
+      } while (++i < s.length && (code = chunk.charCodeAt(i)) > 0x2f && code < 0x3c);
+      i--;
+    }
   }
 }
 
@@ -217,4 +219,19 @@ describe('Params', () => {
       assert.deepEqual(params.toArray(), [0, [0x7FFFFFFF]]);
     });
   });
+  describe('issue 2389', () => {
+    it('should cancel subdigits if beyond params limit', () => {
+      const params = new Params();
+      parse(params, ';;;;;;;;;10;;;;;;;;;;20;;;;;;;;;;30;31;32;33;34;35::::::::');
+      assert.deepEqual(params.toArray(), [
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 10,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 20,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 30, 31, 32]);
+    });
+    it('should carry forward isSub state', () => {
+      const params = new Params();
+      parse(params, ['1:22:33', '44']);
+      assert.deepEqual(params.toArray(), [1, [22, 3344]]);
+    });
+  });
 });

src/common/parser/Params.ts

@@ -41,6 +41,7 @@ export class Params implements IParams {
   private _subParamsIdx: Uint16Array;
   private _rejectDigits: boolean;
   private _rejectSubDigits: boolean;
+  private _digitIsSub: boolean;
 
   /**
    * Create a `Params` type from JS array representation.
@@ -79,6 +80,7 @@ export class Params implements IParams {
     this._subParamsIdx = new Uint16Array(maxLength);
     this._rejectDigits = false;
     this._rejectSubDigits = false;
+    this._digitIsSub = false;
   }
 
   /**
@@ -91,6 +93,9 @@ export class Params implements IParams {
     newParams._subParams.set(this._subParams);
     newParams._subParamsLength = this._subParamsLength;
     newParams._subParamsIdx.set(this._subParamsIdx);
+    newParams._rejectDigits = this._rejectDigits;
+    newParams._rejectSubDigits = this._rejectSubDigits;
+    newParams._digitIsSub = this._digitIsSub;
     return newParams;
   }
 
@@ -121,6 +126,7 @@ export class Params implements IParams {
     this._subParamsLength = 0;
     this._rejectDigits = false;
     this._rejectSubDigits = false;
+    this._digitIsSub = false;
   }
 
   /**
@@ -131,6 +137,7 @@ export class Params implements IParams {
    * store up to 30.
    */
   public addParam(value: number): void {
+    this._digitIsSub = false;
     if (this.length >= this.maxLength) {
       this._rejectDigits = true;
       return;
@@ -150,10 +157,11 @@ export class Params implements IParams {
    * sub parameter will be ignored.
    */
   public addSubParam(value: number): void {
+    this._digitIsSub = true;
     if (!this.length) {
       return;
     }
-    if (this._subParamsLength >= this.maxSubParamsLength) {
+    if (this._rejectDigits || this._subParamsLength >= this.maxSubParamsLength) {
       this._rejectSubDigits = true;
       return;
     }
@@ -204,30 +212,18 @@ export class Params implements IParams {
   /**
    * Add a single digit value to current parameter.
    * This is used by the parser to account digits on a char by char basis.
-   * Do not use this method directly, consider using `addParam` instead.
    */
-  public addParamDigit(value: number): void {
-    if (this._rejectDigits) {
+  public addDigit(value: number): void {
+    let length;
+    if (this._rejectDigits
+      || !(length = this._digitIsSub ? this._subParamsLength : this.length)
+      || (this._digitIsSub && this._rejectSubDigits)
+    ) {
       return;
     }
-    const v = this.params[this.length - 1] * 10 + value;
-    this.params[this.length - 1] = v > MAX_VALUE ? MAX_VALUE : v;
-  }
 
-  /**
-   * Add a single digit value to current sub parameter.
-   * This is used by the parser to account digits on a char by char basis.
-   * Do not use this method directly, consider using `addSubParam` instead.
-   */
-  public addSubParamDigit(value: number): void {
-    if (!this._subParamsLength || this._rejectDigits || this._rejectSubDigits) {
-      return;
-    }
-    if (this._subParams[this._subParamsLength - 1] === -1) {
-      this._subParams[this._subParamsLength - 1] = value;
-    } else {
-      const v = this._subParams[this._subParamsLength - 1] * 10 + value;
-      this._subParams[this._subParamsLength - 1] = v > MAX_VALUE ? MAX_VALUE : v;
-    }
+    const store = this._digitIsSub ? this._subParams : this.params;
+    const cur = store[length - 1];
+    store[length - 1] = ~cur ? Math.min(cur * 10 + value, MAX_VALUE) : value;
   }
 }