Skip to content

decode process output streams using the native encoding#2689

Open
netliomax25-code wants to merge 1 commit into
apache:masterfrom
netliomax25-code:process-output-native-encoding
Open

decode process output streams using the native encoding#2689
netliomax25-code wants to merge 1 commit into
apache:masterfrom
netliomax25-code:process-output-native-encoding

Conversation

@netliomax25-code

Copy link
Copy Markdown
Contributor
  1. getText(Process) and TextDumper (which backs consumeProcessOutput, waitForProcessOutput and the new waitForResult/ProcessResult) decode the process output stream through new InputStreamReader(stream) with no charset, so they use Charset.defaultCharset().
  2. since JEP 400 that default is UTF-8 regardless of platform, while a child process still writes its output in the OS native encoding, so on a non-UTF-8 host (a Windows code page, or Linux under a C/legacy locale) non-ASCII output no longer round-trips and is silently corrupted (a native 0xE9 byte for é decodes to U+FFFD).

Decode with the native.encoding system property (JDK 17+, the documented way to obtain the OS native encoding), falling back to Charset.defaultCharset() when it is unavailable. UTF-8 hosts are unaffected.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Groovy’s process I/O helpers to decode child process output using the OS native encoding (via the native.encoding system property) instead of relying on Charset.defaultCharset(), avoiding silent character corruption on non-UTF-8 hosts post–JEP 400.

Changes:

  • Add a nativeEncoding() helper that resolves System.getProperty("native.encoding") with fallback to Charset.defaultCharset().
  • Use nativeEncoding() when decoding Process#getInputStream() in getText(Process).
  • Use nativeEncoding() in TextDumper (used by consumeProcessOutput* / waitForProcessOutput* pathways) when reading process output.
Comments suppressed due to low confidence (1)

src/main/java/org/codehaus/groovy/runtime/ProcessGroovyMethods.java:658

  • TextDumper creates BufferedReader/InputStreamReader but never closes it, so the underlying process stream can remain open if the thread exits early (e.g., due to an exception). Wrapping the reader in try-with-resources ensures the stream is always closed and avoids resource leaks.
            InputStreamReader isr = new InputStreamReader(in, nativeEncoding());
            BufferedReader br = new BufferedReader(isr);
            String next;
            try {
                while ((next = br.readLine()) != null) {

Comment on lines +98 to +107
String name = System.getProperty("native.encoding");
if (name != null && !name.isEmpty()) {
try {
return Charset.forName(name);
} catch (IllegalArgumentException ignore) {
// unknown or unsupported name - fall back below
}
}
return Charset.defaultCharset();
}
*/
public static String getText(Process self) throws IOException {
String text = IOGroovyMethods.getText(new BufferedReader(new InputStreamReader(self.getInputStream())));
String text = IOGroovyMethods.getText(new BufferedReader(new InputStreamReader(self.getInputStream(), nativeEncoding())));
@testlens-app

testlens-app Bot commented Jul 12, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 913be1a
▶️ Tests: 103404 executed
⚪️ Checks: 23/23 completed


Learn more about TestLens at testlens.app.

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.

2 participants