diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 89b2efb13..76d1494f7 100644 --- a/src/server/internalServer.cpp +++ b/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 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 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 query = request.get_query(); - if ( ! query.empty() ) - contentUrl += "?" + query; - return Response::build_redirect(*this, contentUrl); + const std::string contentUrl = m_root + "/content" + urlEncode(url); + const std::string query = getSearchComponent(request); + return Response::build_redirect(*this, contentUrl + query); } catch (std::exception& e) { fprintf(stderr, "===== Unhandled error : %s\n", e.what()); return HTTP500Response(*this, request) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 017521666..9e0b965c0 100644 --- a/src/server/request_context.cpp +++ b/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 { diff --git a/test/server.cpp b/test/server.cpp index a1277a77d..94a3da5c9 100644 --- a/test/server.cpp +++ b/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 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"); diff --git a/test/server_testing_tools.h b/test/server_testing_tools.h index 6fa147588..777bb48e8 100644 --- a/test/server_testing_tools.h +++ b/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 nameMapper; std::unique_ptr server; std::unique_ptr 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->setBlockExternalLinks(options & BLOCK_EXTERNAL_LINKS); + server->setTaskbar(cfg.options & WITH_TASKBAR, cfg.options & WITH_LIBRARY_BUTTON); + 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 {