Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

minimessage: Parsing corner cases with quotes #819

Merged
merged 3 commits into from Nov 7, 2022
Merged

minimessage: Parsing corner cases with quotes #819

merged 3 commits into from Nov 7, 2022

Commits on Nov 7, 2022

  1. fix(minimessage): Ignore quotes with no matching closing quote 

    Don't enter string parsing mode during the first pass when encountering
a quote, but no matching quote exists later on in the message.

Note that this doesn't catch every case of invalid tags causing other
valid tags to be ignored, in this case there could simply be an
unrelated quote elsewhere in the message and the bug would still occur.

The parser could:

  * Only allow quotes after specific characters, such as the colon `:`,
    which is currently the only place where quotes are really valid.
    However, even then, the buggy payload could just include a colon
    before the quote

  * Only allow quotes when inside of a valid tag, as in the test.
    (`<3` isn't a valid introducer for a tag). However, even then, the
    buggy payload could use a character valid for tag. Additionally,
    checking the validity of a tag isn't done by the first parse stage

  * Backtrack when a quote-delimited section doesn't end up being
    inside of a valid tag. That would (presumably) require some pretty
    complicated checks in the first phase, but checking for it in the
    second phase is probably too late, because it'd have to reparse
    something it has already passed by
    @rymiel
    rymiel committed Nov 7, 2022
    Configuration menu
    Copy the full SHA
    3890d27 View commit details
    Browse the repository at this point in the history

  2. fix(minimessage): Backtrack when the content of a tag never ends 

    A tag is opened by `<` and closed by `>`. Upon encountering an opening
angle bracket `<`, the parser enters the "TAG" state, during which it'll
parse quote-delimited sequences. However, if by the end of the input,
a closing angle bracket `>` is never encountered, the parser simply
adds everything as a single text node, including the contents of the
quote-delimited sequences.

These quote delimited sequences could have contained other valid tags
that are now directly added to the output without being parsed as tags.

With this change, when the parser finds itself at the end of the input
in the TAG state, it will backtrack back to the beginning of the `<`
that caused a state change, and parse again from there on, but this time
*not* in the tag state. This means that quotes are effectively ignored
and their contents parsed as normal text

This *still* doesn't account for all corner case, for example:

`<red>foo <tag:"</red>"> bar`. This is an example of a *correct* use of
quotes, and the expected behaviour is for the whole output to be red,
and the actual behaviour matches that, however:

`<red>foo <3:"</red>"> bar`. This is nearly identical to the above, but
<3> isn't a valid tag, which means the red tag should be closed in the
middle of the message. Instead, the message ends up wholly red.

`<red>foo <tag"</red>"> bar`. The same applies here, but instead of an
invalid tag, it is invalid syntax, since quotes should follow a colon.
    @rymiel
    rymiel committed Nov 7, 2022
    Configuration menu
    Copy the full SHA
    108dc34 View commit details
    Browse the repository at this point in the history

  3. Apply suggestion from review 

    Co-authored-by: zml <zml@stellardrift.ca>
    @rymiel @zml2008
    rymiel and zml2008 committed Nov 7, 2022
    Configuration menu
    Copy the full SHA
    8b71690 View commit details
    Browse the repository at this point in the history