r90814 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90813‎ | r90814 | r90815 >
Date:02:40, 26 June 2011
Author:bawolff
Status:ok (Comments)
Tags:
Comment:
(bug 8556) When using $wgRawHtml, don't show scary session_failure/possible attack
warning when you try to edit categories.

Categories automatically go to preview on open, but there's no edit token
on the initial open of the edit window, which causes big warning over nothing.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
——@@ -123,6 +123,8 @@
124124 correctly.
125125 * (bug 29567) mw.util.addPortletLink should only wrap link in <span> for
126126 "vectorTabs" portlets.
 127+* (bug 8556) Incorrect session failure warning on preview-on-open
 128+ namespaces (categories) when combined with $wgRawHtml.
127129
128130 === API changes in 1.19 ===
129131 * BREAKING CHANGE: action=watch now requires POST and token.
Index: trunk/phase3/includes/EditPage.php
——@@ -2019,8 +2019,14 @@
20202020 if ( $wgRawHtml && !$this->mTokenOk ) {
20212021 // Could be an offsite preview attempt. This is very unsafe if
20222022 // HTML is enabled, as it could be an attack.
2023 - $parsedNote = $wgOut->parse( "<div class='previewnote'>" .
2024 - wfMsg( 'session_fail_preview_html' ) . "</div>" );
 2023+ $parsedNote = '';
 2024+ if ( $this->textbox1 !== '' ) {
 2025+ // Do not put big scary notice, if previewing the empty
 2026+ // string, which happens when you initially edit
 2027+ // a category page, due to automatic preview-on-open.
 2028+ $parsedNote = $wgOut->parse( "<div class='previewnote'>" .
 2029+ wfMsg( 'session_fail_preview_html' ) . "</div>" );
 2030+ }
20252031 wfProfileOut( __METHOD__ );
20262032 return $parsedNote;
20272033 }

Comments

#Comment by Aaron Schulz (talk | contribs)   22:27, 26 June 2011

Should the $this->textbox1 !== check be up there with if ( $wgRawHtml && !$this->mTokenOk )? What about the code after that check?

#Comment by Bawolff (talk | contribs)   00:05, 27 June 2011

I did it down there to be paranoid. In theory extensions could use the EditPageGetPreviewText hook to look at other request variables (like MetadataEdit extension). This way the function will still return before that if there is no edit token.

Status & tagging log