Fix markdown formatting issue #1286

Zain-Mahmoud · 2026-01-21T01:43:29Z

Proposed Changes

This PR fixes a bug that caused user input containing certain markdown characters to be rendered as markdown by the markdown renderer (issue #1285)

...

Screenshots of your changes (if applicable)

Before:

After:

Type of Change

(Write an X or a brief description next to the type or types that best describe your changes.)

Type	Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)	x
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📚 Documentation update (change that only updates documentation)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

I have performed a self-review of my changes.
- Check that all changed files included in this pull request are intentional changes.
- Check that all changes are relevant to the purpose of this pull request, as described above.
I have added tests for my changes, if applicable.
- This is required for all bug fixes and new features.
I have updated the project documentation, if applicable.
- This is required for new features.
I have updated the project Changelog (this is required for all changes).
If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

I have verified that the pre-commit.ci checks have passed.
I have verified that the CI tests have passed.
I have reviewed the test coverage changes reported by Coveralls.
I have requested a review from a project maintainer.

Questions and Comments

david-yz-liu · 2026-01-24T15:32:46Z

packages/python-ta/src/python_ta/reporters/core.py

        This is used by our patched version of MessagesHandlerMixIn.add_message
        (see python_ta/patches/messages.py).
        """
+        msg_definition.msg = ExtendedMarkup(msg_definition.msg)


@Zain-Mahmoud I noticed in your presentation yesterday: this isn't the right place to do this conversion. Look for where the PythonTA error messages are first loaded; this is part of how we handle the configuration files, before starting the "linting" process.

coveralls · 2026-01-29T19:40:35Z

Pull Request Test Coverage Report for Build 21813417438

Details

8 of 8 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.02%) to 89.854%

Totals
Change from base Build 21736461175:	0.02%
Covered Lines:	3436
Relevant Lines:	3824

💛 - Coveralls

david-yz-liu · 2026-01-30T02:23:35Z

packages/python-ta/src/python_ta/check/helpers.py

            linter.msgs_store.register_message(message_definition)

+    if linter.reporter.name == "pyta-html":
+        msg_ids = linter.msgs_store.messages


This variable name is misleading as it's not just a list of ids that are being returned, but full "message" objects. Please pick a better variable name here and below for the loop variable.

david-yz-liu · 2026-01-30T02:23:49Z

packages/python-ta/src/python_ta/reporters/core.py

 from pylint.reporters import BaseReporter

 from .node_printers import LineType, render_message
+from python_ta.util.extended_markup import ExtendedMarkup


Revert all changes in this file

david-yz-liu · 2026-01-30T02:24:25Z

packages/python-ta/src/python_ta/reporters/html_reporter.py

 import os
 import socket
 import sys
+from html import unescape


Revert changes in this file

david-yz-liu · 2026-01-30T02:26:47Z

packages/python-ta/docs/usage/quick_start.md

 Try fixing the above code by adding spaces around the `+`, and replacing `print(result)` with `return result`.
 After making these changes, re-run the file: PythonTA's analysis should not find any more issues!

+Note: any "𝔄" characters in the source code will be rendered as an "&" in the error message's code snippet.


I looked into this a bit more and unicode has some Private Use Areas, which has code points that are guaranteed not to be used. We should be able to use one of these instead, rather than 𝔄. That way we don't need to mention this in the documentation either.

Note that you can use \u to write unicode code points directly in string literals (see here).

david-yz-liu · 2026-01-30T02:27:25Z

packages/python-ta/src/python_ta/util/extended_markup.py

+
+class ExtendedMarkup(Markup):
+    """
+    Extends the standard Markup class defined in markupsafe to include escaping of markdown characters


In general for docstrings, write one-line descriptions on the same line as the """. (Same for the escape method below.)

david-yz-liu · 2026-01-30T02:27:44Z

packages/python-ta/src/python_ta/util/extended_markup.py

+    """
+
+    @classmethod
+    def escape(cls, s):


Add type annotations here

david-yz-liu · 2026-01-30T02:28:14Z

packages/python-ta/src/python_ta/util/extended_markup.py

+        Escape all markdown characters in s by replacing them with their corresponding escape sequence
+        """
+        if isinstance(s, str):
+            markdown_chars = [


This can be pulled out and defined as a top-level constant

david-yz-liu · 2026-01-30T02:28:46Z

...s/test_html_server/test_no_watch_server_is_non_persistent/no_watch_html_server_snapshot.html

                                            <div class="message">
                                                <p class="message-name">
-                                                    <span> [Line 20] Forbidden top-level code found on line 20 </span>
+                                                    <span> [Line 20] Forbidden top-level code found on line None </span>


Be careful when updating snapshots. This test shouldn't have been updated, and indicates a bug in this implementation.

Zain-Mahmoud · 2026-02-05T04:57:36Z

Hi @david-yz-liu,
I wrote a test for the script injection. Just wanted to make sure I'm on the right track before I write more tests

packages/python-ta/tests/test_reporters/test_html_reporter.py

david-yz-liu · 2026-02-06T02:16:51Z

packages/python-ta/tests/fixtures/reporters/content_injection.py

+        "server_port": 2000
+    })
+
+inject_script()


This file doesn't need to call the function or really be executed at all; see my comments on the in the test file below.

david-yz-liu · 2026-02-06T02:17:41Z

packages/python-ta/src/python_ta/util/extended_markup.py

+    "~",
+    ":",
+]
+unmapped_code_point = "\ue000"


Oh I forgot to mention this earlier, but for all top-level constants, please use the ALL_CAPS naming style for them. This applies to both markdown_chars and unmapped_code_point.

david-yz-liu · 2026-02-08T15:14:28Z

packages/python-ta/tests/test_reporters/test_html_reporter.py

+    output_path = os.path.normpath(
+        os.path.join(__file__, "../../test_reporters/output_files/injection_script_output.html")
+    )
+    check_all(module_name=script_path, output=output_path)


This is good but can be improved further: you actually do not need to write the output to a file. Instead, use a StringIO buffer to store the output in memory (you can ask Herena about this, as she's the one who implemented this functionality for check_all)

david-yz-liu · 2026-02-08T15:15:12Z

packages/python-ta/tests/fixtures/reporters/content_injection.py

+def inject_script():
+    a = "hello"
+    x = f'{a + "<script>alert(2);</script>"}'
+    return x


make sure this file ends with a newline

david-yz-liu · 2026-02-08T15:15:51Z

packages/python-ta/tests/fixtures/reporters/markdown_escape_script.py

+def markdown_chars():
+    y = "hello"
+    z = f'{y + "` ##world `"}'
+


Similar to the above comment, I recommend running this file through black to fix some formatting issues

david-yz-liu · 2026-02-08T15:20:30Z

.pre-commit-config.yaml

      packages/python-ta/tests/test_reporters/snapshots/test_html_server/test_watch_persistence/watch_html_server_snapshot.html|
+      packages/python-ta/tests/test_reporters/snapshots/test_html_reporter/test_injection/script_injection.html|
+      packages/python-ta/tests/test_reporters/snapshots/test_html_reporter/test_markdown_escape/markdown_escape.html|
+      packages/python-ta/tests/test_reporters/output_files/injection_script_output.html|


Relating to my above comment, please remove the "output_files" from this PR and this file

david-yz-liu

Nice work, @Zain-Mahmoud!

Zain-Mahmoud added 5 commits January 20, 2026 20:31

updated CHANGELOG.md

f874626

updated docs

142b381

created extended markup class

4b89157

replaced error messages with new markup class

c274470

updated jinja template to unescape input

ca0f1d6

Zain-Mahmoud marked this pull request as ready for review January 21, 2026 01:43

Zain-Mahmoud added 2 commits January 20, 2026 22:39

update snapshots

6032f65

update character list

bec3c05

Zain-Mahmoud marked this pull request as draft January 21, 2026 03:46

Zain-Mahmoud requested review from david-yz-liu and removed request for david-yz-liu January 21, 2026 03:46

david-yz-liu reviewed Jan 24, 2026

View reviewed changes

Zain-Mahmoud added 8 commits January 29, 2026 00:32

apply markup class to message definitions

5d0d4dc

ensure only strings are to be escaped

bc69891

remove use of extended markup class

bb97a16

revert snapshots

d836e14

remove markdown preset

3f18e88

use replace ampersand with different character

60cd50c

update snapshots

b159788

update documentation

5c3927d

Zain-Mahmoud marked this pull request as ready for review January 29, 2026 19:31

Zain-Mahmoud requested a review from david-yz-liu January 29, 2026 19:41

david-yz-liu reviewed Jan 30, 2026

View reviewed changes

Zain-Mahmoud added 6 commits January 29, 2026 22:13

rename variables

66ebb07

fix bug and add annotations

39a411f

revert changes

ecfba5a

remove note from documentation

05d0986

update list and type contracts

3281740

revert snapshots

a0d7c5e

Zain-Mahmoud added 4 commits February 4, 2026 23:06

update pre commit config list

e82d3a2

Merge branch 'master' into issue-1285

5271a0b

update snapshots after merge

2ba0438

update cleaning function

f4cdd89

Zain-Mahmoud requested a review from david-yz-liu February 5, 2026 04:57

david-yz-liu reviewed Feb 6, 2026

View reviewed changes

Zain-Mahmoud added 11 commits February 7, 2026 12:05

rename constants

ee05160

create fixture files

debf8d0

create new tests

b2af1ab

update pre commit config yaml file

1c50e39

created output files directory

2c335cb

added new snapshot to exclusion list

25f7813

reverted output files

831388a

update new snapshot

9805505

remove file path from snapshots

afd27dc

replace double quotes for backwards compatibility

e043659

update snapshots

d4b689f

Zain-Mahmoud requested a review from david-yz-liu February 7, 2026 19:22

david-yz-liu reviewed Feb 8, 2026

View reviewed changes

Zain-Mahmoud added 6 commits February 8, 2026 23:57

removed output files from config

e66aae0

fix formatting

a344a8e

update snapshots and output reading

a1a43a2

rename file

930d328

Merge branch 'master' into issue-1285

6c2d879

restore snapshot

0c9b03a

Zain-Mahmoud requested a review from david-yz-liu February 9, 2026 16:12

david-yz-liu approved these changes Feb 9, 2026

View reviewed changes

david-yz-liu merged commit f06042d into pyta-uoft:master Feb 9, 2026
30 checks passed

david-yz-liu mentioned this pull request Feb 9, 2026

Use markupsafe.Markup templates for checker messages #1285

Closed

Fix markdown formatting issue #1286

Fix markdown formatting issue #1286

Uh oh!

Conversation

Zain-Mahmoud commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Type of Change

Checklist

Questions and Comments

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 21813417438

Details

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zain-Mahmoud commented Feb 5, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Zain-Mahmoud commented Jan 21, 2026 •

edited

Loading

coveralls commented Jan 29, 2026 •

edited

Loading