search

gradle / gradle

Adaptable, fast automation for all
https://gradle.org
Apache License 2.0
16.99k stars 4.76k forks source link

Fix `toLowerCase` sensitivity regarding `Locale` #1506

Closed lacasseio closed 3 months ago

lacasseio commented 7 years ago

Expected Behavior

Using a different Locale than English shouldn't cause unexpected behavior in Gradle.

Current Behavior

Using Turkish locale can cause unexpected behavior in presence of dotless i character (upper->lower). Various place in the code base is vulnerable to this Locale sensitivity. Some places are fixed by rolling out a contextual fix.

Context

This started with PR https://github.com/gradle/gradle/pull/1408. @wolfs suggest we use Findbugs or Pmd to check for this kind of bug. @sterling suggest using a utility method to address this problem:

I figured the utility method would be something like GUtil.toLowerCase(String) that would just call toLowerCase(Locale.ENGLISH) and GUtil.toLowerCaseLocaleSensitive(String) that would call toLowerCase(). The first case is what we would always try to use and the other would only be used in cases where we should be producing a locale dependent message/file/whatever (it doesn't have to be in GUtil)

A quick search in the codebase reveal lots of potential areas that could be affected.

Siedlerchr commented 7 years ago

It is advisable to use Locale.ROOT for case sensitive work: https://garygregory.wordpress.com/2015/11/03/java-lowercase-conversion-turkey/

big-guy commented 3 years ago

This is still an issue.

git grep toLowerCase\(\) | grep main | wc -l shows over 100 instances where this could be a problem.

Vampire commented 3 years ago

Most prominent place: grafik :-D

LouisCAD commented 3 years ago

There's also toUpperCase to check 😄

mlopatkin commented 3 years ago

Another instances of this problem: #17361, #17383

wolfs commented 3 years ago

I suppose writing an architecture test with archunit should also be pretty straightforward. Spotbugs also finds these problems.

Marcono1234 commented 1 year ago

Might also be worth to consider integrating https://github.com/policeman-tools/forbidden-apis in this context? It can detect these methods and also other methods which implicitly depend on the OS settings.

ov7a commented 8 months ago

This issue should address all the current instances in a unified way.

For adding a code quality check, see: