On Mon, 16 Oct 2023 22:27:34 GMT, Shaojin Wen <d...@openjdk.org> wrote:
> When calling String::newStringNoRepl and String::getBytesNoRepl, we need to
> use try...catch to handle CharacterCodingException and throw
> IllegalArgumentException instead of CharacterCodingException to make the
> calling code cleaner.
The thrown CCE was intended for `Files.readString` and `Files.writeString`, and
is in reality, only used by those 2 sites. However, you missed updating those
sites, so they now throw IAE instead of CCE, which should be fixed.
src/java.base/share/classes/java/lang/String.java line 932:
> 930:
> 931: private static byte[] getBytesNoRepl1(String s, Charset cs) {
> 932: return getBytesNoRepl1(s, cs);
This method should probably be removed. This current implementation is an
infinite recursion.
src/java.base/share/classes/java/util/zip/ZipCoder.java line 319:
> 317: }
> 318: } catch (IllegalArgumentException e) {
> 319: return Comparison.NO_MATCH;
Should check if the cause is `CharacterCodingException`
-------------
Changes requested by liach (Author).
PR Review: https://git.openjdk.org/jdk/pull/16209#pullrequestreview-1681283802
PR Review Comment: https://git.openjdk.org/jdk/pull/16209#discussion_r1361455389
PR Review Comment: https://git.openjdk.org/jdk/pull/16209#discussion_r1361455717
