r92044 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92043‎ | r92044 | r92045 >
Date:01:27, 13 July 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
Per wikitech-l, run a bunch of URLs in the API output through wfExpandUrl(), so they become fully-qualified even if they were originally protocol-relative
Modified paths:
  • /trunk/phase3/includes/api/ApiParse.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQuery.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryExtLinksUsage.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryExternalLinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryIWLinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryImageInfo.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryInfo.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryLangLinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQuerySiteinfo.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiQuerySiteinfo.php
——@@ -117,7 +117,7 @@
118118 $data = array();
119119 $mainPage = Title::newMainPage();
120120 $data['mainpage'] = $mainPage->getPrefixedText();
121 - $data['base'] = $mainPage->getFullUrl();
 121+ $data['base'] = wfExpandUrl( $mainPage->getFullUrl() );
122122 $data['sitename'] = $GLOBALS['wgSitename'];
123123 $data['generator'] = "MediaWiki {$GLOBALS['wgVersion']}";
124124 $data['phpversion'] = phpversion();
——@@ -284,7 +284,7 @@
285285 if ( isset( $langNames[$row->iw_prefix] ) ) {
286286 $val['language'] = $langNames[$row->iw_prefix];
287287 }
288 - $val['url'] = $row->iw_url;
 288+ $val['url'] = wfExpandUrl( $row->iw_url );
289289 $val['wikiid'] = $row->iw_wikiid;
290290 $val['api'] = $row->iw_api;
291291
——@@ -448,7 +448,7 @@
449449 protected function appendRightsInfo( $property ) {
450450 global $wgRightsPage, $wgRightsUrl, $wgRightsText;
451451 $title = Title::newFromText( $wgRightsPage );
452 - $url = $title ? $title->getFullURL() : $wgRightsUrl;
 452+ $url = $title ? wfExpandUrl( $title->getFullURL() ) : $wgRightsUrl;
453453 $text = $wgRightsText;
454454 if ( !$text && $title ) {
455455 $text = $title->getPrefixedText();
Index: trunk/phase3/includes/api/ApiParse.php
——@@ -353,7 +353,7 @@
354354
355355 $entry['lang'] = $bits[0];
356356 if ( $title ) {
357 - $entry['url'] = $title->getFullURL();
 357+ $entry['url'] = wfExpandUrl( $title->getFullURL() );
358358 }
359359 $this->getResult()->setContent( $entry, $bits[1] );
360360 $result[] = $entry;
——@@ -435,7 +435,7 @@
436436
437437 $title = Title::newFromText( "{$prefix}:{$title}" );
438438 if ( $title ) {
439 - $entry['url'] = $title->getFullURL();
 439+ $entry['url'] = wfExpandUrl( $title->getFullURL() );
440440 }
441441
442442 $this->getResult()->setContent( $entry, $title->getFullText() );
Index: trunk/phase3/includes/api/ApiQueryExtLinksUsage.php
——@@ -126,6 +126,7 @@
127127 ApiQueryBase::addTitleInfo( $vals, $title );
128128 }
129129 if ( $fld_url ) {
 130+ // We *could* run this through wfExpandUrl() but I think it's better to output the link verbatim, even if it's protocol-relative --Roan
130131 $vals['url'] = $row->el_to;
131132 }
132133 $fit = $result->addValue( array( 'query', $this->getModuleName() ), null, $vals );
——@@ -183,7 +184,7 @@
184185 foreach ( $wgUrlProtocols as $p ) {
185186 $protocols[] = substr( $p, 0, strpos( $p, ':' ) );
186187 }
187 - return $protocols;
 188+ return $protocols;
188189 }
189190
190191 public static function getProtocolPrefix( $protocol ) {
Index: trunk/phase3/includes/api/ApiQuery.php
——@@ -374,7 +374,7 @@
375375 );
376376 if ( $this->iwUrl ) {
377377 $title = Title::newFromText( $rawTitleStr );
378 - $item['url'] = $title->getFullURL();
 378+ $item['url'] = wfExpandUrl( $title->getFullURL() );
379379 }
380380 $intrwValues[] = $item;
381381 }
Index: trunk/phase3/includes/api/ApiQueryIWLinks.php
——@@ -112,7 +112,7 @@
113113 if ( !is_null( $params['url'] ) ) {
114114 $title = Title::newFromText( "{$row->iwl_prefix}:{$row->iwl_title}" );
115115 if ( $title ) {
116 - $entry['url'] = $title->getFullURL();
 116+ $entry['url'] = wfExpandUrl( $title->getFullURL() );
117117 }
118118 }
119119
Index: trunk/phase3/includes/api/ApiQueryLangLinks.php
——@@ -106,7 +106,7 @@
107107 if ( $params['url'] ) {
108108 $title = Title::newFromText( "{$row->ll_lang}:{$row->ll_title}" );
109109 if ( $title ) {
110 - $entry['url'] = $title->getFullURL();
 110+ $entry['url'] = wfExpandUrl( $title->getFullURL() );
111111 }
112112 }
113113 ApiResult::setContent( $entry, $row->ll_title );
Index: trunk/phase3/includes/api/ApiQueryImageInfo.php
——@@ -370,7 +370,7 @@
371371 $vals['thumberror'] = $mto->toText();
372372 }
373373 }
374 - $vals['url'] = $file->getFullURL();
 374+ $vals['url'] = wfExpandUrl( $file->getFullURL() );
375375 $vals['descriptionurl'] = wfExpandUrl( $file->getDescriptionUrl() );
376376 }
377377
Index: trunk/phase3/includes/api/ApiQueryExternalLinks.php
——@@ -86,6 +86,7 @@
8787 break;
8888 }
8989 $entry = array();
 90+ // We *could* run this through wfExpandUrl() but I think it's better to output the link verbatim, even if it's protocol-relative --Roan
9091 ApiResult::setContent( $entry, $row->el_to );
9192 $fit = $this->addPageSubItem( $row->el_from, $entry );
9293 if ( !$fit ) {
Index: trunk/phase3/includes/api/ApiQueryInfo.php
——@@ -380,8 +380,8 @@
381381 }
382382
383383 if ( $this->fld_url ) {
384 - $pageInfo['fullurl'] = $title->getFullURL();
385 - $pageInfo['editurl'] = $title->getFullURL( 'action=edit' );
 384+ $pageInfo['fullurl'] = wfExpandUrl( $title->getFullURL() );
 385+ $pageInfo['editurl'] = wfExpandUrl( $title->getFullURL( 'action=edit' ) );
386386 }
387387 if ( $this->fld_readable && $title->userCanRead() ) {
388388 $pageInfo['readable'] = '';

Follow-up revisions

RevisionCommit summaryAuthorDate
r921721.17wmf1: MFT protocol-relative URL fixes: r91663, r92024, r92028, r92036, r9...catrope17:48, 14 July 2011
r93802Followup r92044: force HTTP on URLs output by the API, now that wfExpandUrl()...catrope07:05, 3 August 2011

Comments

#Comment by Bryan (talk | contribs)   09:06, 13 July 2011

Callers of $title->getFullUrl() should be guaranteed that an absolute url is returned. Protocol relative urls should be returned by a function called $title->getRelativeUrl(). It is imho wrong to let callers of $title->getFullUrl() do the wfExpandUrl, because you are breaking expectations that have been made in the past.

#Comment by Catrope (talk | contribs)   20:34, 14 July 2011

getFullUrl(), currently, will return a fully qualified URL if $wgServer is fully qualified, and a protocol-relative one if $wgServer is protocol-relative. I did it this way because 1) that's what getFullUrl() does right now and 2) there are more places that need protrel URLs than fq ones (fq is an exception that you only need in very specific cases). I could change the behavior of the Title functions to be as you say, then change almost every call to getFullUrl() to getRelativeUrl(), but I don't really see what would be the benefit of that.

#Comment by Bryan (talk | contribs)   14:02, 15 July 2011

Well, that really is a bug with getFullUrl(). I agree however that changing it would not be worth the it.

Status & tagging log