Escaped HTML in footnotes

[AlfredoRamos]

Hi,

Since the latest change in parsedown (erusev/parsedown#495) footnotes are broken (reference AlfredoRamos/parsedown-extra-laravel/jobs/348703852), they display escaped HTML.

Parsedown: 1.6.4
Parsedown Extra: 0.7.1

<p>Parsedown Extra <sup id="fnref1:1"><a href="#fn:1" class="footnote-ref">1</a></sup></p>
<div class="footnotes">
<hr />
<ol>
<li id="fn:1">
<p><a href="http://parsedown.org/extra/">http://parsedown.org/extra/</a>&#160;<a href="#fnref1:1" rev="footnote" class="footnote-backref">&#8617;</a></p>
</li>
</ol>
</div>

Parsedown: 1.7.0
Parsedown Extra: 0.7.1

<p>Parsedown Extra <sup id="fnref1:1"><a href="#fn:1" class="footnote-ref">1</a></sup></p>
<div class="footnotes">
<hr />
<ol>
<li id="fn:1">
&lt;p&gt;&lt;a href="http://parsedown.org/extra/"&gt;http://parsedown.org/extra/&lt;/a&gt;&amp;#160;&lt;a href="#fnref1:1" rev="footnote" class="footnote-backref"&gt;&amp;#8617;&lt;/a&gt;&lt;/p&gt;
</li>
</ol>
</div>

As you can see, the HTML inside the<li> tag is escaped.

My test script:

<?php

require __DIR__ . '/vendor/autoload.php';

$parsedown = new ParsedownExtra;
$parsedown->setSafeMode(false);
$parsedown->setMarkupEscaped(false);

echo $parsedown->text('Parsedown Extra [^1]'.PHP_EOL.'[^1]: http://parsedown.org/extra/');

I've tried to disable safe-mode but it doesn't seem to do anything.

My server runs PHP 7.2.3

[aidantwoods]

Thanks for reporting this – there's a little bit of discussion over at #117 (comment) if you're interested. Hopefully should have a fix out soon.

[jeremycherfas]

Hello. Any idea when we might expect a fix? Thanks.

[jeremycherfas]

Bump

[aidantwoods]

There are some first steps toward what will hopefully become a proper fix in erusev/parsedown#576. Once that is done Parsedown-extra will have access an AST of elements instead of just the final HTML string, so it'll be able to rewrite the parsed structure without directly having to act on an HTML string, and just shoving HTML into what is meant to be text bound within a single element (this is why footnotes are currently being escaped).

It would be possible get a patch out a bit sooner once the smaller erusev/parsedown#574 is finalised, since it'll allow basically keeping the current algorithm with a very minor adjustment.

However from a cursory look (as mentioned in #117 (comment)), I'm pretty sure Parsedown-extra has its own syntax introduced XSS vectors (even just looking within footnotes), so this type of fix wouldn't be suitable if you're dealing with any kind of untrusted user-input (it wouldn't be any worse than the current version is in that regard though – so just a heads up if that applies to anyone).
Does anyone's use case involve user-input?

Anyway, in order to benefit from the new safety features within Parsedown, Parsedown-Extra would need to use the method described above so that escaping can be done properly (and probably some other changes too).

[wottpal]

erusev/parsedown#576 and erusev/parsedown#574 seem to be merged. Let us know if we can provide any help for this @aidantwoods!

[wottpal]

Any updates on this?

[BenjaminHoegh]

The issue is still there @wottpal You can use the pre-release 0.8.0 here is it fixed, but be aware that there are some bugs

[wottpal]

Nice to hear somebody took care of it. I’ll wait for the stable 🙃

[slowpokefarm]

Maybe it can be reverted at least? this issue broke our blogs on October CMS :/

[mnapoli]

Is it possible to tag a stable release so that we can all use the bugfix?

Thanks :)

[adamgreenough]

Is there any update on this?

[BenjaminHoegh]

If you use a Definition Lists some where in your markdown it can broke the footnote, not sure why yet

[terrylinooo]

I develop a WordPress Markwon editor plugin and my user reported this issue to me
terrylinooo/githuber-md#22

After spending time to view the code, I think the only way to solve it is to modify Parsedown.php (line: 1507)

//$markup .= self::escape($Element['text'], true);
$markup .= $Element['text'];

[wottpal]

@erusev Just giving this a little push as a lot of people need a newly tagged stable release to fix this.

[edalzell]

@aidantwoods @erusev is there an ETA on this?

[jackmcdade]

Is there anything I can do to help get this resolved? It's been open for 18 months, would love a fix!

[erusev]

@aidantwoods could you share your latest thoughts on this issue? I'd love to know about causes and possible solutions. Thanks!

[aidantwoods]

This should be resolved in 0.8.0-beta-1.

0.8.0-beta-1 along with Parsedown 1.8.0-beta-x are "beta" for the purpose of not breaking extensions. There were a few internal changes on the protected API that in theory could break extensions (overview here).
I made as many as backwards compatible as I could, but in some cases extensions were depending on a particular element structure returned by block* and inline* methods. In theory this structure might change for any reason (e.g. a bugfix) but since there wasn't really a proper API boundary defined that extensions could rely on, it's sort of hard to say that extensions should or shouldn't have relied upon a particular observed behaviour.

Instead of just releasing these beta versions as major version bumps I wanted to create a proper API boundary for extensions to use so that this doesn't occur in future. There's quite a bit of work gone into what that new version might look like over in erusev/parsedown#708, though I'm hesitant to merge this in without feedback.

[nathanlesage]

Just for clarification: Does this mean that we already see a light at the end of the tunnel? Would like to report this back to the corresponding OctoberCMS-issue so that back there we could also get a heads-up for when it'll work without monkey patching :)

[aidantwoods]

Short-circuited a fix to this in #144 by porting a subset of a parsedown 1.8 beta feature (rawHtml) into stable. It is important that Parsedown 1.7.x does not itself utilise this feature so as to avoid potential breakage caused by adding features like this.

Fix is released as 0.8.1.

Some context on potential breakage for 1.8-beta:

getgrav/grav#1972 (comment)
erusev/parsedown#580 (comment)
erusev/parsedown#601 (comment)

As mentioned before, I think the right thing to do going forward is to put efforts into 2.0 (which aims to define a clear API for "extensions" to use) rather than trying to tiptoe around an undefined boundary for breakage due to extensions being able (and arguably encouraged by docs) to tightly couple themselves to internal observed behaviours.