-
Notifications
You must be signed in to change notification settings - Fork 146
8311906: Improve robustness of String constructors with mutable array inputs #2437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…e exception is off by one
…F16String.java fails with -Xcomp
…m codepoints if CompactStrings is not enabled
|
👋 Welcome back goetz! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
This backport pull request has now been updated with issue from the original commit. |
|
/issue 8321180 8322018 8321514 8325590 |
|
@GoeLin Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: |
|
GHA failure: test/jdk/java/util/concurrent/ExecutorService/CloseTest.java failed with |
I backport this for parity with 21.0.10-oracle.
I had to resolve and adapt the change to the CSR for 21, that differes from the CSR for 22.
In addition, I include four follow-up changes. All steps are seperated into individual commits.
In detail:
Resolved two files (first commit)
src/java.base/share/classes/java/lang/String.java
Resolved complex code in
private String(Charset charset, byte[] bytes, int offset, int length)
at line 563++
This is needed because later change
https://bugs.openjdk.org/browse/JDK-8320570: NegativeArraySizeException decoding >1G UTF8 bytes with non-ascii characters
was already backported.
In 22, this is removed:
- byte[] buf = new byte[length << 1];In 21, it looks like this:
- byte[] buf = StringUTF16.newBytesFor(length);I only updated variable buf to utf16 in that line effecitvely keeping 8320570.
After this change, the method looks the same as in 22.0.2.
src/java.base/share/classes/java/lang/StringUTF16.java
Resolved imports.
Adapted to CSR (second commit)
The CSRs for 21 and 22 differ. The CSR for 21 does not mention that the documentation is
changed. Thus, I omit corresponding edits, see extra commit that removes them. Similar
modifications to backports have been done before.
Follow-ups (commits 3-6)
The original change has some minor issues. Notable for example that the test StringRacyConstructor.java is failing. Three follow-up issues exist, and one recursive fix for a test. These are clean backports on top. I decided to include them here as working with four dependent PRs is quite cumbersome, and the main change needs review anyways. I guess reviewing a correct change has some value, too.
In case a backport to 17 is necessary, all can be grabbed from 21 together this way.
Adapt 8325590 to 21 (last commit)
The test modification of 8325590 is not compatible with Java 21. https://bugs.openjdk.org/browse/JDK-8310047: "Add UTF-32 based Charsets into StandardCharsets" is only in 22. Removed the corresponding test cases.
Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/2437/head:pull/2437$ git checkout pull/2437Update a local copy of the PR:
$ git checkout pull/2437$ git pull https://git.openjdk.org/jdk21u-dev.git pull/2437/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2437View PR using the GUI difftool:
$ git pr show -t 2437Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/2437.diff
Using Webrev
Link to Webrev Comment