Skip to content

fix: Skip grapheme cluster probe on Windows to prevent output corruption#1695

Merged
gnodet merged 3 commits intomasterfrom
fix/windows-grapheme-probe-leak
Mar 10, 2026
Merged

fix: Skip grapheme cluster probe on Windows to prevent output corruption#1695
gnodet merged 3 commits intomasterfrom
fix/windows-grapheme-probe-leak

Conversation

@gnodet
Copy link
Copy Markdown
Member

@gnodet gnodet commented Mar 10, 2026

Summary

  • Skip the DECRQM mode 2027 probe in probeGraphemeClusterMode() on Windows to prevent escape sequence leakage into process output

Problem

On Windows with Cygwin/MSYSTEM (Git Bash), the ExecTerminalProvider creates a PosixSysTerminal whose ExecPty slave output goes to a raw FileDescriptor (stdout/stderr) rather than a real PTY device. When probeGraphemeClusterMode() writes the DECRQM query (\e[?2027$p) to this raw fd, it contaminates the process output.

This was discovered via the Maven JLine 4.0.2 upgrade where all Windows CI jobs fail with:

java.nio.file.InvalidPathException: Illegal char <> at index 0: [?2027$pD:\a\maven\maven\...\localRepository4\.m2\repository

The [?2027$p escape sequence leaks into stdout and gets prepended to paths that Maven parses.

Fix

Added a guard in AbstractTerminal.probeGraphemeClusterMode() to return false on Windows. This is safe because:

  1. AbstractWindowsTerminal already overrides supportsGraphemeClusterMode() to return false — native Windows terminals are unaffected
  2. The only path to probeGraphemeClusterMode() on Windows is through the Cygwin/MSYSTEM PosixSysTerminal, where the probe output goes to a raw FileDescriptor rather than a real PTY device

Does not apply to jline-3.x — the grapheme cluster mode 2027 feature was introduced only in the 4.x line.

Test plan

  • Existing GraphemeClusterModeTest passes (uses LineDisciplineTerminal, not affected)
  • All terminal module tests pass
  • Verify Maven Windows CI passes with the fix

gnodet added 3 commits March 10, 2026 21:08
On Windows with Cygwin/MSYSTEM (Git Bash), the ExecTerminalProvider
creates a PosixSysTerminal whose ExecPty slave output goes to a raw
FileDescriptor (stdout/stderr) rather than a real PTY device. When
probeGraphemeClusterMode() writes the DECRQM query (\e[?2027$p) to
this raw fd, it contaminates the process output. Downstream consumers
that parse stdout (e.g. Maven's ToolboxToolTest) then fail because
paths get prefixed with the escape sequence.

AbstractWindowsTerminal already overrides supportsGraphemeClusterMode()
to return false, so this guard only affects the Cygwin/MSYSTEM
PosixSysTerminal path where the probe cannot work correctly anyway.

Fixes apache/maven#11774
Move the IS_WINDOWS check from AbstractTerminal.probeGraphemeClusterMode()
to PosixSysTerminal.supportsGraphemeClusterMode() so that
LineDisciplineTerminal-based tests (GraphemeClusterModeTest) are not
affected on Windows.
Instead of blanket-disabling grapheme cluster mode on all Windows
Cygwin/MSYSTEM terminals, only skip the probe when the ExecPty output
stream is not actually connected to a terminal (e.g. piped in a
subprocess).  Interactive Cygwin/MSYSTEM terminals that support mode
2027 will still get grapheme cluster support.
@gnodet gnodet merged commit edd28d2 into master Mar 10, 2026
9 checks passed
@gnodet gnodet deleted the fix/windows-grapheme-probe-leak branch March 10, 2026 20:42
gnodet added a commit that referenced this pull request Mar 11, 2026
The previous fix (#1695) only blocked the DECRQM probe when using
ExecPty and the output stream was not detected as a system stream.
This was insufficient because the isSystemStream() check is unreliable
on Windows MSYSTEM environments — it can return true for streams that
are being captured by a parent process.

Simplify the guard to disable the probe entirely on Windows
PosixSysTerminal. On Windows, PosixSysTerminal only exists for
Cygwin/MSYSTEM environments where the slave output goes to a raw
FileDescriptor, making the probe inherently risky.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant