Fix URI percent-decoding

badaix · May 3, 2021 · a2168f1 · a2168f1
1 parent e37d81f
commit a2168f1
Show file tree

Hide file tree

Showing 5 changed files with 18,076 additions and 19 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -6,7 +6,7 @@ set(PROJECT_URL "https://github.com/badaix/snapcast")
 
 option(BUILD_SHARED_LIBS "Build snapcast in a shared context" ON)
 option(BUILD_STATIC_LIBS "Build snapcast in a static context" ON)
-option(BUILD_TESTS "Build tests (run tests with make test)" ON)
+option(BUILD_TESTS "Build tests (in test/snapcast_test)" ON)
 option(WERROR "Treat warnings as errors" OFF)
 
 option(ASAN "Enable AddressSanitizer" OFF)
@@ -20,6 +20,10 @@ IF (REVISION)
     add_definitions(-DREVISION=\"${REVISION}\")
 ENDIF()
 
+if (BUILD_TESTS)
+    add_subdirectory(test)
+endif (BUILD_TESTS)
+
 IF (TIDY)
     FIND_PROGRAM(CLANG_TIDY "clang-tidy")
     IF(CLANG_TIDY)

diff --git a/server/streamreader/stream_uri.cpp b/server/streamreader/stream_uri.cpp
@@ -47,17 +47,17 @@ void StreamUri::parse(const std::string& streamUri)
     while (!uri.empty() && ((uri[uri.length() - 1] == '\'') || (uri[uri.length() - 1] == '"')))
         uri = uri.substr(0, this->uri.length() - 1);
 
-    string decodedUri = strutils::uriDecode(uri);
-    LOG(DEBUG) << "StreamUri decoded: " << decodedUri << "\n";
+    // string decodedUri = strutils::uriDecode(uri);
+    // LOG(DEBUG) << "StreamUri decoded: " << decodedUri << "\n";
 
-    string tmp(decodedUri);
+    string tmp(uri);
 
     pos = tmp.find(':');
     if (pos == string::npos)
         throw invalid_argument("missing ':'");
-    scheme = strutils::trim_copy(tmp.substr(0, pos));
+    scheme = strutils::uriDecode(strutils::trim_copy(tmp.substr(0, pos)));
     tmp = tmp.substr(pos + 1);
-    LOG(DEBUG) << "scheme: '" << scheme << "' tmp: '" << tmp << "'\n";
+    LOG(TRACE) << "scheme: '" << scheme << "', tmp: '" << tmp << "'\n";
 
     if (tmp.find("//") != 0)
         throw invalid_argument("missing host separator: '//'");
@@ -71,27 +71,29 @@ void StreamUri::parse(const std::string& streamUri)
             pos = tmp.length();
     }
 
-    host = strutils::trim_copy(tmp.substr(0, pos));
+    host = strutils::uriDecode(strutils::trim_copy(tmp.substr(0, pos)));
     tmp = tmp.substr(pos);
     path = tmp;
-    LOG(DEBUG) << "host: '" << host << "' tmp: '" << tmp << "' path: '" << path << "'\n";
+    pos = std::min(path.find('?'), path.find('#'));
+    path = strutils::uriDecode(strutils::trim_copy(path.substr(0, pos)));
+    LOG(TRACE) << "host: '" << host << "', tmp: '" << tmp << "', path: '" << path << "'\n";
 
+    string queryStr;
     pos = tmp.find('?');
-    if (pos == string::npos)
-        return;
-
-    path = strutils::trim_copy(tmp.substr(0, pos));
-    tmp = tmp.substr(pos + 1);
-    string queryStr = tmp;
-    LOG(DEBUG) << "path: '" << path << "' tmp: '" << tmp << "' query: '" << queryStr << "'\n";
+    if (pos != string::npos)
+    {
+        tmp = tmp.substr(pos + 1);
+        queryStr = tmp;
+        LOG(TRACE) << "path: '" << path << "', tmp: '" << tmp << "', query: '" << queryStr << "'\n";
+    }
 
     pos = tmp.find('#');
     if (pos != string::npos)
     {
         queryStr = tmp.substr(0, pos);
         tmp = tmp.substr(pos + 1);
-        fragment = strutils::trim_copy(tmp);
-        LOG(DEBUG) << "query: '" << queryStr << "' fragment: '" << fragment << "' tmp: '" << tmp << "'\n";
+        fragment = strutils::uriDecode(strutils::trim_copy(tmp));
+        LOG(TRACE) << "query: '" << queryStr << "', fragment: '" << fragment << "', tmp: '" << tmp << "'\n";
     }
 
     vector<string> keyValueList = strutils::split(queryStr, '&');
@@ -100,8 +102,8 @@ void StreamUri::parse(const std::string& streamUri)
         pos = kv.find('=');
         if (pos != string::npos)
         {
-            string key = strutils::trim_copy(kv.substr(0, pos));
-            string value = strutils::trim_copy(kv.substr(pos + 1));
+            string key = strutils::uriDecode(strutils::trim_copy(kv.substr(0, pos)));
+            string value = strutils::uriDecode(strutils::trim_copy(kv.substr(pos + 1)));
             query[key] = value;
         }
     }

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -0,0 +1,10 @@
+# Prepare "Catch" library for other executables
+set(CATCH_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+add_library(Catch INTERFACE)
+target_include_directories(Catch INTERFACE ${CATCH_INCLUDE_DIR} ${CMAKE_SOURCE_DIR})
+
+# Make test executable
+set(TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/test_main.cpp ${CMAKE_SOURCE_DIR}/server/streamreader/stream_uri.cpp)
+add_executable(snapcast_test ${TEST_SOURCES})
+target_link_libraries(snapcast_test Catch)
+