r51725 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r51724‎ | r51725 | r51726 >
Date:03:20, 11 June 2009
Author:demon
Status:reverted (Comments)
Tags:
Comment:
(bug 18173) Login form exception on malformed REMOTE_ADDR, wfGetIP() now falls back to 127.0.01 if the IP cannot be determined, which is more sane than returning null.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/ProxyTools.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ProxyTools.php
——@@ -76,10 +76,10 @@
7777
7878 /* collect the originating ips */
7979 # Client connecting to this webserver
80 - if ( isset( $_SERVER['REMOTE_ADDR'] ) ) {
 80+ if ( isset( $_SERVER['REMOTE_ADDR'] ) && IP::canonicalize( $_SERVER['REMOTE_ADDR'] ) ) {
8181 $ipchain = array( IP::canonicalize( $_SERVER['REMOTE_ADDR'] ) );
8282 } else {
83 - # Running on CLI?
 83+ # Running on CLI or REMOTE_ADDR is broken
8484 $ipchain = array( '127.0.0.1' );
8585 }
8686 $ip = $ipchain[0];
Index: trunk/phase3/RELEASE-NOTES
——@@ -182,6 +182,8 @@
183183 * Multiple whitespace in TOC anchors is now stripped, for consistency with the
184184 link from the edit comment
185185 * (bug 19112) Preferences now respects $wgUseExternalEditor, $wgExternalDiffEngine
 186+* (bug 18173) Login form exception on malformed REMOTE_ADDR, wfGetIP() now falls
 187+ back to 127.0.01 if the IP cannot be determined
186188
187189 == API changes in 1.16 ==
188190

Follow-up revisions

RevisionCommit summaryAuthorDate
r51774Revert r51725 (fall back to 127.0.0.1 when IP cannot be determined). On furth...demon01:34, 12 June 2009

Comments

#Comment by Simetrical (talk | contribs)   18:51, 11 June 2009

This doesn't seem like the best fix. It's possible to actually edit from localhost, if MediaWiki is running on a desktop or you have shell access to its server. If I saw 127.0.0.1 in the edit history, I'd think this had actually happened. In fact, I do see it on my test wiki, and it's happened on Wikipedia too.

I'd say that a completely invalid REMOTE_HOST is good reason to throw a fatal error. It indicates the configuration is broken and that should be fixed, not ignored. MediaWiki shouldn't silently use known bad IP addresses.

If we really must silently tolerate this for some reason I can't think of, I'd suggest using a totally nonsense IP address, like 0.0.0.0.

#Comment by 😂 (talk | contribs)   02:18, 12 June 2009

Should be resolved in r51774

Status & tagging log