Fixed: Fix CSI parameters parsing like for SGR sequences that start w…

…ith a `;` or have sequential `;` characters https://vt100.net/docs/vt510-rm/chapter4.html#S4.3.3 https://en.wikipedia.org/wiki/ANSI_escape_code#CSI_(Control_Sequence_Introducer)_sequences Credits for finding the issue belongs to @Screwtapello mawww/kakoune#4339 (comment) Closes #2272, Closes mawww/kakoune#4339
termux · Jun 15, 2022 · 4c47f4f · 4c47f4f
1 parent 26ff978
commit 4c47f4f
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 20 deletions.
diff --git a/terminal-emulator/src/main/java/com/termux/terminal/TerminalEmulator.java b/terminal-emulator/src/main/java/com/termux/terminal/TerminalEmulator.java
@@ -126,6 +126,10 @@ public final class TerminalEmulator {
     private String mTitle;
     private final Stack<String> mTitleStack = new Stack<>();
 
+    /** If processing first character of first parameter of {@link #ESC_CSI}. */
+    private boolean mIsCSIStart;
+    /** The last character processed of a parameter of {@link #ESC_CSI}. */
+    private Integer mLastCSIArg;
 
     /** The cursor position. Between (0,0) and (mRows-1, mColumns-1). */
     private int mCursorRow, mCursorCol;
@@ -1386,6 +1390,8 @@ private void doEsc(int b) {
                 break;
             case '[':
                 continueSequence(ESC_CSI);
+                mIsCSIStart = true;
+                mLastCSIArg = null;
                 break;
             case '=': // DECKPAM
                 setDecsetinternalBit(DECSET_BIT_APPLICATION_KEYPAD, true);
@@ -2093,28 +2099,55 @@ private void scrollDownOneLine() {
         }
     }
 
-    /** Process the next ASCII character of a parameter. */
-    private void parseArg(int b) {
-        if (b >= '0' && b <= '9') {
-            if (mArgIndex < mArgs.length) {
-                int oldValue = mArgs[mArgIndex];
-                int thisDigit = b - '0';
-                int value;
-                if (oldValue >= 0) {
-                    value = oldValue * 10 + thisDigit;
-                } else {
-                    value = thisDigit;
-                }
-                mArgs[mArgIndex] = value;
+    /**
+     * Process the next ASCII character of a parameter.
+     *
+     * Parameter characters modify the action or interpretation of the sequence. You can use up to
+     * 16 parameters per sequence. You must use the ; character to separate parameters.
+     * All parameters are unsigned, positive decimal integers, with the most significant
+     * digit sent first. Any parameter greater than 9999 (decimal) is set to 9999
+     * (decimal). If you do not specify a value, a 0 value is assumed. A 0 value
+     * or omitted parameter indicates a default value for the sequence. For most
+     * sequences, the default value is 1.
+     *
+     * https://vt100.net/docs/vt510-rm/chapter4.html#S4.3.3
+     * */
+    private void parseArg(int inputByte) {
+        int[] bytes = new int[]{inputByte};
+        // Only doing this for ESC_CSI and not for other ESC_CSI_* since they seem to be using their
+        // own defaults with getArg*() calls, but there may be missed cases
+        if (mEscapeState == ESC_CSI) {
+            if ((mIsCSIStart && inputByte == ';') || // If sequence starts with a ; character, like \033[;m
+                (!mIsCSIStart && mLastCSIArg != null && mLastCSIArg == ';'  && inputByte == ';')) {  // If sequence contains sequential ; characters, like \033[;;m
+                bytes = new int[]{'0', ';'}; // Assume 0 was passed
             }
-            continueSequence(mEscapeState);
-        } else if (b == ';') {
-            if (mArgIndex < mArgs.length) {
-                mArgIndex++;
+        }
+
+        mIsCSIStart = false;
+
+        for (int b : bytes) {
+            if (b >= '0' && b <= '9') {
+                if (mArgIndex < mArgs.length) {
+                    int oldValue = mArgs[mArgIndex];
+                    int thisDigit = b - '0';
+                    int value;
+                    if (oldValue >= 0) {
+                        value = oldValue * 10 + thisDigit;
+                    } else {
+                        value = thisDigit;
+                    }
+                    mArgs[mArgIndex] = value;
+                }
+                continueSequence(mEscapeState);
+            } else if (b == ';') {
+                if (mArgIndex < mArgs.length) {
+                    mArgIndex++;
+                }
+                continueSequence(mEscapeState);
+            } else {
+                unknownSequence(b);
             }
-            continueSequence(mEscapeState);
-        } else {
-            unknownSequence(b);
+            mLastCSIArg = b;
         }
     }
 

diff --git a/terminal-emulator/src/test/java/com/termux/terminal/TerminalTest.java b/terminal-emulator/src/test/java/com/termux/terminal/TerminalTest.java
@@ -151,6 +151,19 @@ public void testSelectGraphics() {
 		assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
 		assertEquals(TextStyle.COLOR_INDEX_BACKGROUND, mTerminal.mBackColor);
 
+		// Test CSI resetting to default if sequence starts with ; or has sequential ;;
+        // Check TerminalEmulator.parseArg()
+        enterString("\033[31m\033[m");
+        assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
+        enterString("\033[31m\033[;m");
+        assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
+        enterString("\033[31m\033[0m");
+        assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
+        enterString("\033[31m\033[0;m");
+        assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
+        enterString("\033[31;;m");
+        assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
+
 		// 256 colors:
 		enterString("\033[38;5;119m");
 		assertEquals(119, mTerminal.mForeColor);