8285517: System.getenv() returns unexpected value if environment variable has non ASCII character #8378

takiguc · 2022-04-24T09:18:54Z

On JDK19 with Linux ja_JP.eucjp locale,
System.getenv() returns unexpected value if environment variable has Japanese EUC characters.
It seems this issue happens because of JEP 400.
Arguments for ProcessBuilder have same kind of issue.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 reviewer)

Issue

JDK-8285517: System.getenv() returns unexpected value if environment variable has non ASCII character

Reviewers

@kristylee88 (no known github.com user name / role) ⚠️ Review applies to 89ee195a
Naoto Sato (@naotoj - Reviewer)
Roger Riggs (@RogerRiggs - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8378/head:pull/8378
$ git checkout pull/8378

Update a local copy of the PR:
$ git checkout pull/8378
$ git pull https://git.openjdk.java.net/jdk pull/8378/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8378

View PR using the GUI difftool:
$ git pr show -t 8378

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8378.diff

…able has non ASCII character

bridgekeeper · 2022-04-24T09:20:38Z

👋 Welcome back itakiguchi! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-04-24T09:22:56Z

@takiguc The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-04-24T09:26:40Z

Webrevs

naotoj

Thanks for fixing the issue.
As to the test, it has lots of redundancy, and too complicated to me. Can you simplify the test?

naotoj · 2022-04-26T20:35:38Z

src/java.base/unix/classes/java/lang/ProcessImpl.java

@@ -142,11 +143,22 @@ static Platform get() {
    private static final Platform platform = Platform.get();
    private static final LaunchMechanism launchMechanism = platform.launchMechanism();
    private static final byte[] helperpath = toCString(StaticProperty.javaHome() + "/lib/jspawnhelper");
+    private static Charset jnuCharset = null;


Can we simply call jnuCharset() here?

Change jnuCharset to "private static".

naotoj · 2022-04-26T21:09:10Z

test/jdk/java/lang/ProcessBuilder/Basic.java

+            String nativeEncoding = System.getProperty("native.encoding");
+            String fileEncoding = System.getProperty("file.encoding");
+            Charset cs;
+            if (nativeEncoding != null && !nativeEncoding.equals(fileEncoding)) {


What's the reason for comparing file.encoding? Does Charset.forName(nativeEncoding, Charset.defaultCharset()) suffice?

Remove file.encoding related code.

naotoj · 2022-04-26T21:13:47Z

test/jdk/java/lang/System/i18nEnvArg.java

+    final static int maxSize = 4096;
+
+    public static void main(String[] args) throws Exception {
+        ProcessBuilder pb = new ProcessBuilder(javeExe,


Can utilize jdk.test.lib.process.ProcessTools.

Use ProcessTools class.

naotoj · 2022-04-26T21:15:55Z

test/jdk/java/lang/System/i18nEnvArg.java

+
+/*
+ * @test
+ * @bug 8285517


If the test should work only on a particular env, should describe here and add @requires tag.

Add "@requires (os.family == "linux")"

Also @modules jdk.charsets should be added so that the test should be immediately ignored if it does not have EUC-JP charset.

RogerRiggs · 2022-04-27T17:20:11Z

src/java.base/unix/classes/java/lang/ProcessEnvironment.java



 final class ProcessEnvironment
 {
    private static final HashMap<Variable,Value> theEnvironment;
    private static final Map<String,String> theUnmodifiableEnvironment;
    static final int MIN_NAME_LENGTH = 0;
+    static final Charset cs = Charset.forName(StaticProperty.nativeEncoding(),


cs should have an all-CAPS name to make it clear its a static value throughout the implementation.
And private too.

Change to "private static final"

RogerRiggs · 2022-04-27T17:31:42Z

test/jdk/java/lang/System/i18nEnvArg.java

+            Class<?> cls = Class.forName("java.lang.ProcessEnvironment");
+            Method enviorn_mid = cls.getDeclaredMethod("environ");
+            enviorn_mid.setAccessible(true);
+            byte[][] environ = (byte[][]) enviorn_mid.invoke(null,


typo: "enviorn_mid" -> "environ_mid".

RogerRiggs · 2022-04-27T17:41:27Z

test/jdk/java/lang/System/i18nEnvArg.java

+import java.util.StringJoiner;
+
+public class i18nEnvArg {
+    final static String text = "\u6F22\u5B57";


Can this be renamed to indicate it is the string under test? like KANJI_TEXT or EUC_JP_TEXT or similar.

Use EUC_JP_TEXT

RogerRiggs · 2022-04-27T17:44:25Z

test/jdk/java/lang/System/i18nEnvArg.java

+        environ.put("LANG", "ja_JP.eucjp");
+        Process p = pb.start();
+        InputStream is = p.getInputStream();
+        byte[] ba = new byte[maxSize];


You can use InputStream.readAllBytes() here to return a byte buffer.

Its not clear why all the bytes are read? I assume its only for a possible error from the child.

Use InputStream.readAllBytes()

RogerRiggs · 2022-04-27T17:49:13Z

test/jdk/java/lang/System/i18nEnvArg.java

+        + "java";
+    final static int maxSize = 4096;
+
+    public static void main(String[] args) throws Exception {


There are 3 main()'s in this test; it would be useful to add a comment indicating the purpose of each.

Add comments

RogerRiggs · 2022-04-27T17:50:28Z

test/jdk/java/lang/System/i18nEnvArg.java

+                bb.put((byte)'=');
+                bb.put(environ[i+1]);
+                byte[] envb = Arrays.copyOf(ba, bb.position());
+                if (Arrays.equals(kanji, envb)) return;


It seems odd to exit the loop on the first match.

I think AIX always has environment variables not set by the caller.

On this testing, use 2 environment variables:

LANG=ja_JP.eucjp

\u6F22\u5B57=\u6F22\u5B57

If the other environment variable is defined, it's printed.

RogerRiggs · 2022-04-27T17:54:27Z

test/jdk/java/lang/System/i18nEnvArg.java

+                    } else if (b >= 0x20 && b <= 0x7F) {
+                        sb.append((char)b);
+                    } else {
+                        sb.append(String.format("\\x%02X", (int)b & 0xFF));


The methods of java.util.HexFormat may be useful here.
It seems that the "5C" would fit into this "else" clause and not need a separate statement.

Use HexFormat class

…able has non ASCII character

takiguc · 2022-05-01T04:46:21Z

Hello @naotoj and @RogerRiggs .
I appreciate your comments.
I applied the changes.
Additionally, I put bugid into test/jdk/java/lang/ProcessBuilder/Basic.java

RogerRiggs · 2022-05-02T15:00:07Z

Can you clarify the use of different charset properties for the ProcessEnvironment vs the CommandLine strings?

They are used to decode or encode strings in the APIs to native functions respectively.
If I understand native.encoding was introduced to reflect the default encoding of file contents, while sun.jnu.encoding is used to encode filenames.

I think I would have expected to see sun.jnu.encoding to be used in all contexts when encoding/decoding strings to the native representations. (Assuming its not required for backward compatibility).

takiguc · 2022-05-02T16:36:23Z

Hello @RogerRiggs .
You are right. Both case, sun.jnu.encoding should be used.

naotoj · 2022-05-02T20:49:19Z

test/jdk/java/lang/System/i18nEnvArg.java

+
+/*
+ * @test
+ * @bug 8285517


Also @modules jdk.charsets should be added so that the test should be immediately ignored if it does not have EUC-JP charset.

naotoj · 2022-05-02T20:50:11Z

test/jdk/java/lang/System/i18nEnvArg.java

+            .shouldHaveExitValue(0);
+    }
+
+    public static class Start {


Do we need Start class? Can it be merged with the parent to reduce the complexity?

I'm not sure, it expected one...

I created i18nEnvArg.sh for start-up, then add @modules jdk.charsets and remove i18nEnvArg$Start class.

Sorry if my comment was unclear. I meant to not create a shell script (shell scripts in the test are discouraged), but to re-use the main class twice, invoking with different arguments to main(), as both creating a testJvm process (one for eucjp jvm, and the other for Start).

naotoj · 2022-05-02T20:50:47Z

test/jdk/java/lang/System/i18nEnvArg.java

+            String javeExe = System.getProperty("java.home") +
+                File.separator +
+                "bin" +
+                File.separator +
+                "java";
+            ProcessBuilder pb = new ProcessBuilder(javeExe,
+                "--add-opens=java.base/java.lang=ALL-UNNAMED",
+                "-classpath",
+                System.getProperty("java.class.path"),
+                "i18nEnvArg$Verify",
+                EUC_JP_TEXT);


Can utilize ProcessTools class.

Use ProcessTools

naotoj · 2022-05-02T21:11:39Z

test/jdk/java/lang/System/i18nEnvArg.java

+            Class<?> cls = Class.forName("java.lang.ProcessEnvironment");
+            Method environ_mid = cls.getDeclaredMethod("environ");
+            environ_mid.setAccessible(true);
+            byte[][] environ = (byte[][]) environ_mid.invoke(null,
+                (Object[])null);


I am not sure I like this piece, invoking a package private method of ProcessEnvironment class. Can we simply verify the result of System.getenv(EUC_JP_TEXT)?

I think 3 processes required for this testing.

Start test process on ja_JP.eucjp locale

Set the test data by encoder

Verify the encoded data by decoder

To verify the encoded data, I think I need to check the byte data.

Do we need to verify the intermediate byte encoding? Could we simply compare the text given to environ.put() in Start process and System.getenv() in Verify process match?

Hello @naotoj .
I think if 2nd process' encoder (like UTF-8) and 3rd process' decoder are same encoding, System.getenv() returns expected data.
Actually the testcase checks 3 parts on Verify process:

Expected environment variable is defined or not (it uses System.getenv())

Expected argument is received or not

Expected environment variable is encoded by proper encoding

When I ran this testcase with jdk18.0.1, I got following result:

Unexpected argument was received: \u6F22\u5B57<->\u7FB2\u221A\uFFFD Unexpected environment variables: \xE6\xBC\xA2\xE5\xAD\x97\x3D\xE6\xBC\xA2\xE5\xAD\x97

It means 1st test was passed, 2nd and 3rd test were failed.
I don't think environment variable issue can be seen without 3rd test.
Please let me know if you find out another way.

This part of the test is very brittle; I'm pretty sure it will fail on AIX that adds its own environment variables.
It should not fail if it finds the two entries it expects. It should ignore other entries.

I don't see what value it has over checking the entries from System.getEnv(), please elaborate.

…able has non ASCII character

takiguc · 2022-05-03T17:30:23Z

Hello @RogerRiggs and @naotoj .

I put new testcase.
I'm not sure, it's expected one...

During my code changes, SecurityManager related testcases were failed.
I'm not sure I can use System.getProperty("sun.jnu.encoding") or not.
So I put SecurityManager related code.

…able has non ASCII character

takiguc · 2022-05-04T17:08:33Z

Hello @naotoj .
I removed i18nEnvArg.sh and modified i18nEnvArg.java.
Could you review it again ?

naotoj · 2022-05-05T01:30:39Z

src/java.base/unix/classes/java/lang/ProcessImpl.java

+        @SuppressWarnings("removal")
+        String jnuEncoding = AccessController.doPrivileged((PrivilegedAction<String>) ()
+            -> System.getProperty("sun.jnu.encoding"));


naotoj · 2022-05-05T01:31:24Z

src/java.base/unix/classes/java/lang/ProcessEnvironment.java

+        @SuppressWarnings("removal")
+        String jnuEncoding = AccessController.doPrivileged((PrivilegedAction<String>) ()
+            -> System.getProperty("sun.jnu.encoding"));


No need to issue doPrivileged(), which is deprecated

Hello @naotoj .
If I just use System.getProperty("sun.jnu.encoding"), following testcases were failed.

java/lang/ProcessBuilder/SecurityManagerClinit.java java/lang/ProcessHandle/PermissionTest.java

Please give me your suggestion again.

Ah, OK. Never mind.

It might be worthwhile to cache the sun.jnu.encoding property value in jdk/internal/util/StaticProperty.
And perhaps even cache the Charset (not just the string).

naotoj · 2022-05-05T01:34:48Z

test/jdk/java/lang/System/i18nEnvArg.java

+            var cmds = List.of(
+                "--add-modules=" + System.getProperty("test.modules"),
+                "-classpath",
+                System.getProperty("test.class.path"),
+                "-Dtest.jdk=" + System.getProperty("test.jdk"),
+                "-Dtest.class.path=" + System.getProperty("test.class.path"),
+                "-Dtest.modules=" + System.getProperty("test.modules"),
+                "i18nEnvArg",
+                "Start");
+            pb = ProcessTools.createTestJvm(cmds);
+            Map<String, String> environ = pb.environment();
+            environ.clear();
+            environ.put("LANG", "ja_JP.eucjp");


There are many duplicate pieces of code here and in the else block below. Can you simplify this if statement more?

Modified.
But I'm not sure, it's expected one.

naotoj · 2022-05-05T01:37:23Z

test/jdk/java/lang/System/i18nEnvArg.java

+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            PrintStream ps = new PrintStream(baos);


Can utilize try-with-resources pattern.

Use shouldNotContain() to find the error message.

I was suggesting try (ByteArrayOutputStream baos = ...) { so that no need to clean them up, but I see you removed them. But I prefer not to use shouldNotContain("ERROR: ") but to check the return value as before.

…able has non ASCII character

takiguc · 2022-05-06T14:28:24Z

Hello @RogerRiggs and @naotoj .
I put sun.jnu.encoding related code into sStaticProperty.java.
Could you review the files again including javadoc comment ?

RogerRiggs · 2022-05-06T15:13:38Z

test/jdk/java/lang/ProcessBuilder/Basic.java

+            Charset cs = jnuEncoding != null
+                ? Charset.forName(jnuEncoding, Charset.defaultCharset())
+                : Charset.defaultCharset();
+            if (new String(tested.getBytes(cs), cs).equals(tested)) {


Isn't it always true that the round trip encoding to bytes and back (using the same Charset) to a string is equal()?
And if it is always true, then the if(...) can be removed.

Above code is related to following code:

jdk/test/jdk/java/lang/ProcessBuilder/Basic.java

Lines 1569 to 1570 in 5212535

testEncoding("Latin1", "\u00f1\u00e1");

testEncoding("Unicode", "\u22f1\u11e1");

If Charset.defaultCharset() is UTF-8, this code is not skipped.
I think this code will be skipped on JDK17 on C locale.

RogerRiggs · 2022-05-06T15:18:17Z

test/jdk/java/lang/System/i18nEnvArg.java

+            byte[] eucjp = "LANG=ja_JP.eucjp".getBytes(cs);
+            String s = System.getenv(EUC_JP_TEXT);
+            if (!EUC_JP_TEXT.equals(s)) {
+                System.err.println("ERROR: getenv() returns unexpected data");


Please add the unexpected data s to the output string.

RogerRiggs · 2022-05-06T15:21:10Z

test/jdk/java/lang/System/i18nEnvArg.java

+                for(char ch : EUC_JP_TEXT.toCharArray()) {
+                   System.err.printf("\\u%04X", (int)ch);
+                }
+                System.err.print("<->");


This might be clearer if labeled as the actual/incorrect value and on a separate line.

RogerRiggs · 2022-05-06T15:22:37Z

test/jdk/java/lang/System/i18nEnvArg.java

+            }
+            if (!EUC_JP_TEXT.equals(args[0])) {
+                System.err.print("ERROR: Unexpected argument was received: ");
+                for(char ch : EUC_JP_TEXT.toCharArray()) {


This is the expected value, the previous "Unexpected" labeling might be mis-understood.

RogerRiggs · 2022-05-06T16:17:34Z

test/jdk/java/lang/System/i18nEnvArg.java

+            Class<?> cls = Class.forName("java.lang.ProcessEnvironment");
+            Method environ_mid = cls.getDeclaredMethod("environ");
+            environ_mid.setAccessible(true);
+            byte[][] environ = (byte[][]) environ_mid.invoke(null,
+                (Object[])null);


This part of the test is very brittle; I'm pretty sure it will fail on AIX that adds its own environment variables.
It should not fail if it finds the two entries it expects. It should ignore other entries.

I don't see what value it has over checking the entries from System.getEnv(), please elaborate.

naotoj · 2022-05-06T20:03:35Z

src/java.base/share/classes/jdk/internal/util/StaticProperty.java

+        jnuCharset = Charset.forName(SUN_JNU_ENCODING, Charset.defaultCharset());
    }


I am not sure it is OK to initialize Charset here, as sun_jnu_encoding is initialized in System.initPhase1() and pulling Charset there may cause some init order change. I'd only cache the encoding string here.

Note the initialization of sun.jnu.encoding in System.java:2142'ish.
This happens before StaticProperty is initialized; at line 2147.
If the sun.jnu.encoding is not supported (this is before Providers are enabled)
then it is forced to UTF-8.
So it is the case that the encoding is supported by that point.
Note that Charset.isSupported(...) does the full lookup in its implementation.

If it is not supported, the system still comes up using UTF-8 and a warning is printed at line 2282.

Comparing the class initialization order may be a useful thing to cross check.

…able has non ASCII character

takiguc · 2022-05-07T06:46:14Z

Hello @RogerRiggs .
When I executed the new testcase with JDK18.0.1, I got following errors:

ERROR: argument EUC_JP_TEXT is:
  Actual:   \u7FB2\u221A\uFFFD
  Expected: \u6F22\u5B57
ERROR: Variable EUC_JP_TEXT is encoded by:
  Actual:   \xE6\xBC\xA2\xE5\xAD\x97
  Expected: \xB4\xC1\xBB\xFA
ERROR: Value EUC_JP_TEXT is encoded by:
  Actual:   \xE6\xBC\xA2\xE5\xAD\x97
  Expected: \xB4\xC1\xBB\xFA

The new testcase verifies internal byte data for EUC_JP_TEXT environment variable and value.

takiguc · 2022-05-08T16:13:40Z

I checked internal data

$ cat StaticPropertyVals.java 
import java.lang.reflect.*;

public class StaticPropertyVals {
  public static void main(String[] args) throws Exception {
    Class<?> cls = Class.forName("jdk.internal.util.StaticProperty");
    for (Field fid : cls.getDeclaredFields()) {
      if (Modifier.isStatic(fid.getModifiers())) {
        fid.setAccessible(true);
        System.out.println(fid.getName() + "=" + fid.get(null));
      }
    }
  }
}
$ env LANG=kk_KZ.pt154 LC_ALL=kk_KZ.pt154 java -XshowSettings:properties --add-opens=java.base/jdk.internal.util=ALL-UNNAMED StaticPropertyVals 2>&1 | egrep -i 'encoding|charset'
WARNING: The encoding of the underlying platform's file system is not supported: PT154
    file.encoding = UTF-8
    native.encoding = PT154
    sun.io.unicode.encoding = UnicodeLittle
    sun.jnu.encoding = UTF-8
NATIVE_ENCODING=PT154
FILE_ENCODING=UTF-8
SUN_JNU_ENCODING=UTF-8
jnuCharset=UTF-8
$

RogerRiggs · 2022-05-11T18:07:42Z

src/java.base/unix/classes/java/lang/ProcessEnvironment.java



 final class ProcessEnvironment
 {
    private static final HashMap<Variable,Value> theEnvironment;
    private static final Map<String,String> theUnmodifiableEnvironment;
    static final int MIN_NAME_LENGTH = 0;
+    private static final Charset jnuCharset = StaticProperty.jnuCharset();


Since the Charset is cached in StaticProperty, I don't think its worthwhile to make a local copy of the same ref.
Just refer to StaticProperty.jnuCharset() where it is needed. (Here and in ProcessImpl)

…able has non ASCII character

takiguc · 2022-05-13T17:39:49Z

Hello @RogerRiggs and @naotoj .
I modified ProcessEnvironment.javaand ProcessImpl.java.
i18nEnvArg.java checks return code instead of string detection.

naotoj · 2022-05-13T18:29:56Z

src/java.base/share/classes/jdk/internal/util/StaticProperty.java

@@ -241,4 +246,29 @@ public static String fileEncoding() {
    public static String javaPropertiesDate() {
        return JAVA_PROPERTIES_DATE;
    }
+
+    /**
+     * Return the {@code sun.jnu.encoding} system property.


This can be eliminated by changing the@return block tag below to {@return the {@code sun.jnu.encoding} ...}.

Hello @naotoj .
I appreciate your comment.
I'd like to confirm one thing.
I applied following change

/** * {@return the {@code sun.jnu.encoding} system property} * * <strong>{@link SecurityManager#checkPropertyAccess} is NOT checked * in this method. The caller of this method should take care to ensure * that the returned property is not made accessible to untrusted code.</strong> */

By javadoc command with -html5 option, above part was converted to

Returns the sun.jnu.encoding system property. SecurityManager.checkPropertyAccess(java.lang.String) is NOT checked in this method. The caller of this method should take care to ensure that the returned property is not made accessible to untrusted code.

The 1st word changed from Return to Returns.
Is it OK ?
And it seems @return is translated to Japanese on Japanese environment.

The 1st word changed from Return to Returns.
Is it OK ?

That's more of plain English to me. Maybe some natives can correct it if needed.

And it seems @return is translated to Japanese on Japanese environment.

Yes. I believe that's what is supposed to work.

naotoj · 2022-05-13T18:31:13Z

src/java.base/share/classes/jdk/internal/util/StaticProperty.java

+    }
+
+    /**
+     * Return charset for {@code sun.jnu.encoding} system property.


Same as above. charset can be capitalized and changed to {@code}.

…able has non ASCII character

takiguc · 2022-05-17T01:19:16Z

Hello @naotoj .
To keep consistency, I modified other javadoc texts on StaticProperty.java.
And fix javdoc tag error.

naotoj

Looks good. Thanks for the fix!

openjdk · 2022-05-17T15:48:12Z

@takiguc This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8285517: System.getenv() returns unexpected value if environment variable has non ASCII character

Reviewed-by: naoto, rriggs

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 212 new commits pushed to the master branch:

9f562ef: 8286872: Refactor add/modify notification icon (TrayIcon)
b089229: 8271078: jdk/incubator/vector/Float128VectorTests.java failed a subtest
079312c: 8286182: [BACKOUT] x86: Handle integral division overflow during parsing
7b19226: 8267038: Update IANA Language Subtag Registry to Version 2022-03-02
e60d8b5: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException
aa50625: 8286447: [Linux] AWT should start in Headless mode if headful AWT library not installed
655500a: 8286654: Add an optional description accessor on ToolProvider interface
ac6a7d7: 8278367: JNI critical region violation in CTextPipe.m:363
83cec4b: 8284273: Early crashes in os::print_context on AArch64
fd36f37: 8286797: Guards of constant value false are not permitted
... and 202 more: https://git.openjdk.java.net/jdk/compare/4f5d73f2d411aa6147c5388b024e0d2996378d5a...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

takiguc · 2022-05-18T00:38:06Z

Hello @RogerRiggs .
Do you have any comment ?

RogerRiggs

Looks fine.
(I don't personally like how {@return looks in the source; but its done now. Usually it is preferred to stick to the style in the file and not mix stylistic changes with functional changes).
Thanks for the updates.

takiguc · 2022-05-19T23:37:21Z

/integrate

openjdk · 2022-05-19T23:38:29Z

Going to push as commit 890771e.
Since your change was applied there have been 213 commits pushed to the master branch:

de74e0e: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable
9f562ef: 8286872: Refactor add/modify notification icon (TrayIcon)
b089229: 8271078: jdk/incubator/vector/Float128VectorTests.java failed a subtest
079312c: 8286182: [BACKOUT] x86: Handle integral division overflow during parsing
7b19226: 8267038: Update IANA Language Subtag Registry to Version 2022-03-02
e60d8b5: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException
aa50625: 8286447: [Linux] AWT should start in Headless mode if headful AWT library not installed
655500a: 8286654: Add an optional description accessor on ToolProvider interface
ac6a7d7: 8278367: JNI critical region violation in CTextPipe.m:363
83cec4b: 8284273: Early crashes in os::print_context on AArch64
... and 203 more: https://git.openjdk.java.net/jdk/compare/4f5d73f2d411aa6147c5388b024e0d2996378d5a...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-05-19T23:38:45Z

@takiguc Pushed as commit 890771e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8285517: System.getenv() returns unexpected value if environment vari…

89ee195

…able has non ASCII character

openjdk bot added the rfr Pull request is ready for review label Apr 24, 2022

openjdk bot added the core-libs core-libs-dev@openjdk.org label Apr 24, 2022

naotoj reviewed Apr 26, 2022

View reviewed changes

RogerRiggs reviewed Apr 27, 2022

View reviewed changes

8285517: System.getenv() returns unexpected value if environment vari…

050dd66

…able has non ASCII character

naotoj reviewed May 2, 2022

View reviewed changes

8285517: System.getenv() returns unexpected value if environment vari…

9db7987

…able has non ASCII character

openjdk bot removed the rfr Pull request is ready for review label May 3, 2022

8285517: System.getenv() returns unexpected value if environment vari…

84e6d63

…able has non ASCII character

openjdk bot added the rfr Pull request is ready for review label May 4, 2022

naotoj reviewed May 5, 2022

View reviewed changes

8285517: System.getenv() returns unexpected value if environment vari…

5bd8492

…able has non ASCII character

RogerRiggs reviewed May 6, 2022

View reviewed changes

naotoj reviewed May 6, 2022

View reviewed changes

8285517: System.getenv() returns unexpected value if environment vari…

994a7fd

…able has non ASCII character

Merge branch 'master' into 8285517

6dbaa75

RogerRiggs reviewed May 11, 2022

View reviewed changes

8285517: System.getenv() returns unexpected value if environment vari…

1201801

…able has non ASCII character

naotoj reviewed May 13, 2022

View reviewed changes

8285517: System.getenv() returns unexpected value if environment vari…

b940a0d

…able has non ASCII character

naotoj approved these changes May 17, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label May 17, 2022

RogerRiggs approved these changes May 19, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label May 19, 2022

openjdk bot closed this May 19, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 19, 2022

		ByteArrayOutputStream baos = new ByteArrayOutputStream();
		PrintStream ps = new PrintStream(baos);

	testEncoding("Latin1", "\u00f1\u00e1");
	testEncoding("Unicode", "\u22f1\u11e1");

		jnuCharset = Charset.forName(SUN_JNU_ENCODING, Charset.defaultCharset());
		}

8285517: System.getenv() returns unexpected value if environment variable has non ASCII character #8378

8285517: System.getenv() returns unexpected value if environment variable has non ASCII character #8378

Conversation

takiguc commented Apr 24, 2022 • edited by openjdk bot

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Apr 24, 2022

openjdk bot commented Apr 24, 2022

mlbridge bot commented Apr 24, 2022 • edited

Webrevs

naotoj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takiguc commented May 1, 2022

RogerRiggs commented May 2, 2022

takiguc commented May 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takiguc commented May 3, 2022

takiguc commented May 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takiguc commented May 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takiguc commented May 7, 2022

takiguc commented May 8, 2022

Choose a reason for hiding this comment

takiguc commented May 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takiguc commented Apr 24, 2022 •

edited by openjdk bot

mlbridge bot commented Apr 24, 2022 •

edited

openjdk bot commented May 17, 2022 •

edited