RFR: 8311943: Cleanup usages of toLowerCase() and toUpperCase() in java.base [v2]

Alan Bateman alanb at openjdk.org
Fri Jul 14 10:27:54 UTC 2023


On Wed, 12 Jul 2023 16:17:49 GMT, Glavo <duke at openjdk.org> wrote:

>> Maybe a small suggestion to make it clear whats wanted here. In other projects I am involved in (Apache Lucene/Solr, Apache TIKA, PostgresSQL JDBC, Checkstyle itsself, Elasticserach/Opensearch), which use the [forbiddenapis Maven/Gradle/Ant plugin](https://github.com/policeman-tools/forbidden-apis/), we forbid all calls to several Java APIs (including toLowerCase/toUpperCase case). All bytecode using this will build failure (FYI, we also disallow other stuff like relying of default timezone or characterset).
>> To make it clear what is really intended, those projects agreed on having `toLowerCase(Locale.getDefault())`, so it is explicit what's wanted.
>> Without that it could be that somebody else starts the discussion again.
>> 
>> This is just a suggestion to be explicit as it makes maintaining the code easier.
>
>> Maybe a small suggestion to make it clear whats wanted here. In other projects I am involved in (Apache Lucene/Solr, Apache TIKA, PostgresSQL JDBC, Checkstyle itsself, Elasticserach/Opensearch), which use the [forbiddenapis Maven/Gradle/Ant plugin](https://github.com/policeman-tools/forbidden-apis/), we forbid all calls to several Java APIs (including toLowerCase/toUpperCase case). All bytecode using this will build failure (FYI, we also disallow other stuff like relying of default timezone or characterset). To make it clear what is really intended, those projects agreed on having `toLowerCase(Locale.getDefault())`, so it is explicit what's wanted. Without that it could be that somebody else starts the discussion again.
>> 
>> This is just a suggestion to be explicit as it makes maintaining the code easier.
> 
> I agree with this.
> 
> I'm working on deprecating `toLowerCase()` and `toUpperCase()`, this PR is part of that effort. I wish to convert all use cases of them to `toLowerCase(Locale)` and `toUpperCase(Locale)`.
> 
> More backstory is detailed in https://github.com/openjdk/jdk/pull/13434#issuecomment-1503989660.

> However, while I think this corrects the behavior, this caused a change in the behavior of the API, so a CSR may be required. I don't want to debate this in this PR, so I'll revert this change and open a new PR in the future.

StreamTokenizer is a very old API and changing long standing behavior may break something or be observable with existing code/usages. I see youve reverted this part (thanks) and looking at it separately is fine. It might be that the conclusion is that it's just too risky to change, in which case Uwe's suggestion is good and would avoid it showing up on someone's else radar in the future.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14763#discussion_r1263570865


More information about the security-dev mailing list