Skip to content

Don't let an impermissible literal shadow an overlapping argument node#163

Open
WouterGritter wants to merge 1 commit into
Mojang:masterfrom
WouterGritter:fix-impermissible-literal-shadows-argument
Open

Don't let an impermissible literal shadow an overlapping argument node#163
WouterGritter wants to merge 1 commit into
Mojang:masterfrom
WouterGritter:fix-impermissible-literal-shadows-argument

Conversation

@WouterGritter

Copy link
Copy Markdown

CommandNode#getRelevantNodes short-circuits to a literal as soon as its name matches the next token, without considering whether the source can use it:

final LiteralCommandNode<S> literal = literals.get(text);
if (literal != null) {
  return Collections.singleton(literal);
} else {
  return arguments.values();
}

parseNodes then filters that node out with canUse(source) and moves on:

for (final CommandNode<S> child : node.getRelevantNodes(originalReader)) {
  if (!child.canUse(source)) {
      continue;
  }
  // ...
}

Because the relevant-node set was the matched literal alone, there is nothing to fall back to; the argument siblings are never tried. So if a literal (e.g. bar, gated behind requires(...)) sits next to an argument node whose text overlaps it (e.g. ), a source that can't use bar can't reach either, even though the argument would happily accept the input. A permission-gated literal silently makes an overlapping argument unreachable.

Fix

Add a source-aware getRelevantNodes(StringReader, S source) overload that only short-circuits to the matched literal when the source can actually use it; otherwise it falls back to the argument nodes. parseNodes now calls this overload.

public Collection<? extends CommandNode<S>> getRelevantNodes(final StringReader input, final S source) {
  if (literals.size() > 0) {
      final LiteralCommandNode<S> literal = getMatchingLiteral(input);
      if (literal != null && literal.canUse(source)) {
          return Collections.singleton(literal);
      }
  }
  return arguments.values();
}

The crucial property: when the literal is usable, the result is identical to before (singleton(literal)), so literal priority and the short-circuit are fully preserved. The fallback only kicks in for the shadowing case. This avoids the regression that "always return literal + arguments" would cause, where e.g. a greedy-string sibling could start beating a previously-prioritised literal in the potentials sort.

`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant