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.

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

Fix edge-case crash in InlineProcessor #1406

Merged
merged 2 commits into from Nov 3, 2023

Conversation

oprypin
Copy link
Contributor

@oprypin oprypin commented Nov 2, 2023

If an inlineprocessor returns an AtomicString (even though that is pointless, a plain string is atomic in that context), there can be an exception in 2 separate places. The added test case was crashing before this change.

  File "tests/test_apis.py", line 722, in testInlineProcessorDoesntCrashWithAtomicString
    new = self.inlineprocessor.run(tree)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "markdown/treeprocessors.py", line 384, in run
    lst = self.__processPlaceholders(
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "markdown/treeprocessors.py", line 223, in __processPlaceholders
    if child.tail:
       ^^^^^^^^^^
AttributeError: 'AtomicString' object has no attribute 'tail'
  File "tests/test_apis.py", line 722, in testInlineProcessorDoesntCrashWithAtomicString
    new = self.inlineprocessor.run(tree)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "markdown/treeprocessors.py", line 387, in run
    self.__handleInline(text), child
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "markdown/treeprocessors.py", line 136, in __handleInline
    data, matched, startIndex = self.__applyPattern(
                                ^^^^^^^^^^^^^^^^^^^^
  File "markdown/treeprocessors.py", line 310, in __applyPattern
    if not isinstance(node.text, util.AtomicString):
                      ^^^^^^^^^
AttributeError: 'AtomicString' object has no attribute 'text'

If an inlineprocessor returns an AtomicString (even though that is pointless, a plain string is atomic in that context), there can be an exception in 2 separate places. The added test case was crashing before this change.
Copy link
Member

@waylan waylan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it looks like I was wrong about this. Turns out any string should not follow that code path.

Thanks for the which make sit clear.

@waylan waylan merged commit a63e6f3 into Python-Markdown:master Nov 3, 2023
15 of 16 checks passed
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.

None yet

2 participants