Redirection of slashless root URL

With non-empty root location, the canonic form of the root URL for a
kiwix server is now required to end with a slash (to match the situation
for an empty root location). This requirement enables usage of relative
URLs on the welcome page and resources/scripts loaded through that page.

A slashless root URL is redirected to the slashful version.

src/server/internalServer.cpp

@@ -97,14 +97,19 @@ inline std::string normalizeRootUrl(std::string rootUrl)
 std::string
 fullURL2LocalURL(const std::string& fullUrl, const std::string& rootLocation)
 {
-  assert(rootLocation.size() > 0 && rootLocation.back() == '/');
   if ( kiwix::startsWith(fullUrl, rootLocation) ) {
-    return fullUrl.substr(rootLocation.size() - 1);
+    return fullUrl.substr(rootLocation.size());
   } else {
-    return "";
+    return "INVALID URL";
   }
 }
 
+std::string getSearchComponent(const RequestContext& request)
+{
+    const std::string query = request.get_query();
+    return query.empty() ? query : "?" + query;
+}
+
 Filter get_search_filter(const RequestContext& request, const std::string& prefix="")
 {
     auto filter = kiwix::Filter().valid(true).local(true);
@@ -415,7 +420,7 @@ InternalServer::InternalServer(Library* library,
   m_addr(addr),
   m_port(port),
   m_root(normalizeRootUrl(root)),
-  m_rootPrefixOfDecodedURL(m_root + "/"),
+  m_rootPrefixOfDecodedURL(m_root),
   m_nbThreads(nbThreads),
   m_multizimSearchLimit(multizimSearchLimit),
   m_verbose(verbose),
@@ -585,6 +590,13 @@ std::unique_ptr<Response> InternalServer::handle_request(const RequestContext& r
              + urlNotFoundMsg;
     }
 
+    if ( request.get_url() == "" ) {
+      // Redirect /ROOT_LOCATION to /ROOT_LOCATION/ (note the added slash)
+      // so that relative URLs are resolved correctly
+      const std::string query = getSearchComponent(request);
+      return Response::build_redirect(*this, m_root + "/" + query);
+    }
+
     const ETag etag = get_matching_if_none_match_etag(request, getLibraryId());
     if ( etag )
       return Response::build_304(*this, etag);
@@ -623,11 +635,9 @@ std::unique_ptr<Response> InternalServer::handle_request(const RequestContext& r
     if (isEndpointUrl(url, "catch"))
       return handle_catch(request);
 
-    std::string contentUrl = m_root + "/content" + urlEncode(url);
+    const std::string contentUrl = m_root + "/content" + urlEncode(url);
-    const std::string query = request.get_query();
+    const std::string query = getSearchComponent(request);
-    if ( ! query.empty() )
+    return Response::build_redirect(*this, contentUrl + query);
-      contentUrl += "?" + query;
-    return Response::build_redirect(*this, contentUrl);
   } catch (std::exception& e) {
     fprintf(stderr, "===== Unhandled error : %s\n", e.what());
     return HTTP500Response(*this, request)

src/server/request_context.cpp

@@ -181,7 +181,7 @@ std::string RequestContext::get_root_path() const {
 }
 
 bool RequestContext::is_valid_url() const {
-  return !url.empty();
+  return url.empty() || url[0] == '/';
 }
 
 ByteRange RequestContext::get_range() const {

test/server.cpp

@@ -359,6 +359,10 @@ TEST_F(ServerTest, 400)
 const char* urls404[] = {
   "/",
   "/zimfile",
+  "/ROOT",
+  "/ROOT%23%",
+  "/ROOT%23%3",
+  "/ROOT%23%3Fxyz",
   "/ROOT%23%3F/skin/non-existent-skin-resource",
   "/ROOT%23%3F/skin/autoComplete.min.js?cacheid=wrongcacheid",
   "/ROOT%23%3F/catalog",
@@ -1268,6 +1272,36 @@ TEST_F(ServerTest, UserLanguageControl)
   }
 }
 
+TEST_F(ServerTest, SlashlessRootURLIsRedirectedToSlashfulURL)
+{
+  const std::pair<const char*, const char*> test_data[] = {
+    // URL                            redirect
+    { "/ROOT%23%3F",                  "/ROOT%23%3F/" },
+    { "/ROOT%23%3F?abcd=123&xyz=890", "/ROOT%23%3F/?abcd=123&xyz=890" }
+  };
+
+  for ( const auto& t : test_data )
+  {
+    const TestContext ctx{ {"url", t.first} };
+    const auto g = zfs1_->GET(t.first);
+    ASSERT_EQ(302, g->status) << ctx;
+    ASSERT_TRUE(g->has_header("Location")) << ctx;
+    ASSERT_EQ(g->get_header_value("Location"), t.second) << ctx;
+    ASSERT_EQ(getCacheControlHeader(*g), "max-age=0, must-revalidate") << ctx;
+    ASSERT_FALSE(g->has_header("ETag")) << ctx;
+  }
+}
+
+TEST_F(ServerTest, EmptyRootIsNotRedirected)
+{
+  ZimFileServer::Cfg serverCfg;
+  serverCfg.root = "";
+
+  resetServer(serverCfg);
+
+  ASSERT_EQ(200, zfs1_->GET("/")->status);
+}
+
 TEST_F(ServerTest, RandomPageRedirectsToAnExistingArticle)
 {
   auto g = zfs1_->GET("/ROOT%23%3F/random?content=zimfile");

test/server_testing_tools.h

@@ -68,10 +68,18 @@ public: // types
     DEFAULT_OPTIONS = WITH_TASKBAR | WITH_LIBRARY_BUTTON
   };
 
+  struct Cfg
+  {
+    std::string root = "ROOT#?";
+    Options options = DEFAULT_OPTIONS;
+
+    Cfg(Options opts = DEFAULT_OPTIONS) : options(opts) {}
+  };
+
 public: // functions
-  ZimFileServer(int serverPort, Options options, std::string libraryFilePath);
+  ZimFileServer(int serverPort, Cfg cfg, std::string libraryFilePath);
   ZimFileServer(int serverPort,
-                Options options,
+                Cfg cfg,
                 const FilePathCollection& zimpaths,
                 std::string indexTemplateString = "");
   ~ZimFileServer();
@@ -95,12 +103,12 @@ private: // data
   std::unique_ptr<kiwix::NameMapper> nameMapper;
   std::unique_ptr<kiwix::Server> server;
   std::unique_ptr<httplib::Client> client;
-  const Options options = DEFAULT_OPTIONS;
+  const Cfg cfg;
 };
 
-ZimFileServer::ZimFileServer(int serverPort, Options _options, std::string libraryFilePath)
+ZimFileServer::ZimFileServer(int serverPort, Cfg _cfg, std::string libraryFilePath)
 : manager(&this->library)
-, options(_options)
+, cfg(_cfg)
 {
   if ( kiwix::isRelativePath(libraryFilePath) )
     libraryFilePath = kiwix::computeAbsolutePath(kiwix::getCurrentDirectory(), libraryFilePath);
@@ -109,11 +117,11 @@ ZimFileServer::ZimFileServer(int serverPort, Options _options, std::string libra
 }
 
 ZimFileServer::ZimFileServer(int serverPort,
-                             Options _options,
+                             Cfg _cfg,
                              const FilePathCollection& zimpaths,
                              std::string indexTemplateString)
 : manager(&this->library)
-, options(_options)
+, cfg(_cfg)
 {
   for ( const auto& zimpath : zimpaths ) {
     if (!manager.addBookFromPath(zimpath, zimpath, "", false))
@@ -125,19 +133,19 @@ ZimFileServer::ZimFileServer(int serverPort,
 void ZimFileServer::run(int serverPort, std::string indexTemplateString)
 {
   const std::string address = "127.0.0.1";
-  if (options & NO_NAME_MAPPER) {
+  if (cfg.options & NO_NAME_MAPPER) {
     nameMapper.reset(new kiwix::IdNameMapper());
   } else {
     nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false));
   }
   server.reset(new kiwix::Server(&library, nameMapper.get()));
-  server->setRoot("ROOT#?");
+  server->setRoot(cfg.root);
   server->setAddress(address);
   server->setPort(serverPort);
   server->setNbThreads(2);
   server->setVerbose(false);
-  server->setTaskbar(options & WITH_TASKBAR, options & WITH_LIBRARY_BUTTON);
+  server->setTaskbar(cfg.options & WITH_TASKBAR, cfg.options & WITH_LIBRARY_BUTTON);
-  server->setBlockExternalLinks(options & BLOCK_EXTERNAL_LINKS);
+  server->setBlockExternalLinks(cfg.options & BLOCK_EXTERNAL_LINKS);
   server->setMultiZimSearchLimit(3);
   if (!indexTemplateString.empty()) {
     server->setIndexTemplateString(indexTemplateString);
@@ -171,9 +179,9 @@ protected:
     resetServer(ZimFileServer::DEFAULT_OPTIONS);
   }
 
-  void resetServer(ZimFileServer::Options options) {
+  void resetServer(ZimFileServer::Cfg cfg) {
     zfs1_.reset();
-    zfs1_.reset(new ZimFileServer(SERVER_PORT, options, ZIMFILES));
+    zfs1_.reset(new ZimFileServer(SERVER_PORT, cfg, ZIMFILES));
   }
 
   void TearDown() override {