Dropped the 2nd param of urlEncode()

`urlEncode(str)` is now equivalent to the previous `urlEncode(str, true)`.
kiwix · Jan 25, 2023 · 43d57b2 · 43d57b2
1 parent 77c35d1
commit 43d57b2
Show file tree

Hide file tree

Showing 9 changed files with 32 additions and 40 deletions.
diff --git a/src/opds_catalog.cpp b/src/opds_catalog.cpp
@@ -42,19 +42,19 @@ std::string buildSearchString(const Filter& f)
 {
   std::ostringstream oss;
   if ( f.hasQuery() )
-    oss << AMP << "q=" << urlEncode(f.getQuery(), true);
+    oss << AMP << "q=" << urlEncode(f.getQuery());
 
   if ( f.hasCategory() )
-    oss << AMP << "category=" << urlEncode(f.getCategory(), true);
+    oss << AMP << "category=" << urlEncode(f.getCategory());
 
   if ( f.hasLang() )
-    oss << AMP << "lang=" << urlEncode(f.getLang(), true);
+    oss << AMP << "lang=" << urlEncode(f.getLang());
 
   if ( f.hasName() )
-    oss << AMP << "name=" << urlEncode(f.getName(), true);
+    oss << AMP << "name=" << urlEncode(f.getName());
 
   if ( !f.getAcceptTags().empty() )
-    oss << AMP << "tag=" << urlEncode(join(f.getAcceptTags(), ";"), true);
+    oss << AMP << "tag=" << urlEncode(join(f.getAcceptTags(), ";"));
 
   return oss.str();
 }

diff --git a/src/opds_dumper.cpp b/src/opds_dumper.cpp
@@ -82,7 +82,7 @@ std::string fullEntryXML(const Book& book, const std::string& rootLocation, cons
       {"title", book.getTitle()},
       {"description", book.getDescription()},
       {"language", book.getLanguage()},
-      {"content_id",  urlEncode(contentId, true)},
+      {"content_id",  urlEncode(contentId)},
       {"updated", bookDate}, // XXX: this should be the entry update datetime
       {"book_date", bookDate},
       {"category", book.getCategory()},
@@ -241,7 +241,7 @@ std::string OPDSDumper::categoriesOPDSFeed() const
   const auto now = gen_date_str();
   kainjow::mustache::list categoryData;
   for ( const auto& category : library->getBooksCategories() ) {
-    const auto urlencodedCategoryName = urlEncode(category, true);
+    const auto urlencodedCategoryName = urlEncode(category);
     categoryData.push_back(kainjow::mustache::object{
       {"name", category},
       {"urlencoded_name",  urlencodedCategoryName},

diff --git a/src/search_renderer.cpp b/src/search_renderer.cpp
@@ -94,7 +94,7 @@ kainjow::mustache::data buildQueryData
   kainjow::mustache::data query;
   query.set("pattern", kiwix::encodeDiples(pattern));
   std::ostringstream ss;
-  ss << searchProtocolPrefix << "?pattern=" << urlEncode(pattern, true);
+  ss << searchProtocolPrefix << "?pattern=" << urlEncode(pattern);
   ss << "&" << bookQuery;
   query.set("unpaginatedQuery", ss.str());
   auto lang = extractValueFromQuery(bookQuery, "books.filter.lang");
@@ -174,7 +174,7 @@ std::string SearchRenderer::renderTemplate(const std::string& tmpl_str)
     const std::string zim_id(it.getZimId());
     const auto path = mp_nameMapper->getNameForId(zim_id) + "/" + it.getPath();
     result.set("title", it.getTitle());
-    result.set("absolutePath", absPathPrefix + urlEncode(path, true));
+    result.set("absolutePath", absPathPrefix + urlEncode(path));
     result.set("snippet", it.getSnippet());
     if (mp_library) {
       result.set("bookTitle", mp_library->getBookById(zim_id).getTitle());

diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp
@@ -1030,7 +1030,7 @@ ParameterizedMessage suggestSearchMsg(const std::string& searchURL, const std::s
 std::unique_ptr<Response>
 InternalServer::build_redirect(const std::string& bookName, const zim::Item& item) const
 {
-  const auto path = kiwix::urlEncode(item.getPath(), true);
+  const auto path = kiwix::urlEncode(item.getPath());
   const auto redirectUrl = m_root + "/content/" + bookName + "/" + path;
   return Response::build_redirect(*this, redirectUrl);
 }
@@ -1055,7 +1055,7 @@ std::unique_ptr<Response> InternalServer::handle_content(const RequestContext& r
   } catch (const std::out_of_range& e) {}
 
   if (archive == nullptr) {
-    const std::string searchURL = m_root + "/search?pattern=" + kiwix::urlEncode(pattern, true);
+    const std::string searchURL = m_root + "/search?pattern=" + kiwix::urlEncode(pattern);
     return HTTP404Response(*this, request)
            + urlNotFoundMsg
            + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern));
@@ -1096,7 +1096,7 @@ std::unique_ptr<Response> InternalServer::handle_content(const RequestContext& r
     if (m_verbose.load())
       printf("Failed to find %s\n", urlStr.c_str());
 
-    std::string searchURL = m_root + "/search?content=" + bookName + "&pattern=" + kiwix::urlEncode(pattern, true);
+    std::string searchURL = m_root + "/search?content=" + bookName + "&pattern=" + kiwix::urlEncode(pattern);
     return HTTP404Response(*this, request)
            + urlNotFoundMsg
            + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern));

diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp
@@ -115,10 +115,10 @@ MHD_Result RequestContext::fill_argument(void *__this, enum MHD_ValueKind kind,
   if ( ! _this->queryString.empty() ) {
     _this->queryString += "&";
   }
-  _this->queryString += urlEncode(key, true);
+  _this->queryString += urlEncode(key);
   if ( value ) {
     _this->queryString += "=";
-    _this->queryString += urlEncode(value, true);
+    _this->queryString += urlEncode(value);
   }
   return MHD_YES;
 }

diff --git a/src/server/request_context.h b/src/server/request_context.h
@@ -98,7 +98,7 @@ class RequestContext {
     std::string get_query(F filter, bool mustEncode) const {
       std::string q;
       const char* sep = "";
-      auto encode = [=](const std::string& value) { return mustEncode?urlEncode(value, true):value; };
+      auto encode = [=](const std::string& value) { return mustEncode?urlEncode(value):value; };
       for ( const auto& a : arguments ) {
         if (!filter(a.first)) {
           continue;

diff --git a/src/tools/stringTools.cpp b/src/tools/stringTools.cpp
@@ -164,9 +164,6 @@ std::string kiwix::encodeDiples(const std::string& str)
 namespace
 {
 
-/* urlEncode() based on javascript encodeURI() &
-   encodeURIComponent(). Mostly code from rstudio/httpuv (GPLv3) */
-
 bool isReservedUrlChar(char c)
 {
   switch (c) {
@@ -246,15 +243,15 @@ int hexToInt(char c) {
 
 } // unnamed namespace
 
-std::string kiwix::urlEncode(const std::string& value, bool encodeReserved)
+std::string kiwix::urlEncode(const std::string& value)
 {
   std::ostringstream os;
   os << std::hex << std::uppercase;
   for (std::string::const_iterator it = value.begin();
        it != value.end();
        it++) {
 
-    if (!needsEscape(*it, encodeReserved)) {
+    if (!needsEscape(*it, true)) {
       os << *it;
     } else {
       const unsigned int charVal = static_cast<unsigned char>(*it);

diff --git a/src/tools/stringTools.h b/src/tools/stringTools.h
@@ -55,7 +55,9 @@ class ICULanguageInfo
 };
 
 
-std::string urlEncode(const std::string& value, bool encodeReserved);
+/* urlEncode() is the equivalent of JS encodeURIComponent(), with the only
+ * difference that the slash (/) symbol is NOT encoded. */
+std::string urlEncode(const std::string& value);
 std::string urlDecode(const std::string& value, bool component = false);
 
 std::string join(const std::vector<std::string>& list, const std::string& sep);

diff --git a/test/stringTools.cpp b/test/stringTools.cpp
@@ -125,26 +125,19 @@ TEST(stringTools, urlEncode)
 {
   using namespace URLEncoding;
 
-  EXPECT_EQ(urlEncode(letters,false), letters);
-  EXPECT_EQ(urlEncode(letters, true), letters);
+  EXPECT_EQ(urlEncode(letters), letters);
 
-  EXPECT_EQ(urlEncode(digits,false), digits);
-  EXPECT_EQ(urlEncode(digits, true), digits);
+  EXPECT_EQ(urlEncode(digits), digits);
 
-  EXPECT_EQ(urlEncode(nonEncodableSymbols,false), nonEncodableSymbols);
-  EXPECT_EQ(urlEncode(nonEncodableSymbols, true), nonEncodableSymbols);
+  EXPECT_EQ(urlEncode(nonEncodableSymbols), nonEncodableSymbols);
 
-  EXPECT_EQ(urlEncode(uriDelimSymbols,false), uriDelimSymbols);
-  EXPECT_EQ(urlEncode(uriDelimSymbols, true), "%3A%40%3F%3D%2B%26%23%24%3B%2C");
+  EXPECT_EQ(urlEncode(uriDelimSymbols), "%3A%40%3F%3D%2B%26%23%24%3B%2C");
 
-  EXPECT_EQ(urlEncode(otherSymbols,false), "%60%25%5E%5B%5D%7B%7D%5C%7C%22%3C%3E");
-  EXPECT_EQ(urlEncode(otherSymbols,false), urlEncode(otherSymbols, true));
+  EXPECT_EQ(urlEncode(otherSymbols), "%60%25%5E%5B%5D%7B%7D%5C%7C%22%3C%3E");
 
-  EXPECT_EQ(urlEncode(whitespace,false), "%20%0A%09%0D");
-  EXPECT_EQ(urlEncode(whitespace,false), urlEncode(whitespace, true));
+  EXPECT_EQ(urlEncode(whitespace), "%20%0A%09%0D");
 
-  EXPECT_EQ(urlEncode(someNonASCIIChars,false), "%CE%A3%E2%99%82%E2%99%80%E3%83%84");
-  EXPECT_EQ(urlEncode(someNonASCIIChars,false), urlEncode(someNonASCIIChars, true));
+  EXPECT_EQ(urlEncode(someNonASCIIChars), "%CE%A3%E2%99%82%E2%99%80%E3%83%84");
 }
 
 TEST(stringTools, urlDecode)
@@ -161,13 +154,13 @@ TEST(stringTools, urlDecode)
 
   for ( const char c : allTestChars ) {
     const std::string str(1, c);
-    EXPECT_EQ(urlDecode(urlEncode(str,false),false), str);
-    EXPECT_EQ(urlDecode(urlEncode(str, true), true), str);
-    EXPECT_EQ(urlDecode(urlEncode(str,false)),       str);
+    EXPECT_EQ(urlDecode(urlEncode(str), true), str);
   }
 
-  EXPECT_EQ(urlDecode(urlEncode(allTestChars,false),false), allTestChars);
-  EXPECT_EQ(urlDecode(urlEncode(allTestChars, true), true), allTestChars);
+  EXPECT_EQ(urlDecode(urlEncode(allTestChars), true), allTestChars);
+
+  const std::string encodedUriDelimSymbols = urlEncode(uriDelimSymbols);
+  EXPECT_EQ(urlDecode(encodedUriDelimSymbols, false), encodedUriDelimSymbols);
 }
 
 };