r95663 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r95662‎ | r95663 | r95664 >
Date:14:37, 29 August 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
Last commit to make WMF-deployed extensions HTTPS-ready (hopefully): use wfExpandUrl() in a bunch of places
* SpamBlacklist: code is weird but I'm pretty sure this needs HTTP
* ContributionTracking: expand return URL to current protocol. Use HTTP in the test suite (PROTO_CURRENT makes no sense in tests since they run from the command line)
* GlobalUsage: remove URL expansion, not needed after r95651
* CentralNotice: expand URL because it gets fed to window.location indirectly via JS
* OpenSearhXml: use canonical URLs in XML output
* MobileFrontend: expand a URL that's used in a Location: header
Modified paths:
  • /trunk/extensions/CentralNotice/special/SpecialCentralNoticeLogs.php (modified) (history)
  • /trunk/extensions/ContributionTracking/ContributionTracking.processor.php (modified) (history)
  • /trunk/extensions/ContributionTracking/tests/ContributionTrackingAPITest.php (modified) (history)
  • /trunk/extensions/ContributionTracking/tests/ContributionTrackingProcessorTest.php (modified) (history)
  • /trunk/extensions/ContributionTracking/tests/ContributionTrackingTest.php (modified) (history)
  • /trunk/extensions/GlobalUsage/ApiQueryGlobalUsage.php (modified) (history)
  • /trunk/extensions/MobileFrontend/MobileFrontend.php (modified) (history)
  • /trunk/extensions/OpenSearchXml/ApiOpenSearchXml.php (modified) (history)
  • /trunk/extensions/SpamBlacklist/SpamBlacklist_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/GlobalUsage/ApiQueryGlobalUsage.php
——@@ -62,8 +62,7 @@
6363 'wiki' => WikiMap::getWikiName( $wiki )
6464 );
6565 if ( isset( $prop['url'] ) ) {
66 - $result['url'] = wfExpandUrl(
67 - WikiMap::getForeignUrl( $item['wiki'], $title ) );
 66+ $result['url'] = WikiMap::getForeignUrl( $item['wiki'], $title );
6867 }
6968 if ( isset( $prop['pageid'] ) ) {
7069 $result['pageid'] = $item['id'];
Index: trunk/extensions/ContributionTracking/tests/ContributionTrackingAPITest.php
——@@ -90,7 +90,7 @@
9191 'business' => 'donations@wikimedia.org',
9292 'item_number' => 'DONATE',
9393 'no_note' => 0,
94 - 'return' => $returnTitle->getFullUrl(),
 94+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ),
9595 'currency_code' => 'USD',
9696 'cmd' => '_xclick',
9797 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
——@@ -124,7 +124,7 @@
125125 'business' => 'donations@wikimedia.org',
126126 'item_number' => 'DONATE',
127127 'no_note' => 0,
128 - 'return' => $returnTitle->getFullUrl(),
 128+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ),
129129 'currency_code' => 'USD',
130130 'cmd' => '_xclick',
131131 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
——@@ -171,7 +171,7 @@
172172 'business' => 'donations@wikimedia.org',
173173 'item_number' => 'DONATE',
174174 'no_note' => 0,
175 - 'return' => $returnTitle->getFullUrl(), //Important to the language test.
 175+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ), //Important to the language test.
176176 'currency_code' => 'USD',
177177 'cmd' => '_xclick',
178178 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
——@@ -209,7 +209,7 @@
210210 'business' => 'donations@wikimedia.org',
211211 'item_number' => 'DONATE',
212212 'no_note' => 0,
213 - 'return' => $returnTitle->getFullUrl(),
 213+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ),
214214 'currency_code' => 'USD',
215215 'cmd' => '_xclick',
216216 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
Index: trunk/extensions/ContributionTracking/tests/ContributionTrackingProcessorTest.php
——@@ -196,7 +196,7 @@
197197 'business' => 'donations@wikimedia.org',
198198 'item_number' => 'DONATE',
199199 'no_note' => 0,
200 - 'return' => $returnTitle->getFullUrl(),
 200+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ),
201201 'currency_code' => 'USD',
202202 'cmd' => '_xclick',
203203 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
——@@ -261,7 +261,7 @@
262262 'business' => 'donations@wikimedia.org',
263263 'item_number' => 'DONATE',
264264 'no_note' => 0,
265 - 'return' => $returnTitle->getFullURL(), //Important to the language test.
 265+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ), //Important to the language test.
266266 'currency_code' => 'USD',
267267 'cmd' => '_xclick',
268268 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
——@@ -288,7 +288,7 @@
289289 'business' => 'donations@wikimedia.org',
290290 'item_number' => 'DONATE',
291291 'no_note' => 0,
292 - 'return' => $returnTitle->getFullURL(),
 292+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ),
293293 'currency_code' => 'USD',
294294 'cmd' => '_xclick',
295295 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
Index: trunk/extensions/ContributionTracking/tests/ContributionTrackingTest.php
——@@ -113,7 +113,7 @@
114114 'business' => 'donations@wikimedia.org',
115115 'item_number' => 'DONATE',
116116 'no_note' => 0,
117 - 'return' => $returnTitle->getFullUrl(),
 117+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ),
118118 'currency_code' => 'USD',
119119 'cmd' => '_xclick',
120120 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
——@@ -147,7 +147,7 @@
148148 'business' => 'donations@wikimedia.org',
149149 'item_number' => 'DONATE',
150150 'no_note' => 0,
151 - 'return' => $returnTitle->getFullUrl(),
 151+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ),
152152 'currency_code' => 'USD',
153153 'cmd' => '_xclick',
154154 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
——@@ -194,7 +194,7 @@
195195 'business' => 'donations@wikimedia.org',
196196 'item_number' => 'DONATE',
197197 'no_note' => 0,
198 - 'return' => $returnTitle->getFullUrl(), //Important to the language test.
 198+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ), //Important to the language test.
199199 'currency_code' => 'USD',
200200 'cmd' => '_xclick',
201201 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
——@@ -233,7 +233,7 @@
234234 'business' => 'donations@wikimedia.org',
235235 'item_number' => 'DONATE',
236236 'no_note' => 0,
237 - 'return' => $returnTitle->getFullUrl(),
 237+ 'return' => wfExpandUrl( $returnTitle->getFullUrl(), PROTO_HTTP ),
238238 'currency_code' => 'USD',
239239 'cmd' => '_xclick',
240240 'notify_url' => 'https://civicrm.wikimedia.org/fundcore_gateway/paypal',
Index: trunk/extensions/ContributionTracking/ContributionTracking.processor.php
——@@ -322,7 +322,7 @@
323323 $returnText = $input['return'];
324324 $returnTitle = Title::newFromText( $returnText );
325325 if ( $returnTitle ) {
326 - $returnto = $returnTitle->getFullUrl();
 326+ $returnto = wfExpandUrl( $returnTitle->getFullUrl(), PROTO_CURRENT );
327327 } else {
328328 $returnto = $wgContributionTrackingReturnToURLDefault . "/$language";
329329 }
Index: trunk/extensions/MobileFrontend/MobileFrontend.php
——@@ -327,7 +327,7 @@
328328 if ( $mobileAction == 'opt_in_cookie' ) {
329329 $this->setOptInOutCookie( '1' );
330330 $this->disableCaching();
331 - $location = Title::newMainPage()->getFullURL();
 331+ $location = wfExpandUrl( Title::newMainPage()->getFullURL(), PROTO_CURRENT );
332332 $wgRequest->response()->header( 'Location: ' . $location );
333333 }
334334
Index: trunk/extensions/SpamBlacklist/SpamBlacklist_body.php
——@@ -35,7 +35,7 @@
3636 }
3737 }
3838
39 - $thisHttp = $title->getFullUrl( 'action=raw' );
 39+ $thisHttp = wfExpandUrl( $title->getFullUrl( 'action=raw' ), PROT_HTTP );
4040 $thisHttpRegex = '/^' . preg_quote( $thisHttp, '/' ) . '(?:&.*)?$/';
4141
4242 foreach( $this->files as $fileName ) {
Index: trunk/extensions/CentralNotice/special/SpecialCentralNoticeLogs.php
——@@ -53,7 +53,7 @@
5454 $htmlOut .= Xml::element( 'h2', null, wfMsg( 'centralnotice-view-logs' ) );
5555 $htmlOut .= Xml::openElement( 'div', array( 'id' => 'cn-log-switcher' ) );
5656 $title = SpecialPage::getTitleFor( 'CentralNoticeLogs' );
57 - $fullUrl = $title->getFullUrl();
 57+ $fullUrl = wfExpandUrl( $title->getFullUrl(), PROTO_CURRENT );
5858
5959 $htmlOut .= Xml::radio(
6060 'log_type',
Index: trunk/extensions/OpenSearchXml/ApiOpenSearchXml.php
——@@ -122,11 +122,11 @@
123123 $item = array();
124124 $item['Text']['*'] = $title->getPrefixedText();
125125 $item['Description']['*'] = $extract;
126 - $item['Url']['*'] = $title->getFullUrl();
 126+ $item['Url']['*'] = wfExpandUrl( $title->getFullUrl(), PROTO_CURRENT );
127127 if( $image ) {
128128 $thumb = $image->transform( array( 'width' => 50, 'height' => 50 ), 0 );
129129 $item['Image'] = array(
130 - 'source' => wfExpandUrl( $thumb->getUrl() ),
 130+ 'source' => wfExpandUrl( $thumb->getUrl(), PROTO_CANONICAL ),
131131 //alt
132132 'width' => $thumb->getWidth(),
133133 'height' => $thumb->getHeight() );

Follow-up revisions

RevisionCommit summaryAuthorDate
r95855Followup r95663: per CR, use the current protocol for expanding the thumbnail...catrope12:09, 31 August 2011
r95899Fix misspelled constant in r95663catrope19:07, 31 August 2011
r959011.17wmf1: MFT HTTPS / prot rel URL fixes: r95627, r95651, r95652, r95653, r95...catrope19:15, 31 August 2011
r964851.18: MFT protocol-relative URL saga: r95014, r95016, r95017, r95627, r95651,...catrope20:14, 7 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r95651In WikiMap, pull wgCanonicalServer from $wgConf instead of wgServer for build...catrope08:55, 29 August 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   18:54, 29 August 2011

Shouldn't ApiOpenSearchXml::formatItem() use PROTO_CURRENT for the image thumbnail as well as the link? Feels wrong to deliberately load mixed content here.

#Comment by Catrope (talk | contribs)   21:34, 30 August 2011

Hmm, yeah, probably.

Status & tagging log