r89088 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89087‎ | r89088 | r89089 >
Date:09:32, 29 May 2011
Author:happy-melon
Status:ok (Comments)
Tags:
Comment:
Rv r84022 for now: crashes PHP on large url strings (bug29197), which is a nasty DOS vector. Leaving the parser tests in because this should definitely be fixed and reimplemented...
Modified paths:
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/parser/Parser.php
——@@ -68,7 +68,7 @@
6969
7070 # Constants needed for external link processing
7171 # Everything except bracket, space, or control characters
72 - const EXT_LINK_URL_CLASS = '(?:[^\]\[<>"\\x00-\\x20\\x7F]|(?:\[\]))';
 72+ const EXT_LINK_URL_CLASS = '[^][<>"\\x00-\\x20\\x7F]';
7373 const EXT_IMAGE_REGEX = '/^(http:\/\/|https:\/\/)([^][<>"\\x00-\\x20\\x7F]+)
7474 \\/([A-Za-z0-9_.,~%\\-+&;#*?!=()@\\x80-\\xFF]+)\\.((?i)gif|png|jpg|jpeg)$/Sx';
7575
——@@ -184,7 +184,7 @@
185185 $this->mConf = $conf;
186186 $this->mUrlProtocols = wfUrlProtocols();
187187 $this->mExtLinkBracketedRegex = '/\[(\b(' . wfUrlProtocols() . ')'.
188 - '(?:[^\]\[<>"\x00-\x20\x7F]|\[\])+) *([^\]\\x00-\\x08\\x0a-\\x1F]*?)\]/S';
 188+ '[^][<>"\\x00-\\x20\\x7F]+) *([^\]\\x00-\\x08\\x0a-\\x1F]*?)\]/S';
189189 if ( isset( $conf['preprocessorClass'] ) ) {
190190 $this->mPreprocessorClass = $conf['preprocessorClass'];
191191 } elseif ( extension_loaded( 'domxml' ) ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r89208revert r84022 parser tests (code reverted by r89088)...hashar06:17, 31 May 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r84022Accept empty square bracket pairs in external links: http://www.example.com?f...happy-melon15:09, 15 March 2011

Comments

#Comment by Hashar (talk | contribs)   19:54, 30 May 2011

This change break parser tests 'External links:'

php parserTests.php --quiet --regex 'External links:'

They were fine at r89087

#Comment by Happy-melon (talk | contribs)   20:09, 30 May 2011
“ Leaving the parser tests in because this should definitely be fixed and reimplemented ”

Option 1: revert the parser tests from r84022 as well and forget that this feature ever existed. Option 2: leave the parser tests as a reminder that this feature was a Good Thing and should be restored.

?

#Comment by Hashar (talk | contribs)   06:11, 31 May 2011

I am pretty sure someone else encountered a similar PHP issue. I can not remember wich bug / revision though :(

Status & tagging log