Extension talk:Accordion/Archive 1

Does not work. The text is not displayed anymore.

Product	Version
MediaWiki	1.16.2
PHP	5.3.5
MySQL	5.5.8

Can you give some clues about the possible interferences of other extensions ?

In debug this warning is given: Parameter 3 to wfAccordionCallback() expected to be a reference

veders.

my Solution I removed the ampersand from this Callback function in Accordion.php and it worked:

function wfAccordionCallback( $input, $args, &$parser )

function wfAccordionCallback( $input, $args, $parser )

--155.56.68.216 15:06, 29 July 2011 (UTC)Escobar

Accordion.js Hook event

seems to me that the Event should be registered like this:

hookEvent( 'load', onloadListener );

instead of

addHandler( 'load', onloadListener );

otherwise I always get some js errors and accordion extension not working --155.56.68.216 15:06, 29 July 2011 (UTC)Escobar

XSS Vulnerability

just check the $args['level'] var, and everything should be fine

Accordion.php

function wfAccordionCallback( $input, $args, $parser ) {
 $level = preg_match('/[1-6]/',$args['level'])? $args['level']: 2;
 ...
}

--155.56.68.215 09:30, 30 August 2011 (UTC)Escobar

I can tell you one thing at least. That code you posted doesn't fix the vulnerability, it only tests that a digit is present, not that the value is ONLY a digit. You can easily bypass that by including a number in the wikitext you put inside the level. Even in that case the var should be properly escaped... It also drops the isset and hence will start spewing php notices/errors when level is left out. Dantman 09:57, 30 August 2011 (UTC)

Yes you're right Dantman, I've been to fast with my solution:

function wfAccordionCallback( $input, $args, $parser ) {
  $level = is_numeric($args['level']) && strlen($args['level'])<2? $args['level']: 2;
  $result = '<script type="text/javascript">var acc_hlevel = ' . $level . ';</script>';
  $result .= '<div id="acc_container">';
  $result .= $parser->recursiveTagParse( $input );
  $result .= '</div>';
  return $result;
}

I'm sure there's a better solution, since I'm not realy familiar with the MW structre, maybe there is a posibillity to check/sanitize the values with the builtin MW functionality. --155.56.68.217 16:48, 1 September 2011 (UTC) Escobar

Things needed of a fix include:

• isset($args['level'])
• (int)$args['level'] or intval($args['level'])
• Xml::encodeJSVar

Dantman 17:24, 1 September 2011 (UTC)

finnaly...

function wfAccordionCallback( $input, $args, $parser ) {        		        
        $level = isset($args['level']) ? (int) $args['level']:2;				
	$result = '<script type="text/javascript">var acc_hlevel = ' . Xml::encodeJSVar($level) . ';</script>';
        $result .= '<div id="acc_container">';
        $result .= $parser->recursiveTagParse( $input );
        $result .= '</div>';
        return $result;
}

--155.56.68.217 14:59, 2 September 2011 (UTC) Escobar

It only works without quotes around the default value -> $level = isset($args['level']) ? (int) $args['level']:2;

Yup, that's about right. Though honestly besides that actual security issue there's so much wrong with the implementation of this extension. I wish I had the time to completely re-implement a new Accordion extension. Dantman 21:57, 2 September 2011 (UTC)

Might not do what you would expect

Just a warning. This extension lets you create blocks of text that will open and close, just as you might expect. However, these blocks must be opened and closed sequentially; meaning you can only open the next or previous block and doing so closes the current block. It's all a little funny and frankly, it doesn't seem very useful.