From: Thiemo Kreuz <thiemo.kreuz@wikimedia.de>
Date: Wed, 2 Jan 2019 15:29:04 +0000 (+0100)
Subject: filerepo: Replace confusing substr() with rtrim()
X-Git-Tag: 1.34.0-rc.0~3159^2
X-Git-Url: http://git.cyclocoop.org/%7B%7B%20url_for%28%27admin_vote_add%27%29%20%7D%7D?a=commitdiff_plain;h=4dba60597840b21c0ce0af25df4f3c835b9cd321;p=lhc%2Fweb%2Fwiklou.git

filerepo: Replace confusing substr() with rtrim()

I just looked at this code for ten minutes to understand what
sub-string is extracted here. Not only are the numbers hard to
understand: With 0 and -1 the last character is removed – that's
really all this does. Even more problematic is the fact there is
no check, hint, not even a comment explaining *which* character is
removed. If – for whatever reason – the code above returns a string
that does *not* end with a slash, the unconditional substr() call
must destroy this string.

rtrim() shows the character.

I checked and these strings are all guaranteed to use forward
slashes, never backwards (Windows) slashes.

Change-Id: I2e17fd583982920bb8a0ca73035094099e5e5d31
---

diff --git a/includes/filerepo/file/File.php b/includes/filerepo/file/File.php
index 76bdba4b41..923484add8 100644
--- a/includes/filerepo/file/File.php
+++ b/includes/filerepo/file/File.php
@@ -1543,7 +1543,7 @@ abstract class File implements IDBAccessObject {
 	function getArchiveRel( $suffix = false ) {
 		$path = 'archive/' . $this->getHashPath();
 		if ( $suffix === false ) {
-			$path = substr( $path, 0, -1 );
+			$path = rtrim( $path, '/' );
 		} else {
 			$path .= $suffix;
 		}
@@ -1655,7 +1655,7 @@ abstract class File implements IDBAccessObject {
 		$ext = $this->getExtension();
 		$path = $this->repo->getZoneUrl( 'public', $ext ) . '/archive/' . $this->getHashPath();
 		if ( $suffix === false ) {
-			$path = substr( $path, 0, -1 );
+			$path = rtrim( $path, '/' );
 		} else {
 			$path .= rawurlencode( $suffix );
 		}
@@ -1746,7 +1746,7 @@ abstract class File implements IDBAccessObject {
 		$this->assertRepoDefined();
 		$path = $this->repo->getVirtualUrl() . '/public/archive/' . $this->getHashPath();
 		if ( $suffix === false ) {
-			$path = substr( $path, 0, -1 );
+			$path = rtrim( $path, '/' );
 		} else {
 			$path .= rawurlencode( $suffix );
 		}