Fix and merge to release 0.8.0 #110
Conversation
Setting footnote prefix: ParsedownExtra::setFootnotePrefix( string $footnotePrefix ) This should be a string that is valid in an HTML attribute, i.e. escaped properly. Default value is blank, if this is kept blank behavior is as before. Use case in mind is for something like a blog, where you can set the prefix to something unique like a post ID. Added test_footnote_prefix() to test cases.
ignores the list of voided elements when rendering markdown
Stabilizes library by revamping class to utilize methods from php 5.4 or greater.
Does an extra check on valid DOMElements to make sure they have child nodes before calling recursively.
Previously, the 'extra' in 'Markdown Extra' was only supported in links, lists, and headings. This commits adds support for adding a class or ID to fenced code blocks.
That'll tech me to debug my own code.
* Move the custom class to the <pre> rather than the <code> * Add dashes in "truncated-for-brevity" * Code language is specified through 'class="language-python"' rather than 'lang="python"' * Remove newline at end of file
…st a `class` and `id`.
- I wanted the ability to be able to add `width` & `height` attributes to images but wasn't able to in the current state so I updated the RegEx rules and the `parseAttributeData` method. Now a user can add something like `{.test .fu #bar style="color:red; font-style:italic" data-is-cool="'tis true" lang=en_us}` and it'll render out these attributes `class="test fu" id="bar" style="color:red; font-style:italic" data-is-cool="'tis true" lang="en_us"`.
- I also added the `inlineImage` method so attributes like `width` & `height` can be added.
This can be used as a template for Parsedown extensions (like Parsedown Extra) to reuse Parsedown's original tests. You can either extend tests (like ParsedownExtraTest) or reuse them unchanged (like CommonMarkTest). Parsedown extensions simply have to implement their own TestParsedown class (test/TestParsedown.php) and all original tests will run with a instance of this class, rather than a instance of the original Parsedown class. This PR is a follow-up to erusev/parsedown#423, i.e. you must merge erusev/parsedown#423 first, otherwise this doesn't work.
Failing tests don't break builds on purpose, Parsedown Extra doesn't fully comply with the CommonMark specs at the moment. We should switch to test/CommonMarkTest.php of erusev/parsedown later, see erusev/parsedown#423 for details.
Daniel-KM
force-pushed
the
fix/merge
branch
5 times, most recently
from
a3a8f2b
to
4f427f0
Compare
|
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. |
|
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 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 To sum that huge message up: |
|
@aidantwoods |
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
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? |
|
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). |
Daniel-KM
force-pushed
the
fix/merge
branch
3 times, most recently
from
67c3b46
to
b0c512e
Compare
|
code start with |
|
So, uh, does this have a possibility of getting merged? |
|
Yup! Once Parsedown 1.8 is released I hope to be able to take a look at the things in this PR :) |
|
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). |
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