lib/src/HttpClientImpl.h

@@ -110,7 +110,7 @@ class HttpClientImpl final : public HttpClient,
     void addSSLConfigs(const std::vector<std::pair<std::string, std::string>>
                            &sslConfCmds) override;
 
-    void setSockOptCallback(std::function<void(int)> cb) override
+    void setSockOptCallback(std::function<void(int)> cb)
     {
         sockOptCallback_ = std::move(cb);
     }

lib/src/SlashRemover.cc

@@ -2,17 +2,16 @@
 #include <drogon/plugins/Redirector.h>
 #include <drogon/HttpAppFramework.h>
 #include "drogon/utils/FunctionTraits.h"
-#include <cstddef>
-#include <cstdint>
 #include <functional>
 #include <string>
-#include <string_view>
-#include <utility>
+#include <regex>
 
 using namespace drogon;
 using namespace drogon::plugin;
 using std::string;
-using std::string_view;
+
+#define TRAILING_SLASH_REGEX ".+\\/$"
+#define DUPLICATE_SLASH_REGEX ".*\\/{2,}.*"
 
 enum removeSlashMode : uint8_t
 {
@@ -21,177 +20,76 @@ enum removeSlashMode : uint8_t
     both = trailing | duplicate,
 };
 
-/// Returns the index before the trailing slashes,
-/// or 0 if only contains slashes
-static inline size_t findTrailingSlashes(string_view url)
-{
-    auto len = url.size();
-    // Must be at least 2 chars and end with a slash
-    if (len < 2 || url.back() != '/')
-        return string::npos;
+inline constexpr const char* regexes[] = {
+    TRAILING_SLASH_REGEX,
+    DUPLICATE_SLASH_REGEX,
+    TRAILING_SLASH_REGEX "|" DUPLICATE_SLASH_REGEX,
+};
 
-    size_t a = len - 1;  // We already know the last char is '/',
-                         // we will use pre-decrement to account for this
-    while (--a > 0 && url[a] == '/')
-        ;  // We know the first char is '/', so don't check for 0
-    return a;
+static inline bool removeTrailingSlashes(string& url)
+{
+    const auto notSlashIndex = url.find_last_not_of('/');
+    if (notSlashIndex == string::npos)  // Root
+    {
+        url.resize(1);
+        return true;
+    }
+    url.resize(notSlashIndex + 1);
+    return false;
 }
 
-static inline void removeTrailingSlashes(string& url,
-                                         size_t start,
-                                         string_view originalUrl)
+static inline void removeDuplicateSlashes(string& url)
 {
-    url = originalUrl.substr(0, start + 1);
-}
-
-/// Returns the index of the 2nd duplicate slash
-static inline size_t findDuplicateSlashes(string_view url)
-{
-    size_t len = url.size();
-    if (len < 2)
-        return string::npos;
-
-    bool startedPair = true;  // Always starts with a slash
-    for (size_t a = 1; a < len; ++a)
-    {
-        if (url[a] != '/')  // Broken pair
-        {
-            startedPair = false;
-            continue;
-        }
-        if (startedPair)  // Matching pair
-            return a;
-        startedPair = true;
-    }
-
-    return string::npos;
-}
-
-static inline void removeDuplicateSlashes(string& url, size_t start)
-{
-    // +1 because we don't need to look at the same character again,
-    // which was found by `findDuplicateSlashes`, it saves one iteration
-    for (size_t b = (start--) + 1, len = url.size(); b < len; ++b)
+    size_t a = 1, len = url.size();
+    for (; a < len && (url[a - 1] != '/' || url[a] != '/'); ++a)
+        ;
+    for (size_t b = a--; b < len; ++b)
     {
         const char c = url[b];
-        if (c != '/' || url[start] != '/')
+        if (c != '/' || url[a] != '/')
         {
-            ++start;
-            url[start] = c;
+            ++a;
+            url[a] = c;
         }
     }
-    url.resize(start + 1);
+    url.resize(a + 1);
 }
 
-static inline std::pair<size_t, size_t> findExcessiveSlashes(string_view url)
+static inline void removeExcessiveSlashes(string& url)
 {
-    size_t len = url.size();
-    if (len < 2)  // Must have at least 2 characters to count as either trailing
-                  // or duplicate slash
-        return {string::npos, string::npos};
+    if (url.back() == '/' &&  // This check is so we don't search if there is no
+                              // trailing slash to begin with
+        removeTrailingSlashes(
+            url))  // If it is root path, we don't need to check for duplicates
+        return;
 
-    // Trail finder
-    size_t trailIdx = len;  // The pre-decrement will put it on last char
-    while (--trailIdx > 0 && url[trailIdx] == '/')
-        ;  // We know first char is '/', no need to check it
-
-    // Filled with '/'
-    if (trailIdx == 0)
-        return {
-            0,             // Only keep first slash
-            string::npos,  // No duplicate
-        };
-
-    // Look for a duplicate pair
-    size_t dupIdx = 1;
-    for (bool startedPair = true; dupIdx < trailIdx;
-         ++dupIdx)  // Always starts with a slash
-    {
-        if (url[dupIdx] != '/')  // Broken pair
-        {
-            startedPair = false;
-            continue;
-        }
-        if (startedPair)  // Matching pair
-            break;
-        startedPair = true;
+    removeDuplicateSlashes(url);
 }
 
-    // Found no duplicate
-    if (dupIdx == trailIdx)
-        return {
-            trailIdx != len - 1
-                ?  // If has gone past last char, then there is a trailing slash
-                trailIdx
-                : string::npos,  // No trail
-            string::npos,        // No duplicate
-        };
-
-    // Duplicate found
-    return {
-        trailIdx != len - 1
-            ?  // If has gone past last char, then there is a trailing slash
-            trailIdx
-            : string::npos,  // No trail
-        dupIdx,
-    };
-}
-
-static inline void removeExcessiveSlashes(string& url,
-                                          std::pair<size_t, size_t> start,
-                                          string_view originalUrl)
+static inline bool handleReq(const drogon::HttpRequestPtr& req, int removeMode)
 {
-    if (start.first != string::npos)
-        removeTrailingSlashes(url, start.first, originalUrl);
-    else
-        url = originalUrl;
-
-    if (start.second != string::npos)
-        removeDuplicateSlashes(url, start.second);
-}
-
-static inline bool handleReq(const drogon::HttpRequestPtr& req,
-                             uint8_t removeMode)
+    static const std::regex regex(regexes[removeMode - 1]);
+    if (std::regex_match(req->path(), regex))
     {
+        string newPath = req->path();
         switch (removeMode)
         {
             case trailing:
-        {
-            auto find = findTrailingSlashes(req->path());
-            if (find == string::npos)
-                return false;
-
-            string newPath;
-            removeTrailingSlashes(newPath, find, req->path());
-            req->setPath(std::move(newPath));
+                removeTrailingSlashes(newPath);
                 break;
-        }
             case duplicate:
-        {
-            auto find = findDuplicateSlashes(req->path());
-            if (find == string::npos)
-                return false;
-
-            string newPath = req->path();
-            removeDuplicateSlashes(newPath, find);
-            req->setPath(std::move(newPath));
+                removeDuplicateSlashes(newPath);
                 break;
-        }
             case both:
             default:
-        {
-            auto find = findExcessiveSlashes(req->path());
-            if (find.first == string::npos && find.second == string::npos)
-                return false;
-
-            string newPath;
-            removeExcessiveSlashes(newPath, find, req->path());
-            req->setPath(std::move(newPath));
+                removeExcessiveSlashes(newPath);
                 break;
         }
-    }
+        req->setPath(std::move(newPath));
         return true;
     }
+    return false;
+}
 
 void SlashRemover::initAndStart(const Json::Value& config)
 {

lib/tests/unittests/SlashRemoverTest.cc

@@ -1,179 +1,43 @@
 #include <drogon/drogon_test.h>
 #include <../../lib/src/SlashRemover.cc>
-#include <string>
 
 using std::string;
 
 DROGON_TEST(SlashRemoverTest)
 {
-    string cleanUrl;
+    const string url = "///home//page//", urlNoTrail = "///home//page",
+                 urlNoDup = "/home/page/", urlNoExcess = "/home/page",
+                 root = "///", rootNoExcess = "/";
 
-    {  // Regular URL
-        const string urlNoTrail = "///home//page", urlNoDup = "/home/page/",
-                     urlNoExcess = "/home/page";
-
-        SUBSECTION(Full)
-        {
-            const string url = "///home//page//";
-            removeTrailingSlashes(cleanUrl, findTrailingSlashes(url), url);
+    string cleanUrl = url;
+    removeTrailingSlashes(cleanUrl);
     CHECK(cleanUrl == urlNoTrail);
 
     cleanUrl = url;
-            removeDuplicateSlashes(cleanUrl, findDuplicateSlashes(url));
+    removeDuplicateSlashes(cleanUrl);
     CHECK(cleanUrl == urlNoDup);
 
-            removeExcessiveSlashes(cleanUrl, findExcessiveSlashes(url), url);
-            CHECK(cleanUrl == urlNoExcess);
-        }
-
-        SUBSECTION(Partial)
-        {
-            removeExcessiveSlashes(cleanUrl,
-                                   findExcessiveSlashes(urlNoTrail),
-                                   urlNoTrail);
-            CHECK(cleanUrl == urlNoExcess);
-
-            removeExcessiveSlashes(cleanUrl,
-                                   findExcessiveSlashes(urlNoDup),
-                                   urlNoDup);
-            CHECK(cleanUrl == urlNoExcess);
-        }
-    }
-
-    SUBSECTION(Root)
-    {
-        const string urlNoExcess = "/";
-
-        {  // Overlapping indices
-            const string url = "//";
-
     cleanUrl = url;
-            removeTrailingSlashes(cleanUrl, findTrailingSlashes(url), url);
+    removeExcessiveSlashes(cleanUrl);
     CHECK(cleanUrl == urlNoExcess);
 
-            cleanUrl = url;
-            removeDuplicateSlashes(cleanUrl, findDuplicateSlashes(url));
+    cleanUrl = urlNoTrail;
+    removeExcessiveSlashes(cleanUrl);
     CHECK(cleanUrl == urlNoExcess);
 
-            removeExcessiveSlashes(cleanUrl, findExcessiveSlashes(url), url);
-            CHECK(cleanUrl == urlNoExcess);
-        }
-
-        {  // Intersecting indices
-            const string url = "///";
-
-            cleanUrl = url;
-            removeTrailingSlashes(cleanUrl, findTrailingSlashes(url), url);
+    cleanUrl = urlNoDup;
+    removeExcessiveSlashes(cleanUrl);
     CHECK(cleanUrl == urlNoExcess);
 
-            cleanUrl = url;
-            removeDuplicateSlashes(cleanUrl, findDuplicateSlashes(url));
-            CHECK(cleanUrl == urlNoExcess);
+    cleanUrl = root;
+    removeTrailingSlashes(cleanUrl);
+    CHECK(cleanUrl == rootNoExcess);
 
-            removeExcessiveSlashes(cleanUrl, findExcessiveSlashes(url), url);
-            CHECK(cleanUrl == urlNoExcess);
-        }
-    }
-
-    SUBSECTION(Overlap)
-    {
-        const string urlNoDup = "/a/", urlNoExcess = "/a";
-
-        {  // Overlapping indices
-            const string url = "/a//";
-
-            removeTrailingSlashes(cleanUrl, findTrailingSlashes(url), url);
-            CHECK(cleanUrl == urlNoExcess);
-
-            cleanUrl = url;
-            removeDuplicateSlashes(cleanUrl, findDuplicateSlashes(url));
-            CHECK(cleanUrl == urlNoDup);
-
-            removeExcessiveSlashes(cleanUrl, findExcessiveSlashes(url), url);
-            CHECK(cleanUrl == urlNoExcess);
-        }
-
-        {  // Intersecting indices
-            const string url = "/a///";
-
-            removeTrailingSlashes(cleanUrl, findTrailingSlashes(url), url);
-            CHECK(cleanUrl == urlNoExcess);
-
-            cleanUrl = url;
-            removeDuplicateSlashes(cleanUrl, findDuplicateSlashes(url));
-            CHECK(cleanUrl == urlNoDup);
-
-            removeExcessiveSlashes(cleanUrl, findExcessiveSlashes(url), url);
-            CHECK(cleanUrl == urlNoExcess);
-        }
-    }
-
-    SUBSECTION(NoTrail)
-    {
-        const string url = "//a", urlNoExcess = "/a";
-
-        cleanUrl.clear();
-        {
-            auto find = findTrailingSlashes(url);
-            if (find != string::npos)
-                removeTrailingSlashes(cleanUrl, find, url);
-        }
-        CHECK(cleanUrl.empty());
-
-        cleanUrl = url;
-        removeDuplicateSlashes(cleanUrl, findDuplicateSlashes(url));
-        CHECK(cleanUrl == urlNoExcess);
-
-        removeExcessiveSlashes(cleanUrl, findExcessiveSlashes(url), url);
-        CHECK(cleanUrl == urlNoExcess);
-    }
-
-    SUBSECTION(NoDuplicate)
-    {
-        const string url = "/a/", urlNoExcess = "/a";
-
-        removeTrailingSlashes(cleanUrl, findTrailingSlashes(url), url);
-        CHECK(cleanUrl == urlNoExcess);
-
-        cleanUrl = url;
-        {
-            auto find = findDuplicateSlashes(cleanUrl);
-            if (find != string::npos)
-                removeDuplicateSlashes(cleanUrl, find);
-        }
-        CHECK(cleanUrl == url);
-
-        cleanUrl = url;
-        removeExcessiveSlashes(cleanUrl, findExcessiveSlashes(url), url);
-        CHECK(cleanUrl == urlNoExcess);
-    }
-
-    SUBSECTION(None)
-    {
-        const string url = "/a";
-
-        cleanUrl.clear();
-        {
-            auto find = findTrailingSlashes(url);
-            if (find != string::npos)
-                removeTrailingSlashes(cleanUrl, find, url);
-        }
-        CHECK(cleanUrl.empty());
-
-        cleanUrl = url;
-        {
-            auto find = findDuplicateSlashes(cleanUrl);
-            if (find != string::npos)
-                removeDuplicateSlashes(cleanUrl, find);
-        }
-        CHECK(cleanUrl == url);
-
-        cleanUrl.clear();
-        {
-            auto find = findExcessiveSlashes(url);
-            if (find.first != string::npos || find.second != string::npos)
-                removeExcessiveSlashes(cleanUrl, find, url);
-        }
-        CHECK(cleanUrl.empty());
-    }
+    cleanUrl = root;
+    removeDuplicateSlashes(cleanUrl);
+    CHECK(cleanUrl == rootNoExcess);
+
+    cleanUrl = root;
+    removeExcessiveSlashes(cleanUrl);
+    CHECK(cleanUrl == rootNoExcess);
 }