Fix and merge to release 0.8.0

[Daniel-KM]

Hi,

I merged all previous requests and fixes, so parsedown-extra works now as it should. In particular, it allows to add specific attributes to links. This can be the release 0.8.0.

Fixes #44, #55, #58, #71, #82, #85, #86, #96, #99, #100, #101, #106, #109, probably some other issues, and some issues fixed in forks that were not pr.

Sincerely,

Daniel Berthereau Infodoc & Knowledge management

[cooperaj]

This PR adds many necessary fixes and tweaks. The one especially hitting me is the footnote count being incorrect on subsequent calls.

I'd really really like to see this merged.

[Daniel-KM]

Note that because it allows any attributes and potentially any xss attacks, the merge will be released after the upstream of parsedown (see https://github.com/erusev/parsedown/pull/495 or https://github.com/erusev/parsedown/issues/161).

[aidantwoods]

@Daniel-KM

Note that because it allows any attributes and potentially any xss attacks, the merge will be released after the upstream of parsedown (see erusev/parsedown#495 or erusev/parsedown#161).

If I'm reading this right, some of these changes will allow arbitrary attribute (names) to be set?

A feature in erusev/parsedown#495 will prune the event handlers (their only purpose is scripting, so getting rid of them is an easy win).

But I would strongly encourage the use of the new $this->safeMode bool that'll be available once that PR is merged, so as to decide whether or not to permit arbitrary attribute names. ($this->safeMode should indicate that the user would like Parsedown to operate as necessary to be safe).

I mention this because, even without event handlers, there are still possible XSS vectors if arbitrary attribute names can be set, and if the browser permits these ridiculous things (e.g. here and some after here – thankfully e.g. Chrome does not permit javascript in a stylesheet url as executable, but I bet you could find a browser that does permit it).

Equally, there might be an attribute that just plain permits scripting that I haven't considered – dropping event attributes is ultimately a blacklist, so is unfortunately doomed to play the catchup game as (I miss things to include || new attributes come to exist). Still, dropping event handlers is still better than not dropping them IMO if an extension already permits arbitrary attribute names (hence why the feature exists), but to be safe the extension should ideally change its behaviour to be less permissive when $this->safeMode is set to true.

To sum that huge message up: Recommended change: utilise $this->safeMode to decide whether to permit arbitrary attribute names to be set.

[Daniel-KM]

@aidantwoods Yes, markdown-extra is a specific markdown to allow to add class, id, and anything else in markdown. For example, I need to use the attributes "rel" and "lang", but this is impossible with markdown. Of course, this is not for public writing, but for controlled edition.

[aidantwoods]

Of course, this is not for public writing, but for controlled edition.

Looks like (from a cursory look at the code) that there is no setter to turn this on/off though? So Parsedown-extra would automatically allow these attributes, with no option to let Parsedown-extra know whether or not you trust the input contents (i.e. if you use this version of parsedown-extra, it can only be used for trusted input).

If you wanted to retain the xss protection added by erusev/parsedown#495, then I'd suggest toggling such attribute behaviour based on the user-set $this->safeMode setting.

Yes, markdown-extra is a specific markdown to allow to add class, id, and anything else in markdown. For example, I need to use the attributes "rel" and "lang", but this is impossible with markdown.

It seems like Parsedown-extra adds more than that? Abbreviations, footnotes, ...? Those would still be useful in their own right, without being able to set arbitrary html attributes?

[Daniel-KM]

I'm going to rebase this commit on your secure parsedown. To allow attributes is the purpose of markdown extra... You can look at https://michelf.ca/projects/php-markdown/extra/ for it. Or a double setter, one for basic attributes (rel, lang, etc.), by default, the other for any attributes (if somebody really want anything).

[bung87]

code start with self::str needs to remove self:: and self::substr needs to remove self::

[NightScript370]

So, uh, does this have a possibility of getting merged?

[aidantwoods]

Yup! Once Parsedown 1.8 is released I hope to be able to take a look at the things in this PR :)

[sbrl]

Any update on this amazing PR getting merged?

By the looks of things Parsedown 2.x is now being worked on - but it doesn't appear to be a single file any more (which, unfortunately, is actually critical to my extremely unusual use-case).