On Wed, Jan 12, 2011 at 6:13 PM, Jeroen De Dauw <[email protected]>wrote:
> > That seems highly contrary to expectations when dealing with tag hooks.
> What if there's a tag hook "display" registered by another plugin, does it
> get overridden? Overridden only if a "map" parameter is supplied?
>
>
> That's a good point, which I did not consider when naming the hook like
> this. Sorting the tag extensions desc by amount of "words" in their name
> and
> then handling them in that order should prevent anything from getting
> overridden no? In any case, I'll have a closer look at this for the next
> version of the extension.
>
It looks like Parser::setHook() and Parser::setTransparentTagHook() don't do
any validation on tag hook names. At least the search for transparent tag
hooks is very naive, assuming that only valid names were passed and knowing
that valid names contain only characters which carry no special meaning in
regex syntax:
$elements = array_keys( $this->mTransparentTagHooks );
$text = $this->extractTagsAndParams( $elements, $text, $matches,
$uniq_prefix );
... in which happens...
$taglist = implode( '|', $elements );
$start = "/<($taglist)(\\s+[^>]*?|\\s*?)(\/?" . ">)|<(!--)/i";
This would explain why you don't get an error thrown when setting the tag
hook with a space in the name (parser assumes extensions are acting
correctly, and doesn't validate), and why the incorrect entry gets "parsed"
as it does (the code that extracts them assumes valid input, so uses a
string match that's convenient to write when those assumptions hold).
I haven't dared look at the preprocessor code in case horrible things happen
there too. :)
Platonides wrote:
> It is order dependant.
> If you install the extension providing 'display', then the one providing
> 'display map', the latter will never be called. If 'display map' gets
> registered first, both will work (your users might get annoyed, though).
>
> Guess what I tried to do in r80025? :)
I'd recommend whitelisting legit characters rather than blacklisting a
handful of illegit characters; tag hook name validation should probably
sensibly match the rules of XML element naming, as that's what the syntax
aims to look and act like.
It'd also be best to write the validation code only once and call it from
multiple locations, rather than duplicating the code. This will make it much
easier to maintain, as there will be less danger of the multiple copies
getting out of sync.
-- brion
_______________________________________________
Wikitech-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
