r108312 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r108311‎ | r108312 | r108313 >
Date:09:56, 7 January 2012
Author:nikerabbit
Status:resolved (Comments)
Tags:
Comment:
Don't expand templates in html <title>. This seems to have regressed some time ago.
Modified paths:
  • /trunk/phase3/includes/OutputPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/OutputPage.php
——@@ -813,7 +813,7 @@
814814 $this->mPagetitle = $nameWithTags;
815815
816816 # change "<i>foo&amp;bar</i>" to "foo&bar"
817 - $this->setHTMLTitle( $this->msg( 'pagetitle', Sanitizer::stripAllTags( $nameWithTags ) ) );
 817+ $this->setHTMLTitle( $this->msg( 'pagetitle' )->rawParams( htmlspecialchars( Sanitizer::stripAllTags( $nameWithTags ) ) ) );
818818 }
819819
820820 /**

Comments

#Comment by Krinkle (talk | contribs)   19:07, 8 January 2012

This causes titles on history page to be double escaped "Revision history of "Main Page" -> "Revision history of &quot;Main Page&quot; - Sitename" (Revision history of &amp;quot;Main Page&amp;quot; - Sitename)

#Comment by Nikerabbit (talk | contribs)   19:26, 8 January 2012

Is it safe without htmlspecialchars?

#Comment by Krinkle (talk | contribs)   22:01, 8 January 2012

I'm not sure, but it's double escaping right now.

#Comment by Krinkle (talk | contribs)   22:09, 8 January 2012

This seems to fix the problem, and still looks safe: 

Index: OutputPage.php
===================================================================
--- OutputPage.php	(revision 108368)
+++ OutputPage.php	(working copy)
@@ -813,7 +813,7 @@

  # change "<i>foo&amp;bar</i>" to "foo&bar"
- $this->setHTMLTitle( $this->msg( 'pagetitle' )->rawParams( htmlspecialchars( Sanitizer::stripAllTags( $nameWithTags ) ) ) );
+ $this->setHTMLTitle( $this->msg( 'pagetitle' )->rawParams( Sanitizer::stripAllTags( $nameWithTags ) )->escaped() );
 }
#Comment by Krinkle (talk | contribs)   00:16, 9 January 2012

What kind of template do you mean ? {{SITENAME}} and {{test}} expands both before and after this revision for me.

If you don't want this we should probably use ->plain() instead. Although parsing is expected behavior as the default message is "$1 - {{SITENAME}}.

If the $1 value should not be parsed, then rawParams is indeed good, but I don't think it needs additional escaping. Patch applied in r108376.

Status & tagging log