wpsharks / s2clean

s2Clean is a premium product. This public repo is for issue tracking only.
Other
1 stars 0 forks source link

Markdown parsing affecting Markdown list items #26

Closed raamdev closed 9 years ago

raamdev commented 9 years ago

For some reason, the list items in this KB Article are not getting processed as expected on ZenCache.com: https://zencache.com/wp-admin/post.php?post=1094&action=edit

Compare the preview on ZenCache.com with the markdown parsed on GitHub:

2015-03-19_13-56-31 2015-03-19_13-57-07

Notice how on ZenCache.com the list items all start with 1. and on GitHub they properly enumerate 1., 2., etc.

@jaswsinc writes...

I see! Yep, that looks like a bug to me. I'll take a look sometime today. I ran the Markdown through the Dingus here and it runs fine, so it's a bug in s2Clean. http://parsedown.org/extra/

Bug is most likely here: https://github.com/websharks/s2clean-pro/blob/000000-dev/s2clean/includes/functions.php#L2860

Yep, definitely the bug. That method was there for a security issue that I found, but it needs to come out now. I'd like to take a closer look at this before I just yank it though.

jaswrks commented 9 years ago

Partial fix. This is awaiting some updates in ezPHP before I will close it though. The reason for strip_md_indents() in s2Clean, was actually to make Markdown parsing compatible with ezPHP.

Consider the following HTML markup when Markdown processing is enabled:

<?php if(is_user_logged_in()): ?>
    <div>
        <p>Hello user!</p>
    </div>
<?php endif; ?>

Once ezPHP does its thing, Markdown runs. Given the indents for clarity (i.e., when using something like ezPHP), what you would end up with is

<pre><code>&lt;div&gt;
        &lt;p&gt;Hello user!&lt;/p&gt;
    &lt;/div&gt;</code></pre>

This is because Markdown thinks it is an indented code block, when in fact it was only indented because PHP tags were being used to wrap the block of code.

This only impacts a scenario where Markdown encounters a top-level code block. Anything nested would be unaffected, since Markdown does not parse block-level elements.

jaswrks commented 9 years ago

This has now been resolved. I'll let this settle-in over the weekend while I run more tests and look for any remaining issues. I'll update zencache.com once I feel confident there are no problems with the latest changes.

raamdev commented 9 years ago

This has now been resolved.

Awesome! Thanks for fixing this!

jaswrks commented 9 years ago

This has been resolved at ZenCache.com now. As of this morning.

raamdev commented 9 years ago

@jaswsinc Thanks!