r94990 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r94989‎ | r94990 | r94991 >
Date:08:56, 19 August 2011
Author:catrope
Status:resolved
Tags:
Comment:
Followup r94754: move protocol expansion from getIRCLine() to inside getInternalUrl(); these internal URLs should always be fully qualified. Effectively this means that if $wgServer is protocol-relative and $wgInternalServer either isn't set or is also protocol relative (but it really shouldn't be the latter!), we'll add http:// to URLs used for Squid purging and in IRC lines. If people want those to be https:// instead, they can set $wgInternalServer accordingly.

We should really, really break the connection between internal URLs used for Squid purges and URLs used for IRC, and use the latter in e-mails as well. This was briefly discussed in the CR comments on r44412 and I'll be working on that today.
Modified paths:
  • /trunk/phase3/includes/RecentChange.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/RecentChange.php
@@ -707,9 +707,7 @@
708708 // XXX: *HACK^2* the preg_replace() undoes much of what getInternalURL() does, but we
709709 // XXX: need to call it so that URL paths on the Wikimedia secure server can be fixed
710710 // XXX: by a custom GetInternalURL hook --vyznev 2008-12-10
711 - // XXX: Also, getInternalUrl() may return a protocol-relative URL.
712 - // XXX: In that case, expand it to an HTTP URL, even if this is an HTTPS request --catrope 2011-08-17
713 - $url = preg_replace( '/title=[^&]*&/', '', wfExpandUrl( $titleObj->getInternalURL( $url ), PROTO_HTTP ) );
 711+ $url = preg_replace( '/title=[^&]*&/', '', $titleObj->getInternalURL( $url ) );
714712 }
715713
716714 if( isset( $this->mExtra['oldSize'] ) && isset( $this->mExtra['newSize'] ) ) {
Index: trunk/phase3/includes/Title.php
@@ -992,6 +992,10 @@
993993 * Get the URL form for an internal link.
994994 * - Used in various Squid-related code, in case we have a different
995995 * internal hostname for the server from the exposed one.
 996+ *
 997+ * This uses $wgInternalServer to qualify the path, or $wgServer
 998+ * if $wgInternalServer is not set. If the server variable used is
 999+ * protocol-relative, the URL will be expanded to http://
9961000 *
9971001 * @param $query String an optional query string
9981002 * @param $variant String language variant of url (for sr, zh..)
@@ -1000,7 +1004,7 @@
10011005 public function getInternalURL( $query = '', $variant = false ) {
10021006 global $wgInternalServer, $wgServer;
10031007 $server = $wgInternalServer !== false ? $wgInternalServer : $wgServer;
1004 - $url = $server . $this->getLocalURL( $query, $variant );
 1008+ $url = wfExpandUrl( $server . $this->getLocalURL( $query, $variant ), PROTO_HTTP );
10051009 wfRunHooks( 'GetInternalURL', array( &$this, &$url, $query ) );
10061010 return $url;
10071011 }

Sign-offs

UserFlagDate
Nikerabbitinspected09:39, 19 August 2011
Nikerabbittested09:39, 19 August 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r94995Per CR on r44412 and my promise in the commit summary of r94990, stop abusing...catrope11:23, 19 August 2011
r955051.17wmf1: Merge a truckload of HTTPS / prot rel URL fixes: r93847, r94990, r9...catrope19:32, 25 August 2011
r964751.18: MFT r94737, r94990, r95000, r95001, r95002, r95006, r95007, r95010, r95...catrope19:37, 7 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r44412Seems r44406 would've broken the RC feed on secure.wikimedia org: fix by intr...vyznev17:03, 10 December 2008
r94754(bug 30398) Expand any protocol-relative URLs to HTTP in RecentChange::getIRC...catrope13:32, 17 August 2011

Status & tagging log