Skip to content

Commit f5ceaca

Browse files
committed
Don't let an impermissible literal shadow an overlapping argument node
`CommandNode#getRelevantNodes` short-circuits to a literal whose name matches the next token regardless of whether the source can use it. `parseNodes` then skips that literal via canUse and, because the relevant-node set was the literal alone, never falls back to the argument siblings, making an argument node whose text overlaps the literal unreachable for any source lacking the literal's permission. Add a source-aware `getRelevantNodes(StringReader, source)` overload that only short-circuits to the matched literal when the source can use it, otherwise falling back to the argument nodes. The permitted case is byte-for-byte unchanged (still returns the single literal), so literal priority is preserved.
1 parent b5419b1 commit f5ceaca

3 files changed

Lines changed: 55 additions & 12 deletions

File tree

src/main/java/com/mojang/brigadier/CommandDispatcher.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ private ParseResults<S> parseNodes(final CommandNode<S> node, final StringReader
297297
List<ParseResults<S>> potentials = null;
298298
final int cursor = originalReader.getCursor();
299299

300-
for (final CommandNode<S> child : node.getRelevantNodes(originalReader)) {
300+
for (final CommandNode<S> child : node.getRelevantNodes(originalReader, source)) {
301301
if (!child.canUse(source)) {
302302
continue;
303303
}

src/main/java/com/mojang/brigadier/tree/CommandNode.java

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -152,21 +152,32 @@ public Predicate<S> getRequirement() {
152152

153153
public Collection<? extends CommandNode<S>> getRelevantNodes(final StringReader input) {
154154
if (literals.size() > 0) {
155-
final int cursor = input.getCursor();
156-
while (input.canRead() && input.peek() != ' ') {
157-
input.skip();
158-
}
159-
final String text = input.getString().substring(cursor, input.getCursor());
160-
input.setCursor(cursor);
161-
final LiteralCommandNode<S> literal = literals.get(text);
155+
final LiteralCommandNode<S> literal = getMatchingLiteral(input);
162156
if (literal != null) {
163157
return Collections.singleton(literal);
164-
} else {
165-
return arguments.values();
166158
}
167-
} else {
168-
return arguments.values();
169159
}
160+
return arguments.values();
161+
}
162+
163+
public Collection<? extends CommandNode<S>> getRelevantNodes(final StringReader input, final S source) {
164+
if (literals.size() > 0) {
165+
final LiteralCommandNode<S> literal = getMatchingLiteral(input);
166+
if (literal != null && literal.canUse(source)) {
167+
return Collections.singleton(literal);
168+
}
169+
}
170+
return arguments.values();
171+
}
172+
173+
private LiteralCommandNode<S> getMatchingLiteral(final StringReader input) {
174+
final int cursor = input.getCursor();
175+
while (input.canRead() && input.peek() != ' ') {
176+
input.skip();
177+
}
178+
final String text = input.getString().substring(cursor, input.getCursor());
179+
input.setCursor(cursor);
180+
return literals.get(text);
170181
}
171182

172183
@Override

src/test/java/com/mojang/brigadier/CommandDispatcherTest.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import static com.mojang.brigadier.arguments.IntegerArgumentType.getInteger;
2727
import static com.mojang.brigadier.arguments.IntegerArgumentType.integer;
28+
import static com.mojang.brigadier.arguments.StringArgumentType.word;
2829
import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal;
2930
import static com.mojang.brigadier.builder.RequiredArgumentBuilder.argument;
3031
import static org.hamcrest.Matchers.equalTo;
@@ -123,6 +124,37 @@ public void testExecuteImpermissibleCommand() {
123124
}
124125
}
125126

127+
@SuppressWarnings("unchecked")
128+
@Test
129+
public void testImpermissibleLiteralFallsBackToArgument() throws Exception {
130+
final Command<Object> argCommand = mock(Command.class);
131+
when(argCommand.run(any())).thenReturn(100);
132+
subject.register(literal("foo")
133+
.then(literal("bar").requires(s -> false).executes(command))
134+
.then(argument("value", word()).executes(argCommand)));
135+
136+
// The source cannot use the "bar" literal, so "foo bar" must fall back to the <value> argument
137+
// instead of failing because the impermissible literal shadowed it.
138+
assertThat(subject.execute("foo bar", source), is(100));
139+
verify(argCommand).run(any(CommandContext.class));
140+
verify(command, never()).run(any(CommandContext.class));
141+
}
142+
143+
@SuppressWarnings("unchecked")
144+
@Test
145+
public void testUsableLiteralTakesPriorityOverArgument() throws Exception {
146+
final Command<Object> argCommand = mock(Command.class);
147+
when(argCommand.run(any())).thenReturn(100);
148+
subject.register(literal("foo")
149+
.then(literal("bar").executes(command))
150+
.then(argument("value", word()).executes(argCommand)));
151+
152+
// "bar" matches a usable literal, which must keep priority over the overlapping <value> argument.
153+
assertThat(subject.execute("foo bar", source), is(42));
154+
verify(command).run(any(CommandContext.class));
155+
verify(argCommand, never()).run(any(CommandContext.class));
156+
}
157+
126158
@Test
127159
public void testExecuteEmptyCommand() {
128160
subject.register(literal(""));

0 commit comments

Comments
 (0)