Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: Improve handling of empty ';' SGR sequences #4031

Merged
merged 1 commit into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,6 @@ 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;

Expand Down Expand Up @@ -1393,8 +1388,6 @@ private void doEsc(int b) {
break;
case '[':
continueSequence(ESC_CSI);
mIsCSIStart = true;
mLastCSIArg = null;
break;
case '=': // DECKPAM
setDecsetinternalBit(DECSET_BIT_APPLICATION_KEYPAD, true);
Expand Down Expand Up @@ -1765,7 +1758,7 @@ private void doCsi(int b) {
private void selectGraphicRendition() {
if (mArgIndex >= mArgs.length) mArgIndex = mArgs.length - 1;
for (int i = 0; i <= mArgIndex; i++) {
int code = mArgs[i];
int code = getArg(i, 0, false);
if (code < 0) {
if (mArgIndex > 0) {
continue;
Expand Down Expand Up @@ -1823,7 +1816,10 @@ private void selectGraphicRendition() {
if (i + 4 > mArgIndex) {
Logger.logWarn(mClient, LOG_TAG, "Too few CSI" + code + ";2 RGB arguments");
} else {
int red = mArgs[i + 2], green = mArgs[i + 3], blue = mArgs[i + 4];
int red = getArg(i + 2, 0, false);
int green = getArg(i + 3, 0, false);
int blue = getArg(i + 4, 0, false);

if (red < 0 || green < 0 || blue < 0 || red > 255 || green > 255 || blue > 255) {
finishSequenceAndLogError("Invalid RGB: " + red + "," + green + "," + blue);
} else {
Expand All @@ -1837,7 +1833,7 @@ private void selectGraphicRendition() {
i += 4; // "2;P_r;P_g;P_r"
}
} else if (firstArg == 5) {
int color = mArgs[i + 2];
int color = getArg(i + 2, 0, false);
i += 2; // "5;P_s"
if (color >= 0 && color < TextStyle.NUM_INDEXED_COLORS) {
if (code == 38) {
Expand Down Expand Up @@ -2115,44 +2111,29 @@ private void scrollDownOneLine() {
*
* 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
}
}

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;
}
if (value > 9999)
value = 9999;
mArgs[mArgIndex] = value;
}
continueSequence(mEscapeState);
} else if (b == ';') {
if (mArgIndex < mArgs.length) {
mArgIndex++;
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;
}
continueSequence(mEscapeState);
} else {
unknownSequence(b);
if (value > 9999)
value = 9999;
mArgs[mArgIndex] = value;
}
continueSequence(mEscapeState);
} else if (b == ';') {
if (mArgIndex < mArgs.length) {
mArgIndex++;
}
mLastCSIArg = b;
continueSequence(mEscapeState);
} else {
unknownSequence(b);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,13 @@ public void testSelectGraphics() {
assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
enterString("\033[31;;m");
assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
enterString("\033[31;m");
assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
enterString("\033[31;;41m");
assertEquals(TextStyle.COLOR_INDEX_FOREGROUND, mTerminal.mForeColor);
assertEquals(1, mTerminal.mBackColor);
enterString("\033[0m");
assertEquals(TextStyle.COLOR_INDEX_BACKGROUND, mTerminal.mBackColor);

// 256 colors:
enterString("\033[38;5;119m");
Expand All @@ -178,10 +185,18 @@ public void testSelectGraphics() {
assertEquals(129, mTerminal.mBackColor);

// Multiple parameters at once:
enterString("\033[38;5;178;48;5;179;m");
enterString("\033[38;5;178;48;5;179m");
assertEquals(178, mTerminal.mForeColor);
assertEquals(179, mTerminal.mBackColor);

// Omitted parameter means zero:
enterString("\033[38;5;m");
assertEquals(0, mTerminal.mForeColor);
assertEquals(179, mTerminal.mBackColor);
enterString("\033[48;5;m");
assertEquals(0, mTerminal.mForeColor);
assertEquals(0, mTerminal.mBackColor);

// 24 bit colors:
enterString(("\033[0m")); // Reset fg and bg colors.
enterString("\033[38;2;255;127;2m");
Expand All @@ -205,6 +220,16 @@ public void testSelectGraphics() {
enterString("\033[38;2;300;127;2;48;2;1;300;254m");
assertEquals(expectedForeground, mTerminal.mForeColor);
assertEquals(expectedBackground, mTerminal.mBackColor);

// 24 bit colors, omitted parameter means zero:
enterString("\033[38;2;255;127;m");
expectedForeground = 0xff000000 | (255 << 16) | (127 << 8);
assertEquals(expectedForeground, mTerminal.mForeColor);
assertEquals(expectedBackground, mTerminal.mBackColor);
enterString("\033[38;2;123;;77m");
expectedForeground = 0xff000000 | (123 << 16) | 77;
assertEquals(expectedForeground, mTerminal.mForeColor);
assertEquals(expectedBackground, mTerminal.mBackColor);
}

public void testBackgroundColorErase() {
Expand Down
Loading