COMMON: Let Common::normalizePath normalize '..' in paths

There are some unit tests to verify that this works correctly.
There is a small chance that this causes regressions in weird setups.

2 files changed, 44 insertions, 23 deletions

diff --git a/common/str.cpp b/common/str.cpp
index 740e7b6a06..223188bdd6 100644
--- a/common/str.cpp
+++ b/common/str.cpp
@@ -19,11 +19,11 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  */
 
-#include "common/str.h"
 #include "common/hash-str.h"
-#include "common/util.h"
-
+#include "common/list.h"
 #include "common/memorypool.h"
+#include "common/str.h"
+#include "common/util.h"
 
 #include <stdarg.h>
 
@@ -256,7 +256,7 @@ String &String::operator=(char c) {
 
 String &String::operator+=(const char *str) {
 	if (_str <= str && str <= _str + _size)
-		return operator+=(Common::String(str));
+		return operator+=(String(str));
 
 	int len = strlen(str);
 	if (len > 0) {
@@ -270,7 +270,7 @@ String &String::operator+=(const char *str) {
 
 String &String::operator+=(const String &str) {
 	if (&str == this)
-		return operator+=(Common::String(str));
+		return operator+=(String(str));
 
 	int len = str._size;
 	if (len > 0) {
@@ -612,7 +612,7 @@ char *trim(char *t) {
 	return rtrim(ltrim(t));
 }
 
-Common::String lastPathComponent(const Common::String &path, const char sep) {
+String lastPathComponent(const String &path, const char sep) {
 	const char *str = path.c_str();
 	const char *last = str + path.size();
 
@@ -622,7 +622,7 @@ Common::String lastPathComponent(const Common::String &path, const char sep) {
 
 	// Path consisted of only slashes -> return empty string
 	if (last == str)
-		return Common::String();
+		return String();
 
 	// Now scan the whole component
 	const char *first = last - 1;
@@ -632,24 +632,26 @@ Common::String lastPathComponent(const Common::String &path, const char sep) {
 	if (*first == sep)
 		first++;
 
-	return Common::String(first, last);
+	return String(first, last);
 }
 
-Common::String normalizePath(const Common::String &path, const char sep) {
+String normalizePath(const String &path, const char sep) {
 	if (path.empty())
 		return path;
 
 	const char *cur = path.c_str();
-	Common::String result;
+	String result;
 
 	// If there is a leading slash, preserve that:
 	if (*cur == sep) {
 		result += sep;
+		// Skip over multiple leading slashes, so "//" equals "/"
 		while (*cur == sep)
 			++cur;
 	}
 
-	// Scan till the end of the String
+	// Scan for path components till the end of the String
+	List<String> comps;
 	while (*cur != 0) {
 		const char *start = cur;
 
@@ -657,18 +659,16 @@ Common::String normalizePath(const Common::String &path, const char sep) {
 		while (*cur != sep && *cur != 0)
 			cur++;
 
-		const Common::String component(start, cur);
-
-		// Skip empty components and dot components, add all others
-		if (!component.empty() && component != ".") {
-			// Add a separator before the component, unless the result
-			// string already ends with one (which happens only if the
-			// path *starts* with a separator).
-			if (!result.empty() && result.lastChar() != sep)
-				result += sep;
+		const String component(start, cur);
 
-			// Add the component
-			result += component;
+		if (component.empty() || component == ".") {
+			// Skip empty components and dot components
+		} else if (!comps.empty() && component == ".." && comps.back() != "..") {
+			// If stack is non-empty and top is not "..", remove top
+			comps.pop_back();
+		} else {
+			// Add the component to the stack
+			comps.push_back(component);
 		}
 
 		// Skip over separator chars
@@ -676,6 +676,14 @@ Common::String normalizePath(const Common::String &path, const char sep) {
 			cur++;
 	}
 
+	// Finally, assemble all components back into a path
+	while (!comps.empty()) {
+		result += comps.front();
+		comps.pop_front();
+		if (!comps.empty())
+			result += sep;
+	}
+
 	return result;
 }
 
@@ -749,7 +757,7 @@ String tag2string(uint32 tag) {
 		if (!isprint((unsigned char)str[i]))
 			str[i] = '.';
 	}
-	return Common::String(str);
+	return String(str);
 }
 
 size_t strlcpy(char *dst, const char *src, size_t size) {
diff --git a/test/common/str.h b/test/common/str.h
index 0dee16a493..2c563f3132 100644
--- a/test/common/str.h
+++ b/test/common/str.h
@@ -284,6 +284,19 @@ class StringTestSuite : public CxxTest::TestSuite
 		TS_ASSERT_EQUALS(Common::normalizePath("foo/./bar", '/'), "foo/bar");
 		TS_ASSERT_EQUALS(Common::normalizePath("foo//./bar//", '/'), "foo/bar");
 		TS_ASSERT_EQUALS(Common::normalizePath("foo//.bar//", '/'), "foo/.bar");
+
+		TS_ASSERT_EQUALS(Common::normalizePath("..", '/'), "..");
+		TS_ASSERT_EQUALS(Common::normalizePath("../", '/'), "..");
+		TS_ASSERT_EQUALS(Common::normalizePath("/..", '/'), "/..");
+		TS_ASSERT_EQUALS(Common::normalizePath("../bar", '/'), "../bar");
+		TS_ASSERT_EQUALS(Common::normalizePath("foo//../", '/'), "");
+		TS_ASSERT_EQUALS(Common::normalizePath("foo/../bar", '/'), "bar");
+		TS_ASSERT_EQUALS(Common::normalizePath("foo//../bar//", '/'), "bar");
+		TS_ASSERT_EQUALS(Common::normalizePath("foo//..bar//", '/'), "foo/..bar");
+
+		TS_ASSERT_EQUALS(Common::normalizePath("foo/../../bar//", '/'), "../bar");
+		TS_ASSERT_EQUALS(Common::normalizePath("../foo/../bar", '/'), "../bar");
+		TS_ASSERT_EQUALS(Common::normalizePath("../../foo/bar/", '/'), "../../foo/bar");
 	}
 
 	void test_matchString() {