Closed noamgat closed 5 months ago
Thanks for the report. This appears to be an issue with the abbr
extension. In fact, the behavior can be replicated using that extension only, Specifically, the extension correctly identifies the line of input as an abbreviation definition and tries to build a regex to match instances of ^1^
in the document. The regex it builds then fails to compile and raises an error.
Obviously, the carrot (^
) has special meaning in regex, so it would need to be escaped to match the actual character. In fact, the code accounts for the fact that a user could include anything in an abbreviation and wraps each character in a character set ([]
). In other words, the regex for the abbreviation HTML
would be [H][T][M][L]
. As it turns out, there are only 4 characters which have special meaning in character sets (^
, \
, -
, and ]
) and it appears we do not do anything to account for them.
We have a few options to address this:
/H/T/M/L
) or only backslash escaping special characters listed here. Or maybe some other completely different approach.Option 3 would be simple in that it would mostly eliminate the possibility of special characters being used in generated regex . But it could break users' existing documents as the current permissive approach has been in-place for over a decade. And I say it would only "mostly eliminate" the issue because we would likely want to allow -
at a minimum; so we would still need to do some escaping. Option 2 would be more dramatic that option 1 and is listed for completeness. We might want to go that way of it was more performant or provided some other significant benefit. However, if we are just fixing the immediate bug, then option 1 seems like the best choice.
>>> import markdown
>>> txt = "*[^1^]: This is going to crash if extra extension is enabled"
>>> markdown.markdown(txt, extensions=['abbr'])
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "C:\code\md\markdown\core.py", line 482, in markdown
return md.convert(text)
File "C:\code\md\markdown\core.py", line 357, in convert
root = self.parser.parseDocument(self.lines).getroot()
File "C:\code\md\markdown\blockparser.py", line 117, in parseDocument
self.parseChunk(self.root, '\n'.join(lines))
File "C:\code\md\markdown\blockparser.py", line 136, in parseChunk
self.parseBlocks(parent, text.split('\n\n'))
File "C:\code\md\markdown\blockparser.py", line 158, in parseBlocks
if processor.run(parent, blocks) is not False:
File "C:\code\md\markdown\extensions\abbr.py", line 61, in run
AbbrInlineProcessor(self._generate_pattern(abbr), title), 'abbr-%s' % abbr, 2
File "C:\code\md\markdown\extensions\abbr.py", line 94, in __init__
super().__init__(pattern)
File "C:\code\md\markdown\inlinepatterns.py", line 297, in __init__
self.compiled_re = re.compile(pattern, re.DOTALL | re.UNICODE)
File "C:\Users\wlimberg\AppData\Local\Programs\Python\Python38\lib\re.py", line 250, in compile
return _compile(pattern, flags)
File "C:\Users\wlimberg\AppData\Local\Programs\Python\Python38\lib\re.py", line 302, in _compile
p = sre_compile.compile(pattern, flags)
File "C:\Users\wlimberg\AppData\Local\Programs\Python\Python38\lib\sre_compile.py", line 764, in compile
p = sre_parse.parse(p, flags)
File "C:\Users\wlimberg\AppData\Local\Programs\Python\Python38\lib\sre_parse.py", line 948, in parse
p = _parse_sub(source, state, flags & SRE_FLAG_VERBOSE, 0)
File "C:\Users\wlimberg\AppData\Local\Programs\Python\Python38\lib\sre_parse.py", line 443, in _parse_sub
itemsappend(_parse(source, state, verbose, nested + 1,
File "C:\Users\wlimberg\AppData\Local\Programs\Python\Python38\lib\sre_parse.py", line 834, in _parse
p = _parse_sub(source, state, sub_verbose, nested + 1)
File "C:\Users\wlimberg\AppData\Local\Programs\Python\Python38\lib\sre_parse.py", line 443, in _parse_sub
itemsappend(_parse(source, state, verbose, nested + 1,
File "C:\Users\wlimberg\AppData\Local\Programs\Python\Python38\lib\sre_parse.py", line 549, in _parse
raise source.error("unterminated character set",
re.error: unterminated character set at position 17
On second thought, we could go with option 3 and restrict abbreviations to not allow them to include the 3 characters ^
, \
, and ]
. As it is, those 3 characters would have never worked, so we aren't breaking anyone's existing documents. And then we don't need to worry about escaping them. While the backslash is a special character in Markdown, it does not make sense as part of an abbreviation, so being not permitted there should be fine. In fact, we have never supported escaping characters in abbreviations, which also means that a ]
would never end up in one (actually, it already is restricted)
Finally, I will note that while the -
does have special meaning in character sets, it is treated as a regular character if it is the first or last character of a character set. As we only every include a single character in a character set, it has worked fine without escaping and should continue to work without modification. Therefore, it does not need to be addressed at all.
I am rather opposed to the chosen solution, on two fronts:
It is strange to let the regex implementation details dictate the behavior.
It is really easy to implement the correct solution: replace the _generate_pattern
function entirely with just re.escape
- It is strange to let the regex implementation details dictate the behavior.
I will note that there is no change in behavior, which is why I took this route. I simply pushed a bug fix which maintains the existing behavior. Specifically, the change prevents an error from being raised. So the change stands.
- It is really easy to implement the correct solution: replace the
_generate_pattern
function entirely with justre.escape
Apparently, way back when I first wrote this extension, I didn't realize that re.escape
existed. Because, if I did, I would have used it. In any event, a change in behavior would be considered in a separate PR.
I just pushed #1449. It still doesn't allow backslashes, but because of their meaning in Markdown, not due to the regex implementation.
Nice, thanks!
Hi,
When using the "extra" extensions, some invalid markdowns (i think?) are causing exceptions rather than returning plaintext line.
Example:
Is this expected behavior?
Full exception stack trace: