From 2e9bec95b05e84435ba5ca513a0197e928ed42fe Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 8 Feb 2023 15:31:29 +0100 Subject: [PATCH] Proper URI-encoding in InternalServer::build_redirect() - Before this change `InternalServer::build_redirect()` only URI-encoded the article path, ignoring the book name and/or the root location components of the URL. - In order to be able to test this fix, corner_cases.zim was renamed to contain a couple of special URL symbols in its filename. The `create_corner_cases_zim_file` script was updated accordingly. --- src/server/internalServer.cpp | 5 ++-- .../{corner_cases.zim => corner_cases#&.zim} | Bin test/data/create_corner_cases_zim_file | 25 +++++++++++++++--- test/meson.build | 2 +- test/server.cpp | 16 +++++------ test/server_search.cpp | 2 +- test/server_testing_tools.h | 2 +- 7 files changed, 34 insertions(+), 18 deletions(-) rename test/data/{corner_cases.zim => corner_cases#&.zim} (100%) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 8548d4245..bef72d64b 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -1030,9 +1030,8 @@ ParameterizedMessage suggestSearchMsg(const std::string& searchURL, const std::s std::unique_ptr InternalServer::build_redirect(const std::string& bookName, const zim::Item& item) const { - const auto path = kiwix::urlEncode(item.getPath()); - const auto redirectUrl = m_root + "/content/" + bookName + "/" + path; - return Response::build_redirect(*this, redirectUrl); + const auto absPath = m_root + "/content/" + bookName + "/" + item.getPath(); + return Response::build_redirect(*this, kiwix::urlEncode(absPath)); } std::unique_ptr InternalServer::handle_content(const RequestContext& request) diff --git a/test/data/corner_cases.zim b/test/data/corner_cases#&.zim similarity index 100% rename from test/data/corner_cases.zim rename to test/data/corner_cases#&.zim diff --git a/test/data/create_corner_cases_zim_file b/test/data/create_corner_cases_zim_file index 5f11096a2..32615cad9 100755 --- a/test/data/create_corner_cases_zim_file +++ b/test/data/create_corner_cases_zim_file @@ -1,7 +1,24 @@ #!/usr/bin/env bash cd "$(dirname "$0")" -rm -f corner_cases.zim + +# The following symbols (that would be nice to include in testing) are not +# allowed under NTFS and/or FAT32 filesystems, and would result in the +# impossibility to git clone (or rather checkout) the libkiwix repository under +# Windows: +# +# ? +# = +# + (that's a pity, since the + symbol in a ZIM filename is replaced with the +# text 'plus' when the ZIM file is added to kiwix-serve's library and it +# would be nice to test that functionality) +# +# Assuming that tests are NOT run under Windows, above symbols can be included +# in testing if the file is renamed while copying to the build directory (see +# test/meson.build), though that would make maintenance slightly more confusing. +zimfilename='corner_cases#&.zim' + +rm -f "$zimfilename" zimwriterfs --withoutFTIndex --dont-check-arguments \ -w empty.html \ -I empty.png \ @@ -11,6 +28,6 @@ zimwriterfs --withoutFTIndex --dont-check-arguments \ -c "" \ -p "" \ corner_cases \ - corner_cases.zim \ -&& echo 'corner_cases.zim was successfully created' \ -|| echo '!!! Failed to create corner_cases.zim !!!' >&2 + "$zimfilename" \ +&& echo "$zimfilename was successfully created" \ +|| echo '!!! Failed to create' "$zimfilename" '!!!' >&2 diff --git a/test/meson.build b/test/meson.build index 772afda18..f7ed51f5d 100644 --- a/test/meson.build +++ b/test/meson.build @@ -34,7 +34,7 @@ if gtest_dep.found() and not meson.is_cross_build() 'example.zim', 'zimfile.zim', 'zimfile&other.zim', - 'corner_cases.zim', + 'corner_cases#&.zim', 'poor.zim', 'library.xml', 'lib_for_server_search_test.xml', diff --git a/test/server.cpp b/test/server.cpp index 5a224343a..3ac6e6fff 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -166,9 +166,9 @@ const ResourceCollection resources200Uncompressible{ { ZIM_CONTENT, "/ROOT/content/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg" }, - { ZIM_CONTENT, "/ROOT/content/corner_cases/empty.html" }, - { ZIM_CONTENT, "/ROOT/content/corner_cases/empty.css" }, - { ZIM_CONTENT, "/ROOT/content/corner_cases/empty.js" }, + { ZIM_CONTENT, "/ROOT/content/corner_cases%23%26/empty.html" }, + { ZIM_CONTENT, "/ROOT/content/corner_cases%23%26/empty.css" }, + { ZIM_CONTENT, "/ROOT/content/corner_cases%23%26/empty.js" }, // The following url's responses are too small to be compressed @@ -188,7 +188,7 @@ TEST(indexTemplateStringTest, emptyIndexTemplate) { const int PORT = 8001; const ZimFileServer::FilePathCollection ZIMFILES { "./test/zimfile.zim", - "./test/corner_cases.zim" + "./test/corner_cases#&.zim" }; ZimFileServer zfs(PORT, ZimFileServer::DEFAULT_OPTIONS, ZIMFILES, ""); @@ -199,7 +199,7 @@ TEST(indexTemplateStringTest, indexTemplateCheck) { const int PORT = 8001; const ZimFileServer::FilePathCollection ZIMFILES { "./test/zimfile.zim", - "./test/corner_cases.zim" + "./test/corner_cases#&.zim" }; ZimFileServer zfs(PORT, ZimFileServer::DEFAULT_OPTIONS, ZIMFILES, "" @@ -1291,10 +1291,10 @@ TEST_F(ServerTest, NonEndpointUrlsAreRedirectedToContentUrls) TEST_F(ServerTest, RedirectionsToURLsWithSpecialSymbols) { - auto g = zfs1_->GET("/ROOT/content/corner_cases/c_sharp.html"); + auto g = zfs1_->GET("/ROOT/content/corner_cases%23%26/c_sharp.html"); ASSERT_EQ(302, g->status); ASSERT_TRUE(g->has_header("Location")); - ASSERT_EQ(g->get_header_value("Location"), "/ROOT/content/corner_cases/c%23.html"); + ASSERT_EQ(g->get_header_value("Location"), "/ROOT/content/corner_cases%23%26/c%23.html"); ASSERT_EQ(getCacheControlHeader(*g), "max-age=0, must-revalidate"); ASSERT_FALSE(g->has_header("ETag")); } @@ -1611,7 +1611,7 @@ TEST_F(ServerTest, InvalidAndMultiRangeByteRangeRequestsResultIn416Responses) TEST_F(ServerTest, ValidByteRangeRequestsOfZeroSizedEntriesResultIn416Responses) { - const char url[] = "/ROOT/content/corner_cases/empty.js"; + const char url[] = "/ROOT/content/corner_cases%23%26/empty.js"; const char* ranges[] = { "bytes=0-", diff --git a/test/server_search.cpp b/test/server_search.cpp index e8af5f348..c0146a573 100644 --- a/test/server_search.cpp +++ b/test/server_search.cpp @@ -1517,7 +1517,7 @@ TEST(ServerSearchTest, searchInMultilanguageBookSetIsDenied) const ZimFileServer::FilePathCollection ZIMFILES{ "./test/zimfile.zim", // eng "./test/example.zim", // en - "./test/corner_cases.zim" // =en + "./test/corner_cases#&.zim" // =en }; ZimFileServer zfs(SERVER_PORT, ZimFileServer::DEFAULT_OPTIONS, ZIMFILES); diff --git a/test/server_testing_tools.h b/test/server_testing_tools.h index 7af5834e6..463c863fc 100644 --- a/test/server_testing_tools.h +++ b/test/server_testing_tools.h @@ -163,7 +163,7 @@ protected: "./test/zimfile.zim", "./test/example.zim", "./test/poor.zim", - "./test/corner_cases.zim" + "./test/corner_cases#&.zim" }; protected: