From 6c64bf37452f2ee7dd3e41900a0d9ee0f914b929 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 1 Nov 2013 11:40:27 -0700
Subject: [PATCH] filebackend: path normalization fixes

* Only normalize file path parameters, not other ones
* Actually use the normalized paths instead of throwing it away for the raw paths

Change-Id: I8d36735359f804371e2beae64e5ec6f792d87b27
---
 includes/filebackend/FileOp.php | 69 +++++++++++++++------------------
 1 file changed, 32 insertions(+), 37 deletions(-)

diff --git a/includes/filebackend/FileOp.php b/includes/filebackend/FileOp.php
index fe8330846d..3c5b7b26bc 100644
--- a/includes/filebackend/FileOp.php
+++ b/includes/filebackend/FileOp.php
@@ -63,45 +63,28 @@ abstract class FileOp {
 	 */
 	final public function __construct( FileBackendStore $backend, array $params ) {
 		$this->backend = $backend;
-		list( $required, $optional ) = $this->allowedParams();
-		// @todo normalizeAnyStoragePaths() calls are overzealous, use a parameter list
+		list( $required, $optional, $paths ) = $this->allowedParams();
 		foreach ( $required as $name ) {
 			if ( isset( $params[$name] ) ) {
-				// Normalize paths so the paths to the same file have the same string
-				$this->params[$name] = self::normalizeAnyStoragePaths( $params[$name] );
+				$this->params[$name] = $params[$name];
 			} else {
 				throw new MWException( "File operation missing parameter '$name'." );
 			}
 		}
 		foreach ( $optional as $name ) {
 			if ( isset( $params[$name] ) ) {
-				// Normalize paths so the paths to the same file have the same string
-				$this->params[$name] = self::normalizeAnyStoragePaths( $params[$name] );
+				$this->params[$name] = $params[$name];
 			}
 		}
-		$this->params = $params;
-	}
-
-	/**
-	 * Normalize $item or anything in $item that is a valid storage path
-	 *
-	 * @param string $item|array
-	 * @return string|Array
-	 */
-	protected function normalizeAnyStoragePaths( $item ) {
-		if ( is_array( $item ) ) {
-			$res = array();
-			foreach ( $item as $k => $v ) {
-				$k = self::normalizeIfValidStoragePath( $k );
-				$v = self::normalizeIfValidStoragePath( $v );
-				$res[$k] = $v;
+		foreach ( $paths as $name ) {
+			if ( isset( $this->params[$name] ) ) {
+				// Normalize paths so the paths to the same file have the same string
+				$this->params[$name] = self::normalizeIfValidStoragePath( $this->params[$name] );
 			}
-			return $res;
-		} else {
-			return self::normalizeIfValidStoragePath( $item );
 		}
 	}
 
+
 	/**
 	 * Normalize a string if it is a valid storage path
 	 *
@@ -308,10 +291,10 @@ abstract class FileOp {
 	/**
 	 * Get the file operation parameters
 	 *
-	 * @return Array (required params list, optional params list)
+	 * @return Array (required params list, optional params list, list of params that are paths)
 	 */
 	protected function allowedParams() {
-		return array( array(), array() );
+		return array( array(), array(), array() );
 	}
 
 	/**
@@ -459,8 +442,11 @@ abstract class FileOp {
  */
 class CreateFileOp extends FileOp {
 	protected function allowedParams() {
-		return array( array( 'content', 'dst' ),
-			array( 'overwrite', 'overwriteSame', 'headers' ) );
+		return array(
+			array( 'content', 'dst' ),
+			array( 'overwrite', 'overwriteSame', 'headers' ),
+			array( 'dst' )
+		);
 	}
 
 	protected function doPrecheck( array &$predicates ) {
@@ -511,8 +497,11 @@ class CreateFileOp extends FileOp {
  */
 class StoreFileOp extends FileOp {
 	protected function allowedParams() {
-		return array( array( 'src', 'dst' ),
-			array( 'overwrite', 'overwriteSame', 'headers' ) );
+		return array(
+			array( 'src', 'dst' ),
+			array( 'overwrite', 'overwriteSame', 'headers' ),
+			array( 'src', 'dst' )
+		);
 	}
 
 	protected function doPrecheck( array &$predicates ) {
@@ -573,8 +562,11 @@ class StoreFileOp extends FileOp {
  */
 class CopyFileOp extends FileOp {
 	protected function allowedParams() {
-		return array( array( 'src', 'dst' ),
-			array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'headers' ) );
+		return array(
+			array( 'src', 'dst' ),
+			array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'headers' ),
+			array( 'src', 'dst' )
+		);
 	}
 
 	protected function doPrecheck( array &$predicates ) {
@@ -639,8 +631,11 @@ class CopyFileOp extends FileOp {
  */
 class MoveFileOp extends FileOp {
 	protected function allowedParams() {
-		return array( array( 'src', 'dst' ),
-			array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'headers' ) );
+		return array(
+			array( 'src', 'dst' ),
+			array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'headers' ),
+			array( 'src', 'dst' )
+		);
 	}
 
 	protected function doPrecheck( array &$predicates ) {
@@ -715,7 +710,7 @@ class MoveFileOp extends FileOp {
  */
 class DeleteFileOp extends FileOp {
 	protected function allowedParams() {
-		return array( array( 'src' ), array( 'ignoreMissingSource' ) );
+		return array( array( 'src' ), array( 'ignoreMissingSource' ), array( 'src' ) );
 	}
 
 	protected function doPrecheck( array &$predicates ) {
@@ -760,7 +755,7 @@ class DeleteFileOp extends FileOp {
  */
 class DescribeFileOp extends FileOp {
 	protected function allowedParams() {
-		return array( array( 'src' ), array( 'headers' ) );
+		return array( array( 'src' ), array( 'headers' ), array( 'src' ) );
 	}
 
 	protected function doPrecheck( array &$predicates ) {
-- 
2.20.1