On Wed, Jan 12, 2011 at 6:13 PM, Jeroen De Dauw <jeroendedauw(a)gmail.com>wrote;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