Copilot commented on code in PR #2689:
URL: https://github.com/apache/groovy/pull/2689#discussion_r3565557245
##########
src/main/java/org/codehaus/groovy/runtime/ProcessGroovyMethods.java:
##########
@@ -79,11 +80,32 @@ public static InputStream getIn(Process self) {
* @since 1.0
*/
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())));
Review Comment:
This change alters how process output is decoded (using the OS native
encoding rather than Charset.defaultCharset()) but there doesn't appear to be a
regression test that would fail under JEP 400 conditions (e.g., default UTF-8 +
native.encoding set to a legacy single-byte charset and the process emitting a
non-ASCII byte like 0xE9). Adding a focused test around
getText()/waitForProcessOutput output decoding would help prevent silent
corruption regressions.
##########
src/main/java/org/codehaus/groovy/runtime/ProcessGroovyMethods.java:
##########
@@ -79,11 +80,32 @@ public static InputStream getIn(Process self) {
* @since 1.0
*/
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())));
closeStreams(self);
return text;
}
+ /**
+ * Returns the charset used to decode a child process's textual output. A
process writes its
+ * output in the operating system's native encoding; since JEP 400 (JDK 18)
+ * {@link Charset#defaultCharset()} is UTF-8 regardless of platform, so on
a non-UTF-8 host it
+ * no longer matches what the process emitted. The {@code native.encoding}
system property
+ * (JDK 17+) reports the real native encoding.
+ *
+ * @return the native-encoding charset, or {@link
Charset#defaultCharset()} if it is unavailable
+ */
+ private static Charset nativeEncoding() {
+ 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();
+ }
Review Comment:
nativeEncoding() calls System.getProperty("native.encoding") directly; under
a SecurityManager or restricted environment this can throw SecurityException
and break process output decoding. Consider catching SecurityException and
falling back to Charset.defaultCharset() the same way as for an
unavailable/invalid property value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
