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

Creating and modifying plugins.
Post Reply
Mekk
Regular
Posts: 54
Joined: Tue Jul 22, 2008 7:53 pm
Contact:

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

Post by Mekk » Fri May 22, 2009 8:48 pm

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?

User avatar
garvinhicking
Core Developer
Posts: 30020
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

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

Post by garvinhicking » Mon May 25, 2009 9:43 am

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
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/

Mekk
Regular
Posts: 54
Joined: Tue Jul 22, 2008 7:53 pm
Contact:

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

Post by Mekk » Mon May 25, 2009 11:25 am

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...

User avatar
garvinhicking
Core Developer
Posts: 30020
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

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

Post by garvinhicking » Mon May 25, 2009 11:34 am

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
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/

Mekk
Regular
Posts: 54
Joined: Tue Jul 22, 2008 7:53 pm
Contact:

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

Post by Mekk » Wed May 27, 2009 11:48 pm

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]);

Post Reply