Skip to content

Fix: MXP parser blocked on non-escaped '&' and '<' characters#8274

Merged
vadi2 merged 8 commits intoMudlet:developmentfrom
NicolasKeita:development
Oct 21, 2025
Merged

Fix: MXP parser blocked on non-escaped '&' and '<' characters#8274
vadi2 merged 8 commits intoMudlet:developmentfrom
NicolasKeita:development

Conversation

@NicolasKeita
Copy link
Copy Markdown
Contributor

@NicolasKeita NicolasKeita commented Sep 23, 2025 •
edited
Loading

Motivation for adding to Mudlet

This pull request addresses issue #7896.

Problem re-explained

Currently if the MUD game sends a non-escaped "&" or "<" the MXP parser will consider what follows as a "tag" or an "entity" and therefore will not display it on the screen.

MUD sends ->        "Greetings heroes & villains\n"
Mudlet displays ->  "Greetings heroes"

Sometimes it would even freeze.

This behavior usually results from the MUD game not escaping characters properly.

Brief overview of PR changes/additions

  1. If an entity (something that starts with '&') has non legal characters (spaces or newline for example), immediately display the text as raw text.
"Heroes & Villains\n" (a space is following the '&' -> malformed entity detected)

  1. If a tag (something that starts with '<') is never closed after 3seconds, display the text that follows as raw text.

  2. While looking for a closing '>' if the parser encounters another '<', consider the first '<' as malformed and immediately display the text as raw text.

MUD sends ->        "Enemies < Your hero <!EN validTag \"sword\">\n"
Mudlet displays ->  "Enemies < Your hero"

Tests

Additions 1. and 3. have unit tests.

Addition 2 (the change in TBuffer::translateToPlainText()) currently requires one of the following :

either a manual test :

void TMainConsole::printOnDisplay(std::string& incomingSocketData, const bool isFromServer)
{
    Q_ASSERT_X(mpLineEdit_networkLatency, "TMainConsole::printOnDisplay(...)", "mpLineEdit_networkLatency does not point to a valid QLineEdit");
    mProcessingTimer.restart();

      // Add this snippet
    static bool mxpPrefixAlreadySent = false;

    if (mpHost->mTelnet.isMXPEnabled() && !mxpPrefixAlreadySent) {
        mxpPrefixAlreadySent = true;
        incomingSocketData = "heroes < villains" + incomingSocketData;
    }

or

a test that would use a newly created stub of the class TBuffer
(which is currently too dependent on Host* and TConsole*)

or

a functional test (would need to modify the CMakeLists to create a big static lib and link it to the test)

Contact

@spynight on discord or
nicolaskeita2@gmail.com

/claim #7896
Fixes #7896

@NicolasKeita NicolasKeita requested a review from a team as a code owner September 23, 2025 18:38
@algora-pbc algora-pbc bot mentioned this pull request Sep 23, 2025
Closed
@mpconley mpconley self-assigned this Sep 25, 2025
@mpconley mpconley added this to the 4.20.0 milestone Sep 27, 2025
@mpconley
Copy link
Copy Markdown
Contributor

mpconley commented Sep 27, 2025

Hello, this one has issues with normal MXP. You are welcome to connect to this test port stickmud.com 6680 to try it out. Perhaps you'll need to introduce the code slowly to see where things are going wrong.

Screenshot 2025-09-27 at 5 59 12 PM

@NicolasKeita
Copy link
Copy Markdown
Contributor Author

NicolasKeita commented Sep 28, 2025 •
edited
Loading

Hi,

I pushed a commit that fixes the issue. Sorry for the inconvenience.
StickMUD sent a '<' inside quotes (<!ELEMENT RDesc '<p>' FLAG="RoomDesc">) which was not handled correctly by my code.

Change in TMxpProcessingResult TMxpProcessor::processMxpInput():

  before: if (ch == '<' && mMxpTagBuilder.isInsideTag())
  after:  if (ch == '<' && mMxpTagBuilder.isInsideTag() && !mMxpTagBuilder.isQuotedSequence())

Additionally, I added more unit tests to cover all kinds of MXP strings (including this one) and manually tested various games in the default MUD list.

@mpconley
Copy link
Copy Markdown
Contributor

mpconley commented Sep 29, 2025

It did work better with StickMUD. Sorting a few things out to determine if they are this PR or another. I made an MXP test room for StickMUD, but noted that StickMUD already was handling those scenarios since it is MXP ready.

Reviewing as time permits. Finding the places in the other default MUDs that broke will be a side quest POC as well.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Oct 10, 2025

Good with this one @mpconley ?

@NicolasKeita
Copy link
Copy Markdown
Contributor Author

NicolasKeita commented Oct 16, 2025 •
edited
Loading

Once this PR is reviewed, I can move forward with either preparing the groundwork for the mobile version (#8030) or implementing functional tests to prevent regressions. Plenty of opportunities to keep improving from here.

Edit : I fixed the merge conflits.

@mpconley
Copy link
Copy Markdown
Contributor

mpconley commented Oct 19, 2025

Looking to find default MUDs that had this issue to test on.

Tested 7afba92. This passed my StickMUD test room. The game is very resilient already via its parser: mxp_test_room_results.txt

@mpconley
Copy link
Copy Markdown
Contributor

mpconley commented Oct 19, 2025

@taranion have any places to test this?

@taranion
Copy link
Copy Markdown

taranion commented Oct 19, 2025

@mpconley Hm, I fixed my previously broken MXP with & in the output.

But I downloaded the Mudlet-4.19.1-testing-pr8274-7afba926-linux-x64.AppImage.tar snapshot and recreated the server bug locally and can confirm that Mudlet deals with it without problems now.

@mpconley
Copy link
Copy Markdown
Contributor

mpconley commented Oct 20, 2025

This one may also close #3907, or this plus the other things we've done with MXP for the upcoming release.

@vadi2 vadi2 merged commit 32332f6 into Mudlet:development Oct 21, 2025
12 checks passed
@mpconley mpconley mentioned this pull request Feb 8, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vadi2 vadi2 vadi2 approved these changes

Assignees

@mpconley mpconley

Labels

🙋 Bounty claim

Projects

None yet

Milestone

4.20.0

Development

Successfully merging this pull request may close these issues.

Detect MXP issues and offer self-healing options

4 participants

@NicolasKeita @mpconley @vadi2 @taranion