Skip to content

Commit fd9296a

Browse files
committed
fix: Brigadier hate @ it seems, let's fix that
1 parent 19424ad commit fd9296a

4 files changed

Lines changed: 64 additions & 9 deletions

File tree

authme-core/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import fr.xephi.authme.message.MessageKey;
55
import fr.xephi.authme.process.Management;
66
import fr.xephi.authme.service.CommonService;
7+
import fr.xephi.authme.service.ValidationService;
78
import org.bukkit.entity.Player;
89

910
import javax.inject.Inject;
@@ -20,13 +21,17 @@ public class AddEmailCommand extends PlayerCommand {
2021
@Inject
2122
private CommonService commonService;
2223

24+
@Inject
25+
private ValidationService validationService;
26+
2327
@Override
2428
public void runCommand(Player player, List<String> arguments) {
2529
String email = arguments.get(0);
2630
String emailConfirmation = arguments.get(1);
2731

28-
if (email.equals(emailConfirmation)) {
29-
// Closer inspection of the mail address handled by the async task
32+
if (!validationService.validateEmail(email)) {
33+
commonService.send(player, MessageKey.INVALID_EMAIL);
34+
} else if (email.equals(emailConfirmation)) {
3035
management.performAddEmail(player, email);
3136
} else {
3237
commonService.send(player, MessageKey.CONFIRM_EMAIL_MESSAGE);

authme-core/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import fr.xephi.authme.message.Messages;
99
import fr.xephi.authme.process.Management;
1010
import fr.xephi.authme.service.CommonService;
11+
import fr.xephi.authme.service.ValidationService;
1112
import org.bukkit.command.BlockCommandSender;
1213
import org.bukkit.command.CommandSender;
1314
import org.bukkit.entity.Player;
@@ -20,6 +21,7 @@
2021

2122
import static org.hamcrest.Matchers.equalTo;
2223
import static org.hamcrest.MatcherAssert.assertThat;
24+
import static org.mockito.BDDMockito.given;
2325
import static org.mockito.Mockito.mock;
2426
import static org.mockito.Mockito.verify;
2527
import static org.mockito.Mockito.verifyNoInteractions;
@@ -43,6 +45,9 @@ public class AddEmailCommandTest {
4345
@Mock
4446
private Messages messages;
4547

48+
@Mock
49+
private ValidationService validationService;
50+
4651
@Test
4752
public void shouldRejectNonPlayerSender() {
4853
// given
@@ -59,7 +64,8 @@ public void shouldRejectNonPlayerSender() {
5964
public void shouldForwardData() {
6065
// given
6166
Player sender = mock(Player.class);
62-
String email = "mail@example";
67+
String email = "mail@example.com";
68+
given(validationService.validateEmail(email)).willReturn(true);
6369

6470
// when
6571
command.executeCommand(sender, Arrays.asList(email, email));
@@ -68,11 +74,27 @@ public void shouldForwardData() {
6874
verify(management).performAddEmail(sender, email);
6975
}
7076

77+
@Test
78+
public void shouldFailForInvalidEmailFormat() {
79+
// given
80+
Player sender = mock(Player.class);
81+
String email = "notanemail";
82+
given(validationService.validateEmail(email)).willReturn(false);
83+
84+
// when
85+
command.executeCommand(sender, Arrays.asList(email, email));
86+
87+
// then
88+
verifyNoInteractions(management);
89+
verify(commandService).send(sender, MessageKey.INVALID_EMAIL);
90+
}
91+
7192
@Test
7293
public void shouldFailForConfirmationMismatch() {
7394
// given
7495
Player sender = mock(Player.class);
7596
String email = "asdfasdf@example.com";
97+
given(validationService.validateEmail(email)).willReturn(true);
7698

7799
// when
78100
command.executeCommand(sender, Arrays.asList(email, "wrongConf"));

authme-paper-common/src/main/java/fr/xephi/authme/platform/PaperBrigadierCommandRegistrar.java

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package fr.xephi.authme.platform;
22

3+
import com.mojang.brigadier.arguments.ArgumentType;
34
import com.mojang.brigadier.arguments.StringArgumentType;
45
import com.mojang.brigadier.builder.ArgumentBuilder;
56
import com.mojang.brigadier.builder.LiteralArgumentBuilder;
67
import com.mojang.brigadier.builder.RequiredArgumentBuilder;
78
import com.mojang.brigadier.context.CommandContext;
9+
import com.mojang.brigadier.tree.CommandNode;
810
import fr.xephi.authme.AuthMe;
911
import fr.xephi.authme.command.CommandArgumentDescription;
1012
import fr.xephi.authme.command.CommandDescription;
@@ -82,24 +84,43 @@ private LiteralArgumentBuilder<CommandSourceStack> buildLiteralNode(String label
8284

8385
private void addArgumentChain(ArgumentBuilder<CommandSourceStack, ?> parent,
8486
List<CommandArgumentDescription> arguments) {
85-
ArgumentBuilder<CommandSourceStack, ?> currentParent = parent;
86-
for (int index = 0; index < arguments.size(); ++index) {
87+
if (arguments.isEmpty()) {
88+
return;
89+
}
90+
// Build from innermost argument outward: then(ArgumentBuilder) calls build() immediately
91+
// in standard Brigadier, so intermediate nodes must have their children set before building.
92+
CommandNode<CommandSourceStack> innerChain = null;
93+
for (int index = arguments.size() - 1; index >= 0; --index) {
8794
CommandArgumentDescription argument = arguments.get(index);
8895
boolean isLastArgument = index == arguments.size() - 1;
89-
RequiredArgumentBuilder<CommandSourceStack, String> argumentBuilder =
96+
RequiredArgumentBuilder<CommandSourceStack, String> argBuilder =
9097
RequiredArgumentBuilder.<CommandSourceStack, String>argument(argument.getName(),
91-
isLastArgument ? StringArgumentType.greedyString() : StringArgumentType.word())
98+
isLastArgument ? StringArgumentType.greedyString() : anyWord())
9299
.executes(this::executeInput);
93-
currentParent.then(argumentBuilder);
94-
currentParent = argumentBuilder;
100+
if (innerChain != null) {
101+
argBuilder.then(innerChain);
102+
}
103+
innerChain = argBuilder.build();
95104
}
105+
parent.then(innerChain);
96106
}
97107

98108
private RequiredArgumentBuilder<CommandSourceStack, String> createFallbackArgument(String name) {
99109
return RequiredArgumentBuilder.<CommandSourceStack, String>argument(name, StringArgumentType.greedyString())
100110
.executes(this::executeInput);
101111
}
102112

113+
/** Reads any non-whitespace characters — unlike {@code word()}, accepts {@code @}, {@code #}, etc. */
114+
private static ArgumentType<String> anyWord() {
115+
return reader -> {
116+
int start = reader.getCursor();
117+
while (reader.canRead() && reader.peek() != ' ') {
118+
reader.skip();
119+
}
120+
return reader.getString().substring(start, reader.getCursor());
121+
};
122+
}
123+
103124
private int executeInput(CommandContext<CommandSourceStack> context) {
104125
commandExecutor.apply(context.getSource().getSender(), CommandUtils.splitInput(context.getInput()));
105126
return COMMAND_SUCCESS;

authme-paper-common/src/test/java/fr/xephi/authme/platform/PaperBrigadierCommandRegistrarTest.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ public void shouldDelegateExtraArgumentsToExistingHandler() throws Exception {
100100
assertThat(executedCommands, contains(List.of("login", "password", "extra")));
101101
}
102102

103+
@Test
104+
public void shouldHandleAtSignInNonLastArgument() throws Exception {
105+
dispatcher.execute("authme register test@gmail.com myPassword", sourceStack);
106+
107+
assertThat(executedCommands, contains(List.of("authme", "register", "test@gmail.com", "myPassword")));
108+
}
109+
103110
@Test
104111
public void shouldRegisterChildAliasesInBrigadierTree() {
105112
assertThat(dispatcher.getCompletionSuggestions(dispatcher.parse("authme ", sourceStack)).join()

0 commit comments

Comments
 (0)