Page 1 of 1

Markdown mishandling quotes in comm (plugin called twice???)

Posted: Fri May 22, 2009 8:48 pm
by Mekk
I am using markdown plugin, applying it to both the article body and the user comments.

And there is a problem. While most things works as expected, markdown quotations in comments (only in comments) do not work. So, instead of having

> blah blah blah
> bleh bleh bleh

turned into blackquote, I get those > escaped and text posted as is with those greater marks.

I put some diagnostic prints inside the plugin body and it looks like comments are filtered *twice*. First run generates proper output, second run gets > as params and emits them. Not sure whether this is a cause, or just another problem.

I disabled all other text modifying plugins, so nothing else is intervening here.

Any ideas?

Re: Markdown mishandling quotes in comm (plugin called twice???)

Posted: Mon May 25, 2009 9:43 am
by garvinhicking
Hi!

Yeah,that's a restriction of how comments are handled by s9y. ">" is HTML markup, and potentially evil. Thus it's corrected by s9y so that it is escaped, but then markdown won't handle this at that instance properly.

I believe you can only get this to work by using the serendipity_event_unstrip_tags event plugin, but then this makes your comments possibly vulnerable to XSS/HTML injection.

So, actually, I would refrain from offering/using the ">" feature of markdown in comments for the sake of security?

Regards,
Garvin

Re: Markdown mishandling quotes in comm (plugin called twice???)

Posted: Mon May 25, 2009 11:25 am
by Mekk
garvinhicking wrote: Yeah,that's a restriction of how comments are handled by s9y. ">" is HTML markup, and potentially evil. Thus it's corrected by s9y so that it is escaped, but then markdown won't handle this at that instance properly.
Hmm, so it is by design? Could you elaborate a little bit on where exactly (in s9 code) this escaping happens?

I'd just patch this very place to omit (while escaping) this single case: of ">" just on the beginning on the line, and followed with space. While such a patch is not suitable for s9 in general, it would nicely work for me...

Re: Markdown mishandling quotes in comm (plugin called twice???)

Posted: Mon May 25, 2009 11:34 am
by garvinhicking
Hi!
Hmm, so it is by design? Could you elaborate a little bit on where exactly (in s9 code) this escaping happens?
Yes, inside include/functions_comments.inc.php, function serendipity_printComments:

Code: Select all

$comment['comment'] = htmlspecialchars(strip_tags($comment['body']));
HTH,
Garvin

Re: Markdown mishandling quotes in comm (plugin called twice???)

Posted: Wed May 27, 2009 11:48 pm
by Mekk
After some more consideration I decided it is simpler (and more manageable) to implement workaround in the plugin than in the core - replace leading > with > before filtering with markdown. I don't think this unescape alone brings significant risks when the remaining HTML characters are still filtered.

So here is the simple patch to serendipity_event_markdown.php which handles single-level quotations:

Code: Select all

      $eventData[$element] = Markdown(
               preg_replace(
                   '/^> /m',
                   '> ',
                   $eventData[$element]
               )
       );
and here is the more elaborate version which allows for nested quotations:

Code: Select all

       $eventData[$element] = Markdown(
                 preg_replace_callback(
                             '/^(?:> ?)+/m',
                             create_function(
                                        '$matches',
                                        'return str_replace(">", ">", $matches[0]);'
                                        #'return count($matches);',
                                    ),
                              $eventData[$element]
                )
       );
Either one of them is to be put instead of current

Code: Select all

      $eventData[$element] = Markdown($eventData[$element]);