r70849 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r70848‎ | r70849 | r70850 >
Date:07:19, 11 August 2010
Author:sean_colombo
Status:resolved (Comments)
Tags:
Comment:
Fix to #17031. See bug ticket for test code & test-results.
Modified paths:
  • /trunk/phase3/includes/Sanitizer.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Sanitizer.php
——@@ -40,10 +40,11 @@
4141 * Allows some... latitude.
4242 * Used in Sanitizer::fixTagAttributes and Sanitizer::decodeTagAttributes
4343 */
44 -$attrib = '[A-Za-z0-9]';
 44+$attrib_first = '[:A-Z_a-z]';
 45+$attrib = '[:A-Z_a-z-.0-9]';
4546 $space = '[\x09\x0a\x0d\x20]';
4647 define( 'MW_ATTRIBS_REGEX',
47 - "/(?:^|$space)((?:xml:|xmlns:)?$attrib+)
 48+ "/(?:^|$space)({$attrib_first}{$attrib}*)
4849 ($space*=$space*
4950 (?:
5051 # The attribute value: quoted or alone

Follow-up revisions

RevisionCommit summaryAuthorDate
r70911As per feedback on r70849, fixed bad indenting (spaces) that should be a tab.sean_colombo19:02, 11 August 2010
r74186Tweak variable name to be camel case (as per feedback on r70849).sean_colombo04:31, 3 October 2010
r82480(Bug 27539) Allow attributes beginning with a digit in wiktext tag parameters....platonides20:16, 19 February 2011

Comments

#Comment by MaxSem (talk | contribs)   07:29, 11 August 2010

To make automatic links to Bugzilla, use syntax like this: bug 17031.

#Comment by Platonides (talk | contribs)   14:45, 11 August 2010

Use tabs for indenting, not spaces.

See if you can make https://bugzilla.wikimedia.org/attachment.cgi?id=7619 a place inside maintenance/tests in phpunit form.

#Comment by SColombo~mediawikiwiki (talk | contribs)   19:03, 11 August 2010

Changed spaces to tab in r70911.

Thanks for the feedback (including the bug-link format & phpunit)!

#Comment by Simetrical (talk | contribs)   19:10, 12 August 2010

Looks okay, but one more style nitpick: we use camelCase for variable names, not underscores, so it should be $attribFirst. Other than that, it looks fine -- theoretically we could allow more values, but it's probably unnecessary for practical purposes.

#Comment by SColombo~mediawikiwiki (talk | contribs)   04:38, 3 October 2010

Fixed variable names in r74186

As for allowing other values, just so it's easier to find for people looking at this thread, here is the XML 1.0 spec for the 'Name' type which is mentioned in bug 17031: http://www.w3.org/TR/REC-xml/#NT-Name

#Comment by ThomasV (talk | contribs)   16:17, 18 February 2011

This breaks extension hooks that use parameters beginning with a number. Problem observed after WMF 1.17 deployment. See https://bugzilla.wikimedia.org/show_bug.cgi?id=27539

Status & tagging log