r96934 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r96933‎ | r96934 | r96935 >
Date:00:53, 13 September 2011
Author:raindrift
Status:resolved (Comments)
Tags:
Comment:
If the scaler URL is protocol-relative, things just break here. Sets a sane default.
Modified paths:
  • /trunk/phase3/includes/specials/SpecialUploadStash.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialUploadStash.php
——@@ -210,12 +210,20 @@
211211 // this global probably looks something like 'http://upload.wikimedia.org/wikipedia/test/thumb/temp'
212212 // do not use trailing slash
213213 global $wgUploadStashScalerBaseUrl;
 214+ $scalerBaseUrl = $wgUploadStashScalerBaseUrl;
 215+
 216+ if( preg_match( '/^\/\//', $scalerBaseUrl ) ) {
 217+ // this is apparently a protocol-relative URL, which makes no sense in this context,
 218+ // since this is used for communication that's internal to the application.
 219+ // default to http.
 220+ $scalerBaseUrl = 'http:' . $scalerBaseUrl;
 221+ }
214222
215223 // We need to use generateThumbName() instead of thumbName(), because
216224 // the suffix needs to match the file name for the remote thumbnailer
217225 // to work
218226 $scalerThumbName = $file->generateThumbName( $file->getName(), $params );
219 - $scalerThumbUrl = $wgUploadStashScalerBaseUrl . '/' . $file->getUrlRel() .
 227+ $scalerThumbUrl = $scalerBaseUrl . '/' . $file->getUrlRel() .
220228 '/' . rawurlencode( $scalerThumbName );
221229
222230 // make a curl call to the scaler to create a thumbnail

Follow-up revisions

RevisionCommit summaryAuthorDate
r97078Switched from prepending http for protocol-relative $wgUploadStashScalerBaseU...raindrift17:48, 14 September 2011
r97082Merged revisions 97057,97060,97071,97073-97074,97077-97080 via svnmerge from...dantman18:11, 14 September 2011

Comments

#Comment by 😂 (talk | contribs)   01:29, 13 September 2011

Hardcoding http just seems wrong :\

#Comment by P858snake (talk | contribs)   01:30, 13 September 2011

"+ // since this is used for communication that's internal to the application. + // default to http." Some people force their wikis onto https... I would assume this wouldn't work there?

#Comment by Catrope (talk | contribs)   09:11, 13 September 2011 
+		if( preg_match( '/^\/\//', $scalerBaseUrl ) ) {
+			// this is apparently a protocol-relative URL, which makes no sense in this context,
+			// since this is used for communication that's internal to the application.
+			// default to http.
+			$scalerBaseUrl = 'http:' . $scalerBaseUrl;
+		}

We have a utility function for that: wfExpandUrl( $scalerBaseUrl, PROTO_HTTP ); . To address the other comments, you might use PROTO_CANONICAL instead so the protocol of $wgCanonicalServer is used.

#Comment by Raindrift (talk | contribs)   17:49, 14 September 2011

Yeah, these are good points. Fixed in r97078.

Thanks for the tip, Roan.

#Comment by 😂 (talk | contribs)   23:30, 15 September 2011

Also rather than doing a regular expression, it'd be faster and just as accurate to use a substr().

Status & tagging log