8359732: Make standard i/o encoding related system properties `StaticProperty` #25860

naotoj · 2025-06-17T20:16:05Z

Refactored the internal handling of stdin/out/err.encoding to allow setting them only via command-line options by converting them into StaticProperty. This change prevents unexpected behavior caused by applications modifying these properties at runtime using System.setProperty().

Progress

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

Issue

JDK-8359732: Make standard i/o encoding related system properties StaticProperty (Enhancement - P4)

Reviewers

Volkan Yazici (@vy - Committer) Review applies to 9d1dbf3a
Roger Riggs (@RogerRiggs - Reviewer)
Alan Bateman (@AlanBateman - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25860/head:pull/25860
$ git checkout pull/25860

Update a local copy of the PR:
$ git checkout pull/25860
$ git pull https://git.openjdk.org/jdk.git pull/25860/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25860

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25860.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-06-17T20:16:42Z

👋 Welcome back naoto! 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 · 2025-06-17T20:31:09Z

@naotoj 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:

8359732: Make standard i/o encoding related system properties `StaticProperty`

Reviewed-by: rriggs, alanb, vyazici

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 52 new commits pushed to the master branch:

81985d4: 8358526: Clarify behavior of java.awt.HeadlessException constructed with no-args
d627282: 8360143: ProblemList runtime/NMT/VirtualAllocTestType.java
96f71a9: 8255082: HotSpot Style Guide should permit noexcept
... and 49 more: https://git.openjdk.org/jdk/compare/e55ddabffa90e28d22f546b387007fe4e434c3e0...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.

openjdk · 2025-06-17T20:33:28Z

@naotoj The following labels will be automatically applied to this pull request:

core-libs
hotspot-jfr
kulla
security
serviceability

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

mlbridge · 2025-06-17T20:37:03Z

Webrevs

vy

I've verified that all relevant occurrences of std{in,err,out}.encoding are covered, except the ones src/java.base/share/classes/java/lang/System.java, which, I presume, is left out intentionally.

AlanBateman · 2025-06-18T07:50:31Z

src/java.base/share/classes/module-info.java

        jdk.jlink,
        jdk.jpackage,
+        jdk.jshell,
        jdk.net;


At some point we will need to re-visit all these qualified exports so that java.base exports as few of these internal packages as possible.

The original intent of StaticProperties was to limit it to the base module.
Other modules are free to read the property themselves.

Thanks, Alan and Roger. Limited the modification within the java.base module.

AlanBateman · 2025-06-18T07:51:46Z

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CLHSDB.java

@@ -107,7 +108,7 @@ public void reattach() {
            };


-        Charset charset = Charset.forName(System.getProperty("stdin.encoding"), Charset.defaultCharset());
+        Charset charset = Charset.forName(StaticProperty.stdinEncoding(), Charset.defaultCharset());


This is the jhsdb tool, just wondering why it needs to be changed.

AlanBateman · 2025-06-18T07:53:06Z

src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java

@@ -797,7 +799,7 @@ public TTY() throws Exception {
            this.handler = new EventHandler(this, true, trackVthreads);
        }
        try {
-            Charset charset = Charset.forName(System.getProperty("stdin.encoding"), Charset.defaultCharset());
+            Charset charset = Charset.forName(StaticProperty.stdinEncoding(), Charset.defaultCharset());


This is the example jdb debugger, I assume okay to let it read the stdin.encoding property.

AlanBateman · 2025-06-18T07:57:19Z

src/java.scripting/share/classes/javax/script/SimpleScriptContext.java

@@ -95,7 +97,7 @@ public SimpleScriptContext() {
    }

    private static InputStreamReader stdinReader() {
-        Charset charset = Charset.forName(System.getProperty("stdin.encoding"), Charset.defaultCharset());
+        Charset charset = Charset.forName(StaticProperty.stdinEncoding(), Charset.defaultCharset());


This is the jrunscript command line tool. I suppose it's possible it could run a script that changes stdin.encoding.

RogerRiggs

Static properties should not be used outside of java.base.
The "for internal use only" should have said "java.base internal use only"
Adding a comment to that effect would be a good idea.

RogerRiggs · 2025-06-18T15:03:06Z

src/java.base/share/classes/module-info.java

        jdk.jlink,
        jdk.jpackage,
+        jdk.jshell,
        jdk.net;


The original intent of StaticProperties was to limit it to the base module.
Other modules are free to read the property themselves.

naotoj · 2025-06-18T17:55:58Z

/label remove hotspot-jfr,kulla,serviceability

openjdk · 2025-06-18T17:57:29Z

@naotoj
The hotspot-jfr label was successfully removed.

The kulla label was successfully removed.

The serviceability label was successfully removed.

naotoj · 2025-06-18T18:22:16Z

I've verified that all relevant occurrences of std{in,err,out}.encoding are covered, except the ones src/java.base/share/classes/java/lang/System.java, which, I presume, is left out intentionally.

Initially I left them as it is too early to call, but just noticed StaticProperty class was initialized a few lines above, so it is safe to call them. Replaced.

AlanBateman · 2025-06-19T11:03:05Z

src/java.base/share/classes/java/lang/System.java

-        setOut0(newPrintStream(fdOut, props.getProperty("stdout.encoding")));
-        initialErr = newPrintStream(fdErr, props.getProperty("stderr.encoding"));
+        setOut0(newPrintStream(fdOut, StaticProperty.stdoutEncoding()));
+        initialErr = newPrintStream(fdErr, StaticProperty.stderrEncoding());


This is initPhase1 where we've just saved the initial properties. I think it would be better to not change this. It would be okay to use tempProps here too, but I think clearer as is.

AlanBateman

Latest version looks okay but I think better to drop the change to System.initPhase1.

RogerRiggs

lgtm

…ava.base

naotoj · 2025-06-20T18:46:21Z

Latest version looks okay but I think better to drop the change to System.initPhase1.

Reverted the changes in the System class. Also, there were two sites that used StaticProperty class outside the java.base module. I replaced them with the equivalent System.getProperty() calls.

RogerRiggs

The updates look fine.

naotoj · 2025-06-23T16:15:19Z

Thanks for the reviews!
/integrate

openjdk · 2025-06-23T16:16:28Z

Going to push as commit 9c3eaa4.
Since your change was applied there have been 66 commits pushed to the master branch:

dfcea05: 8358729: jdk/internal/loader/URLClassPath/ClassnameCharTest.java depends on Applet
c748d35: 8360024: Reorganize GC VM operations and implement is_gc_operation
72679c9: 8360042: GHA: Bump MSVC to 14.44
... and 63 more: https://git.openjdk.org/jdk/compare/e55ddabffa90e28d22f546b387007fe4e434c3e0...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-06-23T16:16:35Z

@naotoj Pushed as commit 9c3eaa4.

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

initial commit

7dfdbfe

Several more locations

9d1dbf3

openjdk bot added the rfr Pull request is ready for review label Jun 17, 2025

openjdk bot added security [email protected] serviceability [email protected] core-libs [email protected] kulla [email protected] hotspot-jfr [email protected] labels Jun 17, 2025

vy approved these changes Jun 18, 2025

View reviewed changes

AlanBateman reviewed Jun 18, 2025

View reviewed changes

RogerRiggs suggested changes Jun 18, 2025

View reviewed changes

Reflecting review comments

c5c9338

openjdk bot removed hotspot-jfr [email protected] kulla [email protected] serviceability [email protected] labels Jun 18, 2025

Replaces a couple more

278628b

AlanBateman reviewed Jun 19, 2025

View reviewed changes

RogerRiggs approved these changes Jun 20, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 20, 2025

Reflects review comments. Limits the usage of StaticProperty within j…

0a3684b

…ava.base

openjdk bot removed the ready Pull request is ready to be integrated label Jun 20, 2025

RogerRiggs approved these changes Jun 20, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 20, 2025

AlanBateman approved these changes Jun 21, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jun 23, 2025

openjdk bot closed this Jun 23, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 23, 2025

8359732: Make standard i/o encoding related system properties StaticProperty #25860

8359732: Make standard i/o encoding related system properties StaticProperty #25860

Conversation

naotoj commented Jun 17, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jun 17, 2025

Uh oh!

openjdk bot commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jun 17, 2025

Uh oh!

mlbridge bot commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

vy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RogerRiggs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

naotoj commented Jun 18, 2025

Uh oh!

openjdk bot commented Jun 18, 2025

Uh oh!

naotoj commented Jun 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanBateman left a comment

Choose a reason for hiding this comment

Uh oh!

RogerRiggs left a comment

Choose a reason for hiding this comment

Uh oh!

naotoj commented Jun 20, 2025

Uh oh!

RogerRiggs left a comment

Choose a reason for hiding this comment

Uh oh!

naotoj commented Jun 23, 2025

Uh oh!

openjdk bot commented Jun 23, 2025

Uh oh!

openjdk bot commented Jun 23, 2025

Uh oh!

Uh oh!

8359732: Make standard i/o encoding related system properties `StaticProperty` #25860

8359732: Make standard i/o encoding related system properties `StaticProperty` #25860

naotoj commented Jun 17, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jun 17, 2025 •

edited

Loading

mlbridge bot commented Jun 17, 2025 •

edited

Loading