RFR: 8356439: Rename JavaLangAccess::*NoRepl methods [v6]

Volkan Yazici vyazici at openjdk.org
Fri Aug 22 13:40:53 UTC 2025


On Fri, 22 Aug 2025 13:26:37 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:

>> `NoRepl`-suffixed `String` methods denote methods that do not replace invalid characters, but throw `CharacterCodingException` on encounter. This behavior cannot easily be derived from the method footprints, has been a source of confusion for maintainers, and is not uniformly adopted, e.g., `newStringUTF8NoRepl()` and `getBytesUTF8NoRepl()` does *not* throw `CCE`. This PR replaces the `NoRepl` suffix with `NoReplacement` in method names and consistently uses `throws CCE` in method footprints.
>
> Volkan Yazici has updated the pull request incrementally with five additional commits since the last revision:
> 
>  - Renamed to `malformedASCII`
>  - Improve exception parametrization
>  - Document parametrization on the exception type
>  - Avoid using links in the Javadoc title line
>  - Rename `NoReplTest` and fix its copyright year

src/java.base/share/classes/java/lang/String.java line 892:

> 890:             Charset cs, byte coder, byte[] val, Class<E> exceptionClass)
> 891:             // Parametrizing on exception type to enable callers (using null) to avoid having to declare the exception
> 892:             throws E {

@RogerRiggs, since there is nothing stopping the programmer from passing `TotallyUnrelatedException.class` to `encodeWithEncoder()` (or to 2 other sneakily throwing methods), I've tried adding the following as the first thing in this methods' body:

    assert exceptionClass == null || CharacterCodingException.class.isAssignableFrom(exceptionClass);

This beautifully caused a JVM crash due to early initialization issues surrounding `String`. @liach suggested the following:

1. Move assertions to `StringLatin1` and `StringUTF16` – This solves the issue, but the check is at a relatively later stage.
2. Replace `assert` with a runtime check, e.g., `if (...) { throw new AssertionError(...); }` – This solves the issue, but incurs a runtime penalty

What would you advice?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26413#discussion_r2293767452


More information about the security-dev mailing list