Fixed handling of update URLs which mention a port

This appears to have been a regression in
f405849b49614254f42eb3ee6147434458978623, which for some reason erased
the port (and any trailing parts) from not just the update directory but
also from the update URL.

Unfortunately we can't access Mana-Mantis #381 at the moment, but
presumably the port was removed due to the colon being a problematic
character. Instead, now the colon (and other special characters) are
replaced by _ when determining the update directory.

Closes https://git.themanaworld.org/mana/mana/-/issues/80

7 files changed, 66 insertions, 91 deletions

diff --git a/NEWS b/NEWS
index ed8d1c7f..a2cc3091 100644
--- a/NEWS
+++ b/NEWS
@@ -58,6 +58,7 @@
 - Fixed empty Equipment window on freshly created character
 - Fixed choosing default world when using -D command-line parameter
 - Fixed storing of player relations
+- Fixed handling of custom port in update URL
 - Updated to tmwAthena protocol changes
 - Updated to Manaserv protocol changes (specials, guilds, debug mode, skills, text particles)
 - CMake: Use GNUInstallDirs and made PKG_DATADIR / PKG_BINDIR paths modifiable
diff --git a/src/client.cpp b/src/client.cpp
index 40f41b30..e4832a3e 100644
--- a/src/client.cpp
+++ b/src/client.cpp
@@ -654,22 +654,17 @@ int Client::exec()
                     break;
 
                 case STATE_UPDATE:
-                    // Determine which source to use for the update host
-                    if (!mOptions.updateHost.empty())
-                        mUpdateHost = mOptions.updateHost;
-                    else
-                        mUpdateHost = loginData.updateHost;
-                    initUpdatesDir();
+                    logger->log("State: UPDATE");
 
                     if (mOptions.skipUpdate)
                     {
                         mState = STATE_LOAD_DATA;
                     }
-                    else
+                    else if (initUpdatesDir())
                     {
-                        logger->log("State: UPDATE");
                         mCurrentDialog = new UpdaterWindow(mUpdateHost,
-                                mLocalDataDir + "/" + mUpdatesDir,mOptions.dataPath.empty());
+                                                           mLocalDataDir + "/" + mUpdatesDir,
+                                                           mOptions.dataPath.empty());
                     }
                     break;
 
@@ -1104,39 +1099,39 @@ void Client::initConfiguration()
  * Parse the update host and determine the updates directory
  * Then verify that the directory exists (creating if needed).
  */
-void Client::initUpdatesDir()
+bool Client::initUpdatesDir()
 {
-    // If updatesHost is currently empty, fill it from config file
-    if (mUpdateHost.empty())
+    // Determine which source to use for the update host
+    if (!mOptions.updateHost.empty())
+        mUpdateHost = mOptions.updateHost;
+    else if (!loginData.updateHost.empty())
+        mUpdateHost = loginData.updateHost;
+    else
         mUpdateHost = config.updatehost;
 
-    // Exit on empty update host.
-    if (mUpdateHost.empty())
-        return;
-
-    logger->log("Setting update host: %s", mUpdateHost.c_str());
+    // Remove any trailing slashes at the end of the URL
+    while (!mUpdateHost.empty() && mUpdateHost.back() == '/')
+        mUpdateHost.pop_back();
 
-    std::string updateHost = getHostNameFromURL(mUpdateHost);
-
-    // Exit on a wrong update host.
-    if (updateHost.length() < 2)
+    if (mUpdateHost.empty())
     {
-        // Show the original updateHostname in the error message.
-        errorMessage = strprintf(_("Invalid update host: %s"),
-                                 mUpdateHost.c_str());
-        mState = STATE_ERROR;
-        return;
+        logger->log("No update host provided");
+        mUpdatesDir.clear();
+        mState = STATE_LOAD_DATA;
+        return false;
     }
 
-    mUpdateHost = updateHost;
-    mUpdatesDir = "updates/" + mUpdateHost;
+    mUpdatesDir = "updates/" + getDirectoryFromURL(mUpdateHost);
+
+    logger->log("Update host: %s", mUpdateHost.c_str());
+    logger->log("Updates dir: %s", mUpdatesDir.c_str());
 
     ResourceManager *resman = ResourceManager::getInstance();
 
     // Verify that the updates directory exists. Create if necessary.
-    if (!resman->isDirectory("/" + mUpdatesDir))
+    if (!resman->isDirectory(mUpdatesDir))
     {
-        if (!resman->mkdir("/" + mUpdatesDir))
+        if (!resman->mkdir(mUpdatesDir))
         {
 #if defined _WIN32
             std::string newDir = mLocalDataDir + "\\" + mUpdatesDir;
@@ -1165,9 +1160,12 @@ void Client::initUpdatesDir()
                 strprintf(_("Error creating updates directory!\n(%s/%s)"),
                             mLocalDataDir.c_str(), mUpdatesDir.c_str());
             mState = STATE_ERROR;
+            return false;
 #endif
         }
     }
+
+    return true;
 }
 
 void Client::initScreenshotDir()
diff --git a/src/client.h b/src/client.h
index abfa8d0a..2bf1c906 100644
--- a/src/client.h
+++ b/src/client.h
@@ -202,7 +202,7 @@ private:
     void initRootDir();
     void initHomeDir();
     void initConfiguration();
-    void initUpdatesDir();
+    bool initUpdatesDir();
     void initScreenshotDir();
 
     void accountLogin(LoginData *loginData);
diff --git a/src/gui/updaterwindow.h b/src/gui/updaterwindow.h
index 539783c5..65ee1d88 100644
--- a/src/gui/updaterwindow.h
+++ b/src/gui/updaterwindow.h
@@ -64,7 +64,7 @@ class UpdaterWindow : public Window, public gcn::ActionListener,
      * @param updateHost Host where to get the updated files.
      * @param updatesDir Directory where to store updates (should be absolute
      *                   and already created).
-     * @param applyUpdates If true, the update window will pass the updates to teh
+     * @param applyUpdates If true, the update window will pass the updates to the
      *                     resource manager
      */
     UpdaterWindow(const std::string &updateHost,
diff --git a/src/net/tmwa/loginhandler.cpp b/src/net/tmwa/loginhandler.cpp
index 24bc9ab2..069f9336 100644
--- a/src/net/tmwa/loginhandler.cpp
+++ b/src/net/tmwa/loginhandler.cpp
@@ -103,17 +103,15 @@ void LoginHandler::handleMessage(MessageIn &msg)
         }
             break;
 
-        case SMSG_UPDATE_HOST:
-             int len;
-
-             len = msg.readInt16() - 4;
+        case SMSG_UPDATE_HOST: {
+             int len = msg.readInt16() - 4;
              mUpdateHost = msg.readString(len);
              loginData.updateHost = mUpdateHost;
 
              logger->log("Received update host \"%s\" from login server.",
                      mUpdateHost.c_str());
              break;
-
+        }
         case SMSG_LOGIN_DATA:
             // Skip the length word
             msg.skip(2);
diff --git a/src/utils/stringutils.cpp b/src/utils/stringutils.cpp
index 747243fd..0244dd8c 100644
--- a/src/utils/stringutils.cpp
+++ b/src/utils/stringutils.cpp
@@ -101,16 +101,15 @@ std::string strprintf(char const *format, ...)
     return res;
 }
 
-std::string &removeBadChars(std::string &str)
+std::string &replaceCharacters(std::string &str,
+                               std::string_view chars,
+                               char replacement)
 {
-    std::string::size_type pos;
-    do
+    for (auto &c : str)
     {
-        pos = str.find_first_of("@#[]");
-        if (pos != std::string::npos)
-            str.erase(pos, 1);
-    } while (pos != std::string::npos);
-
+        if (chars.find(c) != std::string::npos)
+            c = replacement;
+    }
     return str;
 }
 
@@ -220,49 +219,27 @@ std::string normalize(const std::string &name)
     return toLower(trim(normalized));
 }
 
-std::string removeTrailingSymbol(const std::string &s, const char c)
+std::string getDirectoryFromURL(const std::string &url)
 {
-    // Remove the trailing symblol at the end of the string
-    if (!s.empty() && s.at(s.size() - 1) == c)
-        return s.substr(0, s.size() - 1);
-    return std::string(s);
-}
-
-std::string getHostNameFromURL(const std::string &url)
-{
-    std::string myHostName;
-
-    // Don't go out of range in the next check
-    if (url.length() < 2)
-        return myHostName;
+    std::string directory = url;
 
-    // Remove any trailing slash at the end of the update host
-    myHostName = removeTrailingSymbol(url, '/');
+    // Parse out any "http://", "ftp://", etc...
+    size_t pos = directory.find("://");
+    if (pos != std::string::npos)
+        directory.erase(0, pos + 3);
 
-    // Parse out any "http://", "ftp://", ect...
-    size_t pos = myHostName.find("://");
-    if (pos == std::string::npos)
-    {
-        logger->log("Warning: no protocol was specified for the url: %s",
-                    url.c_str());
-    }
+    // Replace characters which are not valid or difficult in file system paths
+    replaceCharacters(directory, ":*?\"<>| ", '_');
 
-    if (myHostName.empty() || pos + 3 >= myHostName.length())
+    // Replace ".." (double dots) with "_" to avoid directory traversal.
+    pos = directory.find("..");
+    while (pos != std::string::npos)
     {
-        logger->log("Error: Invalid url: %s", url.c_str());
-        return myHostName;
+        directory.replace(pos, 2, "_");
+        pos = directory.find("..");
     }
-    myHostName = myHostName.substr(pos + 3);
-
-    // Remove possible trailing port (i.e.: localhost:8000 -> localhost)
-    pos = myHostName.find(":");
-    if (pos != std::string::npos)
-        myHostName = myHostName.substr(0, pos);
-
-    // remove possible other junk
-    removeBadChars(myHostName);
 
-    return myHostName;
+    return directory;
 }
 
 std::string join(const std::vector<std::string> &strings, const char *separator)
diff --git a/src/utils/stringutils.h b/src/utils/stringutils.h
index ca54803d..349d2a66 100644
--- a/src/utils/stringutils.h
+++ b/src/utils/stringutils.h
@@ -95,12 +95,16 @@ std::string strprintf(char const *, ...)
 ;
 
 /**
- * Removes bad characters from a string
+ * Replaces a set of characters with another character.
  *
  * @param str the string to remove the bad chars from
+ * @param chars the bad characters to remove
+ * @param replacement the character to replace the bad chars with
  * @return a reference to the string without bad chars
  */
-std::string &removeBadChars(std::string &str);
+std::string &replaceCharacters(std::string &str,
+                               std::string_view chars,
+                               char replacement = '_');
 
 /**
  * Removes colors from a string
@@ -210,15 +214,12 @@ std::string autocomplete(const std::vector<std::string> &candidates,
 std::string normalize(const std::string &name);
 
 /**
- * Remove a potential trailing symbol from a string.
- */
-std::string removeTrailingSymbol(const std::string &s, const char c);
-
-/**
- * Gets the hostname out of the given URL.
- * i.e.: http://www.manasource.org:9601 -> www.manasource.org
+ * Derives a directory from the given URL, stripping the schema and replacing
+ * certain invalid characters.
+ *
+ * i.e.: http://www.manasource.org:9601/updates/ -> www.manasource.org_9601/updates/
  */
-std::string getHostNameFromURL(const std::string &url);
+std::string getDirectoryFromURL(const std::string &url);
 
 /**
  * Joins a vector of strings into one string, separated by the given