Skip to content

Commit

Permalink
Always specify Locale on toLowerCase and toUpperCase usages, fixes Au…
Browse files Browse the repository at this point in the history
…thMe not working correctly on machines with turkish locale. ('I'.toLowerCase() => 'ı')
  • Loading branch information
sgdc3 committed Aug 20, 2022
1 parent c38e2ab commit 75b3a57
Show file tree
Hide file tree
Showing 67 changed files with 217 additions and 143 deletions.
5 changes: 3 additions & 2 deletions src/main/java/fr/xephi/authme/api/v3/AuthMeApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.Locale;
import java.util.Optional;

/**
Expand Down Expand Up @@ -217,7 +218,7 @@ private Long getLastLoginMillis(String playerName) {
* @return true if player is registered, false otherwise
*/
public boolean isRegistered(String playerName) {
String player = playerName.toLowerCase();
String player = playerName.toLowerCase(Locale.ROOT);
return dataSource.isAuthAvailable(player);
}

Expand All @@ -241,7 +242,7 @@ public boolean checkPassword(String playerName, String passwordToCheck) {
* @return true if the player was registered successfully
*/
public boolean registerPlayer(String playerName, String password) {
String name = playerName.toLowerCase();
String name = playerName.toLowerCase(Locale.ROOT);
if (isRegistered(name)) {
return false;
}
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/fr/xephi/authme/command/CommandMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;

import static fr.xephi.authme.command.FoundResultStatus.INCORRECT_ARGUMENTS;
Expand Down Expand Up @@ -130,7 +131,7 @@ private static FoundCommandResult getCommandWithSmallestDifference(CommandDescri
}

private CommandDescription getBaseCommand(String label) {
String baseLabel = label.toLowerCase();
String baseLabel = label.toLowerCase(Locale.ROOT);
if (baseLabel.startsWith("authme:")) {
baseLabel = baseLabel.substring("authme:".length());
}
Expand All @@ -157,7 +158,7 @@ private static CommandDescription getSuitableChild(CommandDescription baseComman
return null;
}

final String label = parts.get(0).toLowerCase();
final String label = parts.get(0).toLowerCase(Locale.ROOT);
final int argumentCount = parts.size() - 1;

for (CommandDescription child : baseCommand.getChildren()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import javax.inject.Inject;
import java.util.List;
import java.util.Locale;

/**
* Shows all accounts registered by the same IP address for the given player name or IP address.
Expand Down Expand Up @@ -44,7 +45,7 @@ public void executeCommand(final CommandSender sender, List<String> arguments) {
});
} else {
bukkitService.runTaskAsynchronously(() -> {
PlayerAuth auth = dataSource.getAuth(playerName.toLowerCase());
PlayerAuth auth = dataSource.getAuth(playerName.toLowerCase(Locale.ROOT));
if (auth == null) {
commonService.send(sender, MessageKey.UNKNOWN_USER);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import javax.inject.Inject;
import java.util.List;
import java.util.Locale;
import java.util.Map;

/**
Expand Down Expand Up @@ -71,7 +72,7 @@ public void executeCommand(CommandSender sender, List<String> arguments) {
private static Class<? extends Converter> getConverterClassFromArgs(List<String> arguments) {
return arguments.isEmpty()
? null
: CONVERTERS.get(arguments.get(0).toLowerCase());
: CONVERTERS.get(arguments.get(0).toLowerCase(Locale.ROOT));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import javax.inject.Inject;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;

/**
Expand All @@ -29,7 +30,7 @@ public void executeCommand(CommandSender sender, List<String> arguments) {
Set<OfflinePlayer> bannedPlayers = bukkitService.getBannedPlayers();
Set<String> namedBanned = new HashSet<>(bannedPlayers.size());
for (OfflinePlayer offlinePlayer : bannedPlayers) {
namedBanned.add(offlinePlayer.getName().toLowerCase());
namedBanned.add(offlinePlayer.getName().toLowerCase(Locale.ROOT));
}

purgeService.purgePlayers(sender, namedBanned, bannedPlayers.toArray(new OfflinePlayer[bannedPlayers.size()]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import javax.inject.Inject;
import java.util.List;
import java.util.Locale;

import static java.util.Collections.singletonList;

Expand Down Expand Up @@ -36,7 +37,7 @@ public void executeCommand(CommandSender sender, List<String> arguments) {
private void executeCommand(CommandSender sender, String name, String option) {
if ("force".equals(option) || !dataSource.isAuthAvailable(name)) {
OfflinePlayer offlinePlayer = bukkitService.getOfflinePlayer(name);
purgeExecutor.executePurge(singletonList(offlinePlayer), singletonList(name.toLowerCase()));
purgeExecutor.executePurge(singletonList(offlinePlayer), singletonList(name.toLowerCase(Locale.ROOT)));
sender.sendMessage("Purged data for player " + name);
} else {
sender.sendMessage("This player is still registered! Are you sure you want to proceed? "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import javax.inject.Inject;
import java.util.List;
import java.util.Locale;

/**
* Admin command to register a user.
Expand Down Expand Up @@ -45,7 +46,7 @@ public void executeCommand(final CommandSender sender, List<String> arguments) {
// Get the player name and password
final String playerName = arguments.get(0);
final String playerPass = arguments.get(1);
final String playerNameLowerCase = playerName.toLowerCase();
final String playerNameLowerCase = playerName.toLowerCase(Locale.ROOT);

// Command logic
ValidationResult passwordValidation = validationService.validatePassword(playerPass, playerName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import javax.inject.Inject;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
Expand Down Expand Up @@ -45,7 +46,7 @@ private DebugSection findDebugSection(List<String> arguments) {
if (arguments.isEmpty()) {
return null;
}
return getSections().get(arguments.get(0).toLowerCase());
return getSections().get(arguments.get(0).toLowerCase(Locale.ROOT));
}

private void sendAvailableSections(CommandSender sender) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import javax.inject.Inject;
import java.util.List;
import java.util.Locale;

/**
* The command for a player to change his password with.
Expand All @@ -35,7 +36,7 @@ public class ChangePasswordCommand extends PlayerCommand {

@Override
public void runCommand(Player player, List<String> arguments) {
String name = player.getName().toLowerCase();
String name = player.getName().toLowerCase(Locale.ROOT);

if (!playerCache.isAuthenticated(name)) {
commonService.send(player, MessageKey.NOT_LOGGED_IN);
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/fr/xephi/authme/data/ProxySessionManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import fr.xephi.authme.util.expiring.ExpiringSet;

import javax.inject.Inject;
import java.util.Locale;
import java.util.concurrent.TimeUnit;

public class ProxySessionManager implements HasCleanup {
Expand All @@ -21,7 +22,7 @@ public ProxySessionManager() {
* @param name the player's name
*/
private void setActiveSession(String name) {
activeProxySessions.add(name.toLowerCase());
activeProxySessions.add(name.toLowerCase(Locale.ROOT));
}

/**
Expand Down
13 changes: 7 additions & 6 deletions src/main/java/fr/xephi/authme/data/VerificationCodeManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import javax.inject.Inject;
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -82,7 +83,7 @@ public boolean isCodeRequired(String name) {
* @return true if the player has been verified, false otherwise
*/
private boolean isPlayerVerified(String name) {
return verifiedPlayers.contains(name.toLowerCase());
return verifiedPlayers.contains(name.toLowerCase(Locale.ROOT));
}

/**
Expand All @@ -92,7 +93,7 @@ private boolean isPlayerVerified(String name) {
* @return true if the code exists, false otherwise
*/
public boolean hasCode(String name) {
return (verificationCodes.get(name.toLowerCase()) != null);
return (verificationCodes.get(name.toLowerCase(Locale.ROOT)) != null);
}

/**
Expand Down Expand Up @@ -135,7 +136,7 @@ private void generateCode(String name) {
final String email = emailResult.getValue();
if (!Utils.isEmailEmpty(email)) {
String code = RandomStringUtils.generateNum(6); // 6 digits code
verificationCodes.put(name.toLowerCase(), code);
verificationCodes.put(name.toLowerCase(Locale.ROOT), code);
emailService.sendVerificationMail(name, email, code);
}
}
Expand All @@ -150,7 +151,7 @@ private void generateCode(String name) {
*/
public boolean checkCode(String name, String code) {
boolean correct = false;
if (code.equals(verificationCodes.get(name.toLowerCase()))) {
if (code.equals(verificationCodes.get(name.toLowerCase(Locale.ROOT)))) {
verify(name);
correct = true;
}
Expand All @@ -163,7 +164,7 @@ public boolean checkCode(String name, String code) {
* @param name the name of the player to generate a code for
*/
public void verify(String name) {
verifiedPlayers.add(name.toLowerCase());
verifiedPlayers.add(name.toLowerCase(Locale.ROOT));
}

/**
Expand All @@ -172,7 +173,7 @@ public void verify(String name) {
* @param name the name of the player to generate a code for
*/
public void unverify(String name){
verifiedPlayers.remove(name.toLowerCase());
verifiedPlayers.remove(name.toLowerCase(Locale.ROOT));
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/fr/xephi/authme/data/auth/PlayerAuth.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import fr.xephi.authme.security.crypts.HashedPassword;
import org.bukkit.Location;

import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.UUID;
Expand Down Expand Up @@ -54,7 +55,7 @@ private PlayerAuth() {


public void setNickname(String nickname) {
this.nickname = nickname.toLowerCase();
this.nickname = nickname.toLowerCase(Locale.ROOT);
}

public String getNickname() {
Expand Down Expand Up @@ -239,7 +240,7 @@ public static final class Builder {
*/
public PlayerAuth build() {
PlayerAuth auth = new PlayerAuth();
auth.nickname = checkNotNull(name).toLowerCase();
auth.nickname = checkNotNull(name).toLowerCase(Locale.ROOT);
auth.realName = Optional.ofNullable(realName).orElse("Player");
auth.password = Optional.ofNullable(password).orElse(new HashedPassword(""));
auth.totpKey = totpKey;
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/fr/xephi/authme/data/auth/PlayerCache.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fr.xephi.authme.data.auth;


import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

Expand All @@ -20,7 +21,7 @@ public class PlayerCache {
* @param auth the player auth object to save
*/
public void updatePlayer(PlayerAuth auth) {
cache.put(auth.getNickname().toLowerCase(), auth);
cache.put(auth.getNickname().toLowerCase(Locale.ROOT), auth);
}

/**
Expand All @@ -29,7 +30,7 @@ public void updatePlayer(PlayerAuth auth) {
* @param user name of the player to remove
*/
public void removePlayer(String user) {
cache.remove(user.toLowerCase());
cache.remove(user.toLowerCase(Locale.ROOT));
}

/**
Expand All @@ -40,7 +41,7 @@ public void removePlayer(String user) {
* @return true if player is logged in, false otherwise.
*/
public boolean isAuthenticated(String user) {
return cache.containsKey(user.toLowerCase());
return cache.containsKey(user.toLowerCase(Locale.ROOT));
}

/**
Expand All @@ -51,7 +52,7 @@ public boolean isAuthenticated(String user) {
* @return the associated auth object, or null if not available
*/
public PlayerAuth getAuth(String user) {
return cache.get(user.toLowerCase());
return cache.get(user.toLowerCase(Locale.ROOT));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import fr.xephi.authme.util.RandomStringUtils;
import fr.xephi.authme.util.expiring.ExpiringMap;

import java.util.Locale;
import java.util.concurrent.TimeUnit;

/**
Expand Down Expand Up @@ -51,7 +52,7 @@ public void setCaptchaLength(int captchaLength) {
* @return the code the player is required to enter
*/
public String getCodeOrGenerateNew(String name) {
String code = captchaCodes.get(name.toLowerCase());
String code = captchaCodes.get(name.toLowerCase(Locale.ROOT));
return code == null ? generateCode(name) : code;
}

Expand All @@ -63,7 +64,7 @@ public String getCodeOrGenerateNew(String name) {
*/
private String generateCode(String name) {
String code = RandomStringUtils.generate(captchaLength);
captchaCodes.put(name.toLowerCase(), code);
captchaCodes.put(name.toLowerCase(Locale.ROOT), code);
return code;
}

Expand All @@ -76,7 +77,7 @@ private String generateCode(String name) {
* @return true if the code matches, false otherwise
*/
public boolean checkCode(String name, String code) {
String nameLowerCase = name.toLowerCase();
String nameLowerCase = name.toLowerCase(Locale.ROOT);
String savedCode = captchaCodes.get(nameLowerCase);
if (savedCode != null && savedCode.equalsIgnoreCase(code)) {
captchaCodes.remove(nameLowerCase);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.bukkit.entity.Player;

import javax.inject.Inject;
import java.util.Locale;
import java.util.concurrent.TimeUnit;

/**
Expand Down Expand Up @@ -36,14 +37,14 @@ public class LoginCaptchaManager implements CaptchaManager, SettingsDependent, H
*/
public void increaseLoginFailureCount(String name) {
if (isEnabled) {
String playerLower = name.toLowerCase();
String playerLower = name.toLowerCase(Locale.ROOT);
playerCounts.increment(playerLower);
}
}

@Override
public boolean isCaptchaRequired(String playerName) {
return isEnabled && playerCounts.get(playerName.toLowerCase()) >= threshold;
return isEnabled && playerCounts.get(playerName.toLowerCase(Locale.ROOT)) >= threshold;
}

@Override
Expand All @@ -53,7 +54,7 @@ public String getCaptchaCodeOrGenerateNew(String name) {

@Override
public boolean checkCode(Player player, String code) {
String nameLower = player.getName().toLowerCase();
String nameLower = player.getName().toLowerCase(Locale.ROOT);
boolean isCodeCorrect = captchaCodeStorage.checkCode(nameLower, code);
if (isCodeCorrect) {
playerCounts.remove(nameLower);
Expand All @@ -68,7 +69,7 @@ public boolean checkCode(Player player, String code) {
*/
public void resetLoginFailureCount(String name) {
if (isEnabled) {
playerCounts.remove(name.toLowerCase());
playerCounts.remove(name.toLowerCase(Locale.ROOT));
}
}

Expand Down
Loading

4 comments on commit 75b3a57

@sgdc3
Copy link
Member Author

@sgdc3 sgdc3 commented on 75b3a57 Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljacqu Hello, do you have any suggestion on how to enforce this good practice in the future?

@ljacqu
Copy link
Member

@ljacqu ljacqu commented on 75b3a57 Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @sgdc3, I spontaneously thought about Checkstyle but they don't seem to have a check for that. Quick win could be a pattern check like we have for "TODO" comments.
More long-term maybe ArchUnit, but not sure that's worth introducing

@sgdc3
Copy link
Member Author

@sgdc3 sgdc3 commented on 75b3a57 Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljacqu how did the TODO comments pattern check work?

@ljacqu
Copy link
Member

@ljacqu ljacqu commented on 75b3a57 Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO comments actually have a dedicated check; I didn't remember correctly. But I PRed what I mean in PR #2606

Please sign in to comment.