Improve ugly interface for Sanitizer::escapeId()

Calling it with no extra arguments will now assume that you're escaping
a whole id, not an id fragment, which is safer.  Also, instead of ugly
bitfield-based options, I've changed the options to use an array of
strings.  I fixed all callers in trunk.  Out-of-tree callers that were
using Sanitizer::NONE will get correct behavior, while those that were
calling it with no arguments will get slightly changed behavior (an x
will be prepended).  I think this is harmless enough that we can skip
back-compat cruft here.

This should cause no visible changes.  No parser test regressions.

diff --git a/includes/ImagePage.php b/includes/ImagePage.php
index e2a0b36..aa5295e 100644 (file)
--- a/includes/ImagePage.php
+++ b/includes/ImagePage.php
@@ -249,6 +249,7 @@ class ImagePage extends Article {
                $r .= "{| id=mw_metadata class=mw_metadata\n";
                foreach ( $metadata as $type => $stuff ) {
                        foreach ( $stuff as $v ) {
+                               # FIXME, why is this using escapeId for a class?!
                                $class = Sanitizer::escapeId( $v['id'] );
                                if( $type == 'collapsed' ) {
                                        $class .= ' collapsable';

diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php
index 28ae4cb..f5f09a8 100644 (file)
--- a/includes/Sanitizer.php
+++ b/includes/Sanitizer.php
@@ -331,9 +331,6 @@ $wgHtmlEntityAliases = array(
  * @ingroup Parser
  */
 class Sanitizer {
-       const NONE = 0;
-       const INITIAL_NONLETTER = 1;
-
        /**
         * Cleans up HTML, removes dangerous tags and attributes, and
         * removes HTML comments
@@ -617,7 +614,7 @@ class Sanitizer {
                        }
 
                        if ( $attribute === 'id' )
-                               $value = Sanitizer::escapeId( $value, Sanitizer::NONE );
+                               $value = Sanitizer::escapeId( $value );
 
                        // If this attribute was previously set, override it.
                        // Output should only have one attribute of each name.
@@ -777,15 +774,15 @@ class Sanitizer {
         *                                                          name attributes
         * @see http://www.w3.org/TR/html401/struct/links.html#h-12.2.3 Anchors with the id attribute
         *
-        * @param string $id    Id to validate
-        * @param int    $flags Currently only two values: Sanitizer::INITIAL_NONLETTER
-        *                      (default) permits initial non-letter characters,
-        *                      such as if you're adding a prefix to them.
-        *                      Sanitizer::NONE will prepend an 'x' if the id
-        *                      would otherwise start with a nonletter.
+        * @param string $id      Id to validate
+        * @param mixed  $options String or array of strings (default is array()):
+        *   'noninitial': This is a non-initial fragment of an id, not a full id,
+        *       so don't prepend an 'x' if the first character isn't valid at the
+        *       beginning of an id.
         * @return string
         */
-       static function escapeId( $id, $flags = Sanitizer::INITIAL_NONLETTER ) {
+       static function escapeId( $id, $options = array() ) {
+               $options = (array)$options;
                static $replace = array(
                        '%3A' => ':',
                        '%' => '.'
@@ -794,8 +791,8 @@ class Sanitizer {
                $id = urlencode( Sanitizer::decodeCharReferences( strtr( $id, ' ', '_' ) ) );
                $id = str_replace( array_keys( $replace ), array_values( $replace ), $id );
 
-               if( ~$flags & Sanitizer::INITIAL_NONLETTER
-               && !preg_match( '/[a-zA-Z]/', $id[0] ) ) {
+               if( preg_match( '/[^a-zA-Z]/', $id[0] )
+               && !in_array( 'noninitial', $options ) )  {
                        // Initial character must be a letter!
                        $id = "x$id";
                }

diff --git a/includes/Title.php b/includes/Title.php
index 033d045..29f8efe 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -451,7 +451,7 @@ class Title {
         * Escape a text fragment, say from a link, for a URL
         */
        static function escapeFragmentForURL( $fragment ) {
-               return Sanitizer::escapeId( $fragment, Sanitizer::NONE );
+               return Sanitizer::escapeId( $fragment );
        }
 
 #----------------------------------------------------------------------------

diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index 853d05f..e4d63af 100644 (file)
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -3615,7 +3615,7 @@ class Parser
 
                        # Save headline for section edit hint before it's escaped
                        $headlineHint = $safeHeadline;
-                       $safeHeadline = Sanitizer::escapeId( $safeHeadline, Sanitizer::NONE );
+                       $safeHeadline = Sanitizer::escapeId( $safeHeadline );
                        # HTML names must be case-insensitively unique (bug 10721)
                        $arrayKey = strtolower( $safeHeadline );
 

diff --git a/skins/Modern.php b/skins/Modern.php
index 1b5e078..cb24baf 100644 (file)
--- a/skins/Modern.php
+++ b/skins/Modern.php
@@ -113,7 +113,7 @@ class ModernTemplate extends QuickTemplate {
                        <ul>
        <?php           foreach($this->data['content_actions'] as $key => $tab) {
                                        echo '
-                                <li id="ca-' . Sanitizer::escapeId($key).'"';
+                                <li id="' . Sanitizer::escapeId( "ca-$key" ) . '"';
                                        if( $tab['class'] ) {
                                                echo ' class="'.htmlspecialchars($tab['class']).'"';
                                        }
@@ -201,7 +201,7 @@ class ModernTemplate extends QuickTemplate {
                <div class="pBody">
                        <ul>
 <?php                  foreach($this->data['personal_urls'] as $key => $item) { ?>
-                               <li id="pt-<?php echo Sanitizer::escapeId($key) ?>"<?php
+                               <li id="<?php echo Sanitizer::escapeId( "pt-$key" ) ?>"<?php
                                        if ($item['active']) { ?> class="active"<?php } ?>><a href="<?php
                                echo htmlspecialchars($item['href']) ?>"<?php echo $skin->tooltipAndAccesskey('pt-'.$key) ?><?php
                                if(!empty($item['class'])) { ?> class="<?php
@@ -289,7 +289,7 @@ class ModernTemplate extends QuickTemplate {
 <?php  }
                if($this->data['feeds']) { ?>
                        <li id="feedlinks"><?php foreach($this->data['feeds'] as $key => $feed) {
-                                       ?><span id="feed-<?php echo Sanitizer::escapeId($key) ?>"><a href="<?php
+                                       ?><span id="<?php echo Sanitizer::escapeId( "feed-$key" ) ?>"><a href="<?php
                                        echo htmlspecialchars($feed['href']) ?>"<?php echo $this->skin->tooltipAndAccesskey('feed-'.$key) ?>><?php echo htmlspecialchars($feed['text'])?></a>&nbsp;</span>
                                        <?php } ?></li><?php
                }
@@ -345,7 +345,7 @@ class ModernTemplate extends QuickTemplate {
        /*************************************************************************************************/
        function customBox( $bar, $cont ) {
 ?>
-               <div class='generated-sidebar portlet' id='p-<?php echo Sanitizer::escapeId($bar) ?>'<?php echo $this->skin->tooltip('p-'.$bar) ?>>
+               <div class='generated-sidebar portlet' id='<?php echo Sanitizer::escapeId( "p-$bar" ) ?>'<?php echo $this->skin->tooltip('p-'.$bar) ?>>
                <h5><?php $out = wfMsg( $bar ); if (wfEmptyMsg($bar, $out)) echo $bar; else echo $out; ?></h5>
                <div class='pBody'>
 <?php   if ( is_array( $cont ) ) { ?>

diff --git a/skins/MonoBook.php b/skins/MonoBook.php
index 38ec39c..5d6a5b1 100644 (file)
--- a/skins/MonoBook.php
+++ b/skins/MonoBook.php
@@ -138,7 +138,7 @@ class MonoBookTemplate extends QuickTemplate {
                        <ul>
        <?php           foreach($this->data['content_actions'] as $key => $tab) {
                                        echo '
-                                <li id="ca-' . Sanitizer::escapeId($key).'"';
+                                <li id="' . Sanitizer::escapeId( "ca-$key" ) . '"';
                                        if( $tab['class'] ) {
                                                echo ' class="'.htmlspecialchars($tab['class']).'"';
                                        }
@@ -165,7 +165,7 @@ class MonoBookTemplate extends QuickTemplate {
                <div class="pBody">
                        <ul>
 <?php                  foreach($this->data['personal_urls'] as $key => $item) { ?>
-                               <li id="pt-<?php echo Sanitizer::escapeId($key) ?>"<?php
+                               <li id="<?php echo Sanitizer::escapeId( "pt-$key" ) ?>"<?php
                                        if ($item['active']) { ?> class="active"<?php } ?>><a href="<?php
                                echo htmlspecialchars($item['href']) ?>"<?php echo $skin->tooltipAndAccesskey('pt-'.$key) ?><?php
                                if(!empty($item['class'])) { ?> class="<?php
@@ -291,7 +291,7 @@ class MonoBookTemplate extends QuickTemplate {
 <?php  }
                if($this->data['feeds']) { ?>
                        <li id="feedlinks"><?php foreach($this->data['feeds'] as $key => $feed) {
-                                       ?><span id="feed-<?php echo Sanitizer::escapeId($key) ?>"><a href="<?php
+                                       ?><span id="<?php echo Sanitizer::escapeId( "feed-$key" ) ?>"><a href="<?php
                                        echo htmlspecialchars($feed['href']) ?>"<?php echo $this->skin->tooltipAndAccesskey('feed-'.$key) ?>><?php echo htmlspecialchars($feed['text'])?></a>&nbsp;</span>
                                        <?php } ?></li><?php
                }
@@ -347,7 +347,7 @@ class MonoBookTemplate extends QuickTemplate {
        /*************************************************************************************************/
        function customBox( $bar, $cont ) {
 ?>
-       <div class='generated-sidebar portlet' id='p-<?php echo Sanitizer::escapeId($bar) ?>'<?php echo $this->skin->tooltip('p-'.$bar) ?>>
+       <div class='generated-sidebar portlet' id='<?php echo Sanitizer::escapeId( "p-$bar" ) ?>'<?php echo $this->skin->tooltip('p-'.$bar) ?>>
                <h5><?php $out = wfMsg( $bar ); if (wfEmptyMsg($bar, $out)) echo $bar; else echo $out; ?></h5>
                <div class='pBody'>
 <?php   if ( is_array( $cont ) ) { ?>