From 3f0ea083e65c33e9ca1c6d2039a634e1226aa0d9 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 5 Dec 2023 16:32:45 +0400 Subject: [PATCH 01/28] Moved microhttpd_wrapper.h under server/ --- src/{ => server}/microhttpd_wrapper.h | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/{ => server}/microhttpd_wrapper.h (100%) diff --git a/src/microhttpd_wrapper.h b/src/server/microhttpd_wrapper.h similarity index 100% rename from src/microhttpd_wrapper.h rename to src/server/microhttpd_wrapper.h From 96b6f41244497864104431b72e49e799d116c41f Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 7 Jan 2024 16:47:01 +0400 Subject: [PATCH 02/28] Added i18n unit test --- test/i18n.cpp | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ test/meson.build | 3 ++- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 test/i18n.cpp diff --git a/test/i18n.cpp b/test/i18n.cpp new file mode 100644 index 000000000..4e891d6e9 --- /dev/null +++ b/test/i18n.cpp @@ -0,0 +1,50 @@ +#include "../src/server/i18n.h" +#include "gtest/gtest.h" + +using namespace kiwix; + +TEST(ParameterizedMessage, parameterlessMessages) +{ + { + const ParameterizedMessage msg("404-page-title", {}); + + EXPECT_EQ(msg.getText("en"), "Content not found"); + EXPECT_EQ(msg.getText("test"), "[I18N TESTING] Not Found - Try Again"); + } + + { + // Make sure that msgId influences the result of getText() + const ParameterizedMessage msg("random-page-button-text", {}); + + EXPECT_EQ(msg.getText("en"), "Go to a randomly selected page"); + EXPECT_EQ(msg.getText("test"), "[I18N TESTING] I am tired of determinism"); + } + + { + // Demonstrate that unwanted parameters are silently ignored + const ParameterizedMessage msg("404-page-title", {{"abc", "xyz"}}); + + EXPECT_EQ(msg.getText("en"), "Content not found"); + EXPECT_EQ(msg.getText("test"), "[I18N TESTING] Not Found - Try Again"); + } +} + +TEST(ParameterizedMessage, messagesWithParameters) +{ + { + const ParameterizedMessage msg("filter-by-tag", + {{"TAG", "scifi"}} + ); + + EXPECT_EQ(msg.getText("en"), "Filter by tag \"scifi\""); + EXPECT_EQ(msg.getText("test"), "Filter [I18N] by [TESTING] tag \"scifi\""); + } + + { + // Omitting expected parameters amounts to using empty values for them + const ParameterizedMessage msg("filter-by-tag", {}); + + EXPECT_EQ(msg.getText("en"), "Filter by tag \"\""); + EXPECT_EQ(msg.getText("test"), "Filter [I18N] by [TESTING] tag \"\""); + } +} diff --git a/test/meson.build b/test/meson.build index 72d7b7331..5ed11246c 100644 --- a/test/meson.build +++ b/test/meson.build @@ -13,7 +13,8 @@ tests = [ 'name_mapper', 'opds_catalog', 'server_helper', - 'lrucache' + 'lrucache', + 'i18n' ] if build_machine.system() != 'windows' From 8993f99587ab02648da443d2457072942d625e7c Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 7 Jan 2024 16:59:13 +0400 Subject: [PATCH 03/28] ParameterizedMessage is actually a class --- src/server/i18n.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/i18n.h b/src/server/i18n.h index 79005b721..36d4b955b 100644 --- a/src/server/i18n.h +++ b/src/server/i18n.h @@ -93,7 +93,7 @@ class GetTranslatedStringWithMsgId } // namespace i18n -struct ParameterizedMessage +class ParameterizedMessage { public: // types typedef kainjow::mustache::object Parameters; From b9323f17bbef027d0bcf0d0fd54b4b1e91efdc47 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 5 Dec 2023 16:34:09 +0400 Subject: [PATCH 04/28] Introduced testing of HTTP response utils --- src/server/request_context.h | 2 +- src/server/response.h | 5 ++++ test/meson.build | 3 ++- test/response.cpp | 46 ++++++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 test/response.cpp diff --git a/src/server/request_context.h b/src/server/request_context.h index d5ab7b515..80d67ea6f 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -29,7 +29,7 @@ #include #include "byte_range.h" -#include "tools/stringTools.h" +#include "../tools/stringTools.h" extern "C" { #include "microhttpd_wrapper.h" diff --git a/src/server/response.h b/src/server/response.h index b1636fa9c..643f67f88 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -101,6 +101,9 @@ class ContentResponse : public Response { kainjow::mustache::data data, const std::string& mimetype); + const std::string& getContent() const { return m_content; } + const std::string& getMimeType() const { return m_mimeType; } + private: MHD_Response* create_mhd_response(const RequestContext& request); @@ -135,6 +138,8 @@ class ContentResponseBlueprint protected: // functions std::string getMessage(const std::string& msgId) const; + +public: virtual std::unique_ptr generateResponseObject() const; public: //data diff --git a/test/meson.build b/test/meson.build index 5ed11246c..78446f473 100644 --- a/test/meson.build +++ b/test/meson.build @@ -14,7 +14,8 @@ tests = [ 'opds_catalog', 'server_helper', 'lrucache', - 'i18n' + 'i18n', + 'response' ] if build_machine.system() != 'windows' diff --git a/test/response.cpp b/test/response.cpp new file mode 100644 index 000000000..5d5efe649 --- /dev/null +++ b/test/response.cpp @@ -0,0 +1,46 @@ +#include "../src/server/response.h" +#include "gtest/gtest.h" + +#include "../src/server/request_context.h" + +namespace +{ + +using namespace kiwix; + +RequestContext makeHttpGetRequest(const std::string& url) +{ + return RequestContext(nullptr, "", url, "GET", "1.1"); +} + +std::string getResponseContent(const ContentResponseBlueprint& crb) +{ + return crb.generateResponseObject()->getContent(); +} + +} // unnamed namespace + + + +TEST(HTTPErrorResponse, shouldBeInEnglishByDefault) { + const RequestContext req = makeHttpGetRequest("/asdf"); + HTTPErrorResponse errResp(req, MHD_HTTP_NOT_FOUND, + "404-page-title", + "404-page-heading", + "/css/error.css"); + + EXPECT_EQ(getResponseContent(errResp), +R"( + + + + Content not found + + + +

Not Found

+ + + +)"); +} From af228bf45f8f35340b4e2b565537bc210006b0ec Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 5 Dec 2023 17:13:28 +0400 Subject: [PATCH 05/28] Dropped cookies from RequestContext This should have been done in PR#997 in order to better guarantee a lasting solution to issue#995. --- src/server/request_context.cpp | 9 --------- src/server/request_context.h | 2 -- 2 files changed, 11 deletions(-) diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 91ff7a72d..2b5b6fd4c 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -66,7 +66,6 @@ RequestContext::RequestContext(struct MHD_Connection* connection, { MHD_get_connection_values(connection, MHD_HEADER_KIND, &RequestContext::fill_header, this); MHD_get_connection_values(connection, MHD_GET_ARGUMENT_KIND, &RequestContext::fill_argument, this); - MHD_get_connection_values(connection, MHD_COOKIE_KIND, &RequestContext::fill_cookie, this); try { acceptEncodingGzip = @@ -107,14 +106,6 @@ MHD_Result RequestContext::fill_argument(void *__this, enum MHD_ValueKind kind, return MHD_YES; } -MHD_Result RequestContext::fill_cookie(void *__this, enum MHD_ValueKind kind, - const char *key, const char* value) -{ - RequestContext *_this = static_cast(__this); - _this->cookies[key] = value == nullptr ? "" : value; - return MHD_YES; -} - void RequestContext::print_debug_info() const { printf("method : %s (%d)\n", method==RequestMethod::GET ? "GET" : method==RequestMethod::POST ? "POST" : diff --git a/src/server/request_context.h b/src/server/request_context.h index 80d67ea6f..0393dce77 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -145,7 +145,6 @@ class RequestContext { ByteRange byteRange_; std::map headers; std::map> arguments; - std::map cookies; std::string queryString; UserLanguage userlang; @@ -153,7 +152,6 @@ class RequestContext { UserLanguage determine_user_language() const; static MHD_Result fill_header(void *, enum MHD_ValueKind, const char*, const char*); - static MHD_Result fill_cookie(void *, enum MHD_ValueKind, const char*, const char*); static MHD_Result fill_argument(void *, enum MHD_ValueKind, const char*, const char*); }; From aee6c23082535009a60e3c2151f4551ed318e905 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 5 Dec 2023 17:48:10 +0400 Subject: [PATCH 06/28] Decoupled RequestContext from MHD_Connection This will simplify testing of Response utilities. --- src/server/internalServer.cpp | 18 +++++++++++++++++- src/server/request_context.cpp | 29 +++++++++++++++-------------- src/server/request_context.h | 15 ++++++++++----- test/response.cpp | 8 +++++--- 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 72b71b044..dcf6f170b 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -513,6 +513,19 @@ static MHD_Result staticHandlerCallback(void* cls, cont_cls); } +namespace +{ + +MHD_Result add_name_value_pair(void *nvp, enum MHD_ValueKind kind, + const char *key, const char *value) +{ + auto& nameValuePairs = *reinterpret_cast(nvp); + nameValuePairs.push_back({key, value}); + return MHD_YES; +} + +} // unnamed namespace + MHD_Result InternalServer::handlerCallback(struct MHD_Connection* connection, const char* fullUrl, const char* method, @@ -529,7 +542,10 @@ MHD_Result InternalServer::handlerCallback(struct MHD_Connection* connection, } const auto url = fullURL2LocalURL(fullUrl, m_rootPrefixOfDecodedURL); - RequestContext request(connection, m_root, url, method, version); + RequestContext::NameValuePairs headers, queryArgs; + MHD_get_connection_values(connection, MHD_HEADER_KIND, add_name_value_pair, &headers); + MHD_get_connection_values(connection, MHD_GET_ARGUMENT_KIND, add_name_value_pair, &queryArgs); + RequestContext request(m_root, url, method, version, headers, queryArgs); if (m_verbose.load() ) { request.print_debug_info(); diff --git a/src/server/request_context.cpp b/src/server/request_context.cpp index 2b5b6fd4c..d879240cd 100644 --- a/src/server/request_context.cpp +++ b/src/server/request_context.cpp @@ -51,11 +51,12 @@ RequestMethod str2RequestMethod(const std::string& method) { } // unnamed namespace -RequestContext::RequestContext(struct MHD_Connection* connection, - const std::string& _rootLocation, // URI-encoded +RequestContext::RequestContext(const std::string& _rootLocation, // URI-encoded const std::string& unrootedUrl, // URI-decoded const std::string& _method, - const std::string& version) : + const std::string& version, + const NameValuePairs& headers, + const NameValuePairs& queryArgs) : rootLocation(_rootLocation), url(unrootedUrl), method(str2RequestMethod(_method)), @@ -64,8 +65,13 @@ RequestContext::RequestContext(struct MHD_Connection* connection, acceptEncodingGzip(false), byteRange_() { - MHD_get_connection_values(connection, MHD_HEADER_KIND, &RequestContext::fill_header, this); - MHD_get_connection_values(connection, MHD_GET_ARGUMENT_KIND, &RequestContext::fill_argument, this); + for ( const auto& kv : headers ) { + add_header(kv.first, kv.second); + } + + for ( const auto& kv : queryArgs ) { + add_argument(kv.first, kv.second); + } try { acceptEncodingGzip = @@ -82,18 +88,14 @@ RequestContext::RequestContext(struct MHD_Connection* connection, RequestContext::~RequestContext() {} -MHD_Result RequestContext::fill_header(void *__this, enum MHD_ValueKind kind, - const char *key, const char *value) +void RequestContext::add_header(const char *key, const char *value) { - RequestContext *_this = static_cast(__this); - _this->headers[lcAll(key)] = value; - return MHD_YES; + this->headers[lcAll(key)] = value; } -MHD_Result RequestContext::fill_argument(void *__this, enum MHD_ValueKind kind, - const char *key, const char* value) +void RequestContext::add_argument(const char *key, const char* value) { - RequestContext *_this = static_cast(__this); + RequestContext *_this = this; _this->arguments[key].push_back(value == nullptr ? "" : value); if ( ! _this->queryString.empty() ) { _this->queryString += "&"; @@ -103,7 +105,6 @@ MHD_Result RequestContext::fill_argument(void *__this, enum MHD_ValueKind kind, _this->queryString += "="; _this->queryString += urlEncode(value); } - return MHD_YES; } void RequestContext::print_debug_info() const { diff --git a/src/server/request_context.h b/src/server/request_context.h index 0393dce77..d4fe53dd5 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -55,12 +55,17 @@ class IndexError: public std::runtime_error {}; class RequestContext { + public: // types + typedef std::vector> NameValuePairs; + public: // functions - RequestContext(struct MHD_Connection* connection, - const std::string& rootLocation, // URI-encoded + RequestContext(const std::string& rootLocation, // URI-encoded const std::string& unrootedUrl, // URI-decoded const std::string& method, - const std::string& version); + const std::string& version, + const NameValuePairs& headers, + const NameValuePairs& queryArgs); + ~RequestContext(); void print_debug_info() const; @@ -151,8 +156,8 @@ class RequestContext { private: // functions UserLanguage determine_user_language() const; - static MHD_Result fill_header(void *, enum MHD_ValueKind, const char*, const char*); - static MHD_Result fill_argument(void *, enum MHD_ValueKind, const char*, const char*); + void add_header(const char* name, const char* value); + void add_argument(const char* name, const char* value); }; template<> std::string RequestContext::get_argument(const std::string& name) const; diff --git a/test/response.cpp b/test/response.cpp index 5d5efe649..c65691b8d 100644 --- a/test/response.cpp +++ b/test/response.cpp @@ -8,9 +8,11 @@ namespace using namespace kiwix; -RequestContext makeHttpGetRequest(const std::string& url) +RequestContext makeHttpGetRequest(const std::string& url, + const RequestContext::NameValuePairs& headers, + const RequestContext::NameValuePairs& queryArgs) { - return RequestContext(nullptr, "", url, "GET", "1.1"); + return RequestContext("", url, "GET", "1.1", headers, queryArgs); } std::string getResponseContent(const ContentResponseBlueprint& crb) @@ -23,7 +25,7 @@ std::string getResponseContent(const ContentResponseBlueprint& crb) TEST(HTTPErrorResponse, shouldBeInEnglishByDefault) { - const RequestContext req = makeHttpGetRequest("/asdf"); + const RequestContext req = makeHttpGetRequest("/asdf", {}, {}); HTTPErrorResponse errResp(req, MHD_HTTP_NOT_FOUND, "404-page-title", "404-page-heading", From c57b8a0c7c949c17a341e76d8e22bd2536425ff8 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 5 Dec 2023 17:50:26 +0400 Subject: [PATCH 07/28] Testing of HTTPErrorResponse translation --- test/response.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/response.cpp b/test/response.cpp index c65691b8d..cbfbfd3d7 100644 --- a/test/response.cpp +++ b/test/response.cpp @@ -46,3 +46,30 @@ R"( )"); } + +TEST(HTTPErrorResponse, shouldBeTranslatable) { + const RequestContext req = makeHttpGetRequest("/asdf", + /* headers */ {}, + /* query args */ {{"userlang", "test"}} + ); + + HTTPErrorResponse errResp(req, MHD_HTTP_NOT_FOUND, + "404-page-title", + "404-page-heading", + "/css/error.css"); + + EXPECT_EQ(getResponseContent(errResp), +R"( + + + + [I18N TESTING] Not Found - Try Again + + + +

[I18N TESTING] Content not found, but at least the server is alive

+ + + +)"); +} From 797f4c432cb920630b0628461911ff0a8f070f46 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 6 Dec 2023 14:42:31 +0400 Subject: [PATCH 08/28] Testing of MIME-type of HTTP 500 response --- test/server.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/server.cpp b/test/server.cpp index 3c1e85954..9beed7c2c 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -1054,6 +1054,7 @@ TEST_F(ServerTest, 500) const auto r = zfs1_->GET("/ROOT%23%3F/content/poor/A/redirect_loop.html"); EXPECT_EQ(r->status, 500); EXPECT_EQ(r->body, expectedBody); + EXPECT_EQ(r->get_header_value("Content-Type"), "text/html;charset=utf-8"); } } From 54191bcfabc2ec65def93db8e5fc4be57a676256 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 6 Dec 2023 14:45:04 +0400 Subject: [PATCH 09/28] Retired HTTP500Response::generateResponseObject() ... whereupon `ContentResponseBlueprint::generateResponseObject()` (and `ContentResponseBlueprint` as a whole) no longer needs to be polymorphic. --- src/server/response.cpp | 8 -------- src/server/response.h | 11 +---------- test/server.cpp | 2 +- 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 8ccab1313..b5a962fd9 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -234,14 +234,6 @@ HTTP500Response::HTTP500Response(const RequestContext& request) *this += nonParameterizedMessage("500-page-text"); } -std::unique_ptr HTTP500Response::generateResponseObject() const -{ - const std::string mimeType = "text/html;charset=utf-8"; - auto r = ContentResponse::build(m_template, m_data, mimeType); - r->set_code(m_httpStatusCode); - return r; -} - std::unique_ptr Response::build_416(size_t resourceLength) { auto response = Response::build(); diff --git a/src/server/response.h b/src/server/response.h index 643f67f88..8fe909ed5 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -128,20 +128,16 @@ class ContentResponseBlueprint , m_template(templateStr) {} - virtual ~ContentResponseBlueprint() = default; - operator std::unique_ptr() const { return generateResponseObject(); } + std::unique_ptr generateResponseObject() const; protected: // functions std::string getMessage(const std::string& msgId) const; -public: - virtual std::unique_ptr generateResponseObject() const; - public: //data const RequestContext& m_request; const int m_httpStatusCode; @@ -180,11 +176,6 @@ struct HTTP400Response : HTTPErrorResponse struct HTTP500Response : HTTPErrorResponse { explicit HTTP500Response(const RequestContext& request); - -private: // overrides - // generateResponseObject() is overriden in order to produce a minimal - // response without any need for additional resources from the server - std::unique_ptr generateResponseObject() const override; }; class ItemResponse : public Response { diff --git a/test/server.cpp b/test/server.cpp index 9beed7c2c..f9d09e509 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -1054,7 +1054,7 @@ TEST_F(ServerTest, 500) const auto r = zfs1_->GET("/ROOT%23%3F/content/poor/A/redirect_loop.html"); EXPECT_EQ(r->status, 500); EXPECT_EQ(r->body, expectedBody); - EXPECT_EQ(r->get_header_value("Content-Type"), "text/html;charset=utf-8"); + EXPECT_EQ(r->get_header_value("Content-Type"), "text/html; charset=utf-8"); } } From 0b7cd614c6e800ee9508bb10831d17f8d7a583c9 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 6 Dec 2023 14:50:15 +0400 Subject: [PATCH 10/28] Fixed an encapsulation breach --- src/server/response.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/response.h b/src/server/response.h index 8fe909ed5..b17b3efb7 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -138,7 +138,7 @@ class ContentResponseBlueprint protected: // functions std::string getMessage(const std::string& msgId) const; -public: //data +protected: //data const RequestContext& m_request; const int m_httpStatusCode; const std::string m_mimeType; From d39e91f6bc631bd310904626b46072f1a5c89f3d Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 9 Jan 2024 22:46:06 +0400 Subject: [PATCH 11/28] Moved constructor into .cpp --- src/server/response.cpp | 10 ++++++++++ src/server/response.h | 7 +------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index b5a962fd9..918b04161 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -151,6 +151,16 @@ std::unique_ptr Response::build_304(const ETag& etag) return response; } +ContentResponseBlueprint::ContentResponseBlueprint(const RequestContext* request, + int httpStatusCode, + const std::string& mimeType, + const std::string& templateStr) + : m_request(*request) + , m_httpStatusCode(httpStatusCode) + , m_mimeType(mimeType) + , m_template(templateStr) +{} + std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const { return getTranslatedString(m_request.get_user_language(), msgId); diff --git a/src/server/response.h b/src/server/response.h index b17b3efb7..c44010766 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -121,12 +121,7 @@ class ContentResponseBlueprint ContentResponseBlueprint(const RequestContext* request, int httpStatusCode, const std::string& mimeType, - const std::string& templateStr) - : m_request(*request) - , m_httpStatusCode(httpStatusCode) - , m_mimeType(mimeType) - , m_template(templateStr) - {} + const std::string& templateStr); operator std::unique_ptr() const { From e72fc2391d532319f9d7719be20e5888c84bd01b Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 9 Jan 2024 22:50:34 +0400 Subject: [PATCH 12/28] Enter ContentResponseBlueprint::Data ContentResponseBlueprint::m_data is now an opaque data member implemented in the .cpp and ready to be switched from kainjow::mustache::data to a different implementation. --- src/server/response.cpp | 16 +++++++++++++--- src/server/response.h | 7 ++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 918b04161..6ae29a3f9 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -151,6 +151,13 @@ std::unique_ptr Response::build_304(const ETag& etag) return response; } +class ContentResponseBlueprint::Data : public kainjow::mustache::data +{ +public: + Data() {} + template Data(const T& t) : kainjow::mustache::data(t) {} +}; + ContentResponseBlueprint::ContentResponseBlueprint(const RequestContext* request, int httpStatusCode, const std::string& mimeType, @@ -159,8 +166,11 @@ ContentResponseBlueprint::ContentResponseBlueprint(const RequestContext* request , m_httpStatusCode(httpStatusCode) , m_mimeType(mimeType) , m_template(templateStr) + , m_data(new Data) {} +ContentResponseBlueprint::~ContentResponseBlueprint() = default; + std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const { return getTranslatedString(m_request.get_user_language(), msgId); @@ -168,7 +178,7 @@ std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const std::unique_ptr ContentResponseBlueprint::generateResponseObject() const { - auto r = ContentResponse::build(m_template, m_data, m_mimeType); + auto r = ContentResponse::build(m_template, *m_data, m_mimeType); r->set_code(m_httpStatusCode); return r; } @@ -184,7 +194,7 @@ HTTPErrorResponse::HTTPErrorResponse(const RequestContext& request, request.get_requested_format() == "html" ? RESOURCE::templates::error_html : RESOURCE::templates::error_xml) { kainjow::mustache::list emptyList; - this->m_data = kainjow::mustache::object{ + *this->m_data = kainjow::mustache::object{ {"CSS_URL", onlyAsNonEmptyMustacheValue(cssUrl) }, {"PAGE_TITLE", getMessage(pageTitleMsgId)}, {"PAGE_HEADING", getMessage(headingMsgId)}, @@ -210,7 +220,7 @@ UrlNotFoundResponse::UrlNotFoundResponse(const RequestContext& request) HTTPErrorResponse& HTTPErrorResponse::operator+(const ParameterizedMessage& details) { const std::string msg = details.getText(m_request.get_user_language()); - m_data["details"].push_back({"p", msg}); + (*m_data)["details"].push_back({"p", msg}); return *this; } diff --git a/src/server/response.h b/src/server/response.h index c44010766..726a51668 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -123,6 +123,8 @@ class ContentResponseBlueprint const std::string& mimeType, const std::string& templateStr); + ~ContentResponseBlueprint(); + operator std::unique_ptr() const { return generateResponseObject(); @@ -130,6 +132,9 @@ class ContentResponseBlueprint std::unique_ptr generateResponseObject() const; +protected: // types + class Data; + protected: // functions std::string getMessage(const std::string& msgId) const; @@ -138,7 +143,7 @@ class ContentResponseBlueprint const int m_httpStatusCode; const std::string m_mimeType; const std::string m_template; - kainjow::mustache::data m_data; + std::unique_ptr m_data; }; struct HTTPErrorResponse : ContentResponseBlueprint From 0b542fe66d7d0d7e3469f5aa98572631e053b3c8 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 6 Dec 2023 16:37:12 +0400 Subject: [PATCH 13/28] New implementation of ContentResponseBlueprint::Data --- src/server/response.cpp | 75 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 67 insertions(+), 8 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 6ae29a3f9..ce418d54c 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -32,6 +32,9 @@ #include #include +#include +#include +#include // This is somehow a magic value. // If this value is too small, we will compress (and lost cpu time) too much @@ -47,6 +50,8 @@ namespace kiwix { namespace { +typedef kainjow::mustache::data MustacheData; + // some utilities std::string get_mime_type(const zim::Item& item) @@ -151,13 +156,66 @@ std::unique_ptr Response::build_304(const ETag& etag) return response; } -class ContentResponseBlueprint::Data : public kainjow::mustache::data +class ContentResponseBlueprint::Data { +public: + typedef std::list List; + typedef std::map Object; + +private: + std::variant data; + public: Data() {} - template Data(const T& t) : kainjow::mustache::data(t) {} + template Data(const T& t) : data(t) {} + + MustacheData toMustache(const std::string& lang) const; + + Data& operator[](const std::string& key) + { + return std::get(data)[key]; + } + + void push_back(const Data& d) { std::get(data).push_back(d); } + + static Data onlyAsNonEmptyValue(const std::string& s) + { + return s.empty() ? Data(false) : Data(s); + } + +private: + bool isString() const { return std::holds_alternative(data); } + bool isList() const { return std::holds_alternative(data); } + bool isObject() const { return std::holds_alternative(data); } + + const std::string& stringValue() const { return std::get(data); } + bool boolValue() const { return std::get(data); } + const List& listValue() const { return std::get(data); } + const Object& objectValue() const { return std::get(data); } }; +MustacheData ContentResponseBlueprint::Data::toMustache(const std::string& lang) const +{ + if ( this->isList() ) { + kainjow::mustache::list l; + for ( const auto& x : this->listValue() ) { + l.push_back(x.toMustache(lang)); + } + return l; + } else if ( this->isObject() ) { + kainjow::mustache::object o; + for ( const auto& kv : this->objectValue() ) { + o[kv.first] = kv.second.toMustache(lang); + } + return o; + } else if ( this->isString() ) { + return this->stringValue(); + } else { + return this->boolValue(); + } +} + + ContentResponseBlueprint::ContentResponseBlueprint(const RequestContext* request, int httpStatusCode, const std::string& mimeType, @@ -178,7 +236,8 @@ std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const std::unique_ptr ContentResponseBlueprint::generateResponseObject() const { - auto r = ContentResponse::build(m_template, *m_data, m_mimeType); + kainjow::mustache::data d = m_data->toMustache(m_request.get_user_language()); + auto r = ContentResponse::build(m_template, d, m_mimeType); r->set_code(m_httpStatusCode); return r; } @@ -193,13 +252,13 @@ HTTPErrorResponse::HTTPErrorResponse(const RequestContext& request, request.get_requested_format() == "html" ? "text/html; charset=utf-8" : "application/xml; charset=utf-8", request.get_requested_format() == "html" ? RESOURCE::templates::error_html : RESOURCE::templates::error_xml) { - kainjow::mustache::list emptyList; - *this->m_data = kainjow::mustache::object{ - {"CSS_URL", onlyAsNonEmptyMustacheValue(cssUrl) }, + Data::List emptyList; + *this->m_data = Data(Data::Object{ + {"CSS_URL", Data::onlyAsNonEmptyValue(cssUrl) }, {"PAGE_TITLE", getMessage(pageTitleMsgId)}, {"PAGE_HEADING", getMessage(headingMsgId)}, {"details", emptyList} - }; + }); } HTTP404Response::HTTP404Response(const RequestContext& request) @@ -220,7 +279,7 @@ UrlNotFoundResponse::UrlNotFoundResponse(const RequestContext& request) HTTPErrorResponse& HTTPErrorResponse::operator+(const ParameterizedMessage& details) { const std::string msg = details.getText(m_request.get_user_language()); - (*m_data)["details"].push_back({"p", msg}); + (*m_data)["details"].push_back(Data::Object{{"p", msg}}); return *this; } From f298acd45fcb180f33743c6cf07b7929aeb1ab3d Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 9 Jan 2024 23:38:44 +0400 Subject: [PATCH 14/28] Unmustached i18n::Parameters --- src/server/i18n.cpp | 6 +++++- src/server/i18n.h | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/server/i18n.cpp b/src/server/i18n.cpp index 0a2cd8c73..5b948f26e 100644 --- a/src/server/i18n.cpp +++ b/src/server/i18n.cpp @@ -112,8 +112,12 @@ std::string expandParameterizedString(const std::string& lang, const std::string& key, const Parameters& params) { + kainjow::mustache::object mustacheParams; + for( const auto& kv : params ) { + mustacheParams[kv.first] = kv.second; + } const std::string tmpl = getTranslatedString(lang, key); - return render_template(tmpl, params); + return render_template(tmpl, mustacheParams); } } // namespace i18n diff --git a/src/server/i18n.h b/src/server/i18n.h index 36d4b955b..7eeb0205a 100644 --- a/src/server/i18n.h +++ b/src/server/i18n.h @@ -20,6 +20,7 @@ #ifndef KIWIX_SERVER_I18N #define KIWIX_SERVER_I18N +#include #include #include @@ -44,7 +45,7 @@ std::string getTranslatedString(const std::string& lang, const std::string& key) namespace i18n { -typedef kainjow::mustache::object Parameters; +typedef std::map Parameters; std::string expandParameterizedString(const std::string& lang, const std::string& key, @@ -96,7 +97,7 @@ class GetTranslatedStringWithMsgId class ParameterizedMessage { public: // types - typedef kainjow::mustache::object Parameters; + typedef i18n::Parameters Parameters; public: // functions ParameterizedMessage(const std::string& msgId, const Parameters& params) From 1553d52593fde8f9acd46f353c12422528d6e50d Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 9 Jan 2024 23:13:23 +0400 Subject: [PATCH 15/28] Lazy translation during error response generation Now when parameterized messages are added to an error response, they are not immediately instantiated (translated). Instead the message id and the parameters of the message are recorded. The instantiation of the messages happens right before generating the final content of the response. --- src/server/i18n.h | 3 +++ src/server/response.cpp | 45 ++++++++++++++++++++++++++++++++--------- src/server/response.h | 3 --- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/server/i18n.h b/src/server/i18n.h index 7eeb0205a..1e42cac68 100644 --- a/src/server/i18n.h +++ b/src/server/i18n.h @@ -107,6 +107,9 @@ class ParameterizedMessage std::string getText(const std::string& lang) const; + const std::string& getMsgId() const { return msgId; } + const Parameters& getParams() const { return params; } + private: // data const std::string msgId; const Parameters params; diff --git a/src/server/response.cpp b/src/server/response.cpp index ce418d54c..53584349b 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -183,6 +183,18 @@ class ContentResponseBlueprint::Data return s.empty() ? Data(false) : Data(s); } + static Data from(const ParameterizedMessage& pmsg) + { + Object obj; + for(const auto& kv : pmsg.getParams()) { + obj[kv.first] = kv.second; + } + return Object{ + { "msgid", pmsg.getMsgId() }, + { "params", Data(obj) } + }; + } + private: bool isString() const { return std::holds_alternative(data); } bool isList() const { return std::holds_alternative(data); } @@ -192,6 +204,16 @@ class ContentResponseBlueprint::Data bool boolValue() const { return std::get(data); } const List& listValue() const { return std::get(data); } const Object& objectValue() const { return std::get(data); } + + const Data* get(const std::string& key) const + { + if ( !isObject() ) + return nullptr; + + const auto& obj = objectValue(); + const auto it = obj.find(key); + return it != obj.end() ? &it->second : nullptr; + } }; MustacheData ContentResponseBlueprint::Data::toMustache(const std::string& lang) const @@ -203,11 +225,22 @@ MustacheData ContentResponseBlueprint::Data::toMustache(const std::string& lang) } return l; } else if ( this->isObject() ) { + const Data* msgId = this->get("msgid"); + const Data* msgParams = this->get("params"); + if ( msgId && msgId->isString() && msgParams && msgParams->isObject() ) { + std::map params; + for(const auto& kv : msgParams->objectValue()) { + params[kv.first] = kv.second.stringValue(); + } + const ParameterizedMessage msg(msgId->stringValue(), ParameterizedMessage::Parameters(params)); + return msg.getText(lang); + } else { kainjow::mustache::object o; for ( const auto& kv : this->objectValue() ) { o[kv.first] = kv.second.toMustache(lang); } return o; + } } else if ( this->isString() ) { return this->stringValue(); } else { @@ -229,11 +262,6 @@ ContentResponseBlueprint::ContentResponseBlueprint(const RequestContext* request ContentResponseBlueprint::~ContentResponseBlueprint() = default; -std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const -{ - return getTranslatedString(m_request.get_user_language(), msgId); -} - std::unique_ptr ContentResponseBlueprint::generateResponseObject() const { kainjow::mustache::data d = m_data->toMustache(m_request.get_user_language()); @@ -255,8 +283,8 @@ HTTPErrorResponse::HTTPErrorResponse(const RequestContext& request, Data::List emptyList; *this->m_data = Data(Data::Object{ {"CSS_URL", Data::onlyAsNonEmptyValue(cssUrl) }, - {"PAGE_TITLE", getMessage(pageTitleMsgId)}, - {"PAGE_HEADING", getMessage(headingMsgId)}, + {"PAGE_TITLE", Data::from(nonParameterizedMessage(pageTitleMsgId))}, + {"PAGE_HEADING", Data::from(nonParameterizedMessage(headingMsgId))}, {"details", emptyList} }); } @@ -278,8 +306,7 @@ UrlNotFoundResponse::UrlNotFoundResponse(const RequestContext& request) HTTPErrorResponse& HTTPErrorResponse::operator+(const ParameterizedMessage& details) { - const std::string msg = details.getText(m_request.get_user_language()); - (*m_data)["details"].push_back(Data::Object{{"p", msg}}); + (*m_data)["details"].push_back(Data::Object{{"p", Data::from(details)}}); return *this; } diff --git a/src/server/response.h b/src/server/response.h index 726a51668..e239173df 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -135,9 +135,6 @@ class ContentResponseBlueprint protected: // types class Data; -protected: // functions - std::string getMessage(const std::string& msgId) const; - protected: //data const RequestContext& m_request; const int m_httpStatusCode; From f3d3ab13cbdafe7951fc020706f942f3719ea9b8 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 9 Jan 2024 20:58:44 +0400 Subject: [PATCH 16/28] Exposed escapeForJSON() in kiwix namespace Note that it is declared in stringTools.h but its definition remains in otherTools.cpp (to minimize the diff). --- src/tools/otherTools.cpp | 8 ++++---- src/tools/stringTools.h | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tools/otherTools.cpp b/src/tools/otherTools.cpp index 85e58a496..97d1567f7 100644 --- a/src/tools/otherTools.cpp +++ b/src/tools/otherTools.cpp @@ -327,10 +327,7 @@ std::string kiwix::render_template(const std::string& template_str, kainjow::mus return ss.str(); } -namespace -{ - -std::string escapeForJSON(const std::string& s) +std::string kiwix::escapeForJSON(const std::string& s) { std::ostringstream oss; for (char c : s) { @@ -345,6 +342,9 @@ std::string escapeForJSON(const std::string& s) return oss.str(); } +namespace +{ + std::string makeFulltextSearchSuggestion(const std::string& lang, const std::string& queryString) { diff --git a/src/tools/stringTools.h b/src/tools/stringTools.h index 14fed7574..890254283 100644 --- a/src/tools/stringTools.h +++ b/src/tools/stringTools.h @@ -53,6 +53,7 @@ class ICULanguageInfo const icu::Locale locale; }; +std::string escapeForJSON(const std::string& s); /* urlEncode() is the equivalent of JS encodeURIComponent(), with the only * difference that the slash (/) symbol is NOT encoded. */ From 8b8a2eede7e9a813d540588ca79525ddb8650835 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 6 Jan 2024 16:29:41 +0400 Subject: [PATCH 17/28] Slight enhancement of escapeForJSON() - More familiar escape sequences for tab, newline and carriage return symbols. - Quote symbol is escaped by default too, however that behaviour can be disabled for uses in HTML-related contexts where quotes should then be replaced with the character entity " --- src/tools/otherTools.cpp | 27 ++++++++++++++++++++------- src/tools/stringTools.h | 2 +- test/otherTools.cpp | 16 ++++++++-------- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/tools/otherTools.cpp b/src/tools/otherTools.cpp index 97d1567f7..0c6eb36ca 100644 --- a/src/tools/otherTools.cpp +++ b/src/tools/otherTools.cpp @@ -327,14 +327,27 @@ std::string kiwix::render_template(const std::string& template_str, kainjow::mus return ss.str(); } -std::string kiwix::escapeForJSON(const std::string& s) +// The escapeQuote parameter of escapeForJSON() defaults to true. +// This constant makes the calls to escapeForJSON() where the quote symbol +// should not be escaped (as it is later replaced with the HTML character entity +// ") more readable. +static const bool DONT_ESCAPE_QUOTE = false; + +std::string kiwix::escapeForJSON(const std::string& s, bool escapeQuote) { std::ostringstream oss; for (char c : s) { if ( c == '\\' ) { oss << "\\\\"; } else if ( unsigned(c) < 0x20U ) { - oss << "\\u" << std::setw(4) << std::setfill('0') << unsigned(c); + switch ( c ) { + case '\n': oss << "\\n"; break; + case '\r': oss << "\\r"; break; + case '\t': oss << "\\t"; break; + default: oss << "\\u" << std::setw(4) << std::setfill('0') << unsigned(c); + } + } else if ( c == '"' && escapeQuote ) { + oss << "\\\""; } else { oss << c; } @@ -370,10 +383,10 @@ void kiwix::Suggestions::add(const zim::SuggestionItem& suggestion) ? suggestion.getSnippet() : suggestion.getTitle(); - result.set("label", escapeForJSON(label)); - result.set("value", escapeForJSON(suggestion.getTitle())); + result.set("label", escapeForJSON(label, DONT_ESCAPE_QUOTE)); + result.set("value", escapeForJSON(suggestion.getTitle(), DONT_ESCAPE_QUOTE)); result.set("kind", "path"); - result.set("path", escapeForJSON(suggestion.getPath())); + result.set("path", escapeForJSON(suggestion.getPath(), DONT_ESCAPE_QUOTE)); result.set("first", m_data.is_empty_list()); m_data.push_back(result); } @@ -383,8 +396,8 @@ void kiwix::Suggestions::addFTSearchSuggestion(const std::string& uiLang, { kainjow::mustache::data result; const std::string label = makeFulltextSearchSuggestion(uiLang, queryString); - result.set("label", escapeForJSON(label)); - result.set("value", escapeForJSON(queryString + " ")); + result.set("label", escapeForJSON(label, DONT_ESCAPE_QUOTE)); + result.set("value", escapeForJSON(queryString + " ", DONT_ESCAPE_QUOTE)); result.set("kind", "pattern"); result.set("first", m_data.is_empty_list()); m_data.push_back(result); diff --git a/src/tools/stringTools.h b/src/tools/stringTools.h index 890254283..97fa34738 100644 --- a/src/tools/stringTools.h +++ b/src/tools/stringTools.h @@ -53,7 +53,7 @@ class ICULanguageInfo const icu::Locale locale; }; -std::string escapeForJSON(const std::string& s); +std::string escapeForJSON(const std::string& s, bool escapeQuote = true); /* urlEncode() is the equivalent of JS encodeURIComponent(), with the only * difference that the slash (/) symbol is NOT encoded. */ diff --git a/test/otherTools.cpp b/test/otherTools.cpp index d437e188d..3a3eb0477 100644 --- a/test/otherTools.cpp +++ b/test/otherTools.cpp @@ -110,10 +110,10 @@ TEST(Suggestions, specialCharHandling) CHECK_SUGGESTIONS(s.getJSON(), R"EXPECTEDJSON([ { - "value" : "Title with \u0009\u0010\u0013\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?", - "label" : "Snippet with \u0009\u0010\u0013\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?", + "value" : "Title with \t\n\r\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?", + "label" : "Snippet with \t\n\r\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?", "kind" : "path" - , "path" : "Path with \u0009\u0010\u0013\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?" + , "path" : "Path with \t\n\r\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?" } ] )EXPECTEDJSON" @@ -128,10 +128,10 @@ R"EXPECTEDJSON([ CHECK_SUGGESTIONS(s.getJSON(), R"EXPECTEDJSON([ { - "value" : "Snippetless title with \u0009\u0010\u0013\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?", - "label" : "Snippetless title with \u0009\u0010\u0013\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?", + "value" : "Snippetless title with \t\n\r\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?", + "label" : "Snippetless title with \t\n\r\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?", "kind" : "path" - , "path" : "Path with \u0009\u0010\u0013\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?" + , "path" : "Path with \t\n\r\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?" } ] )EXPECTEDJSON" @@ -145,8 +145,8 @@ R"EXPECTEDJSON([ CHECK_SUGGESTIONS(s.getJSON(), R"EXPECTEDJSON([ { - "value" : "text with \u0009\u0010\u0013\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.? ", - "label" : "containing 'text with \u0009\u0010\u0013\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?'...", + "value" : "text with \t\n\r\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.? ", + "label" : "containing 'text with \t\n\r\\<>&'"~!@#$%^*()_+`-=[]{}|:;,.?'...", "kind" : "pattern" //EOLWHITESPACEMARKER } From b151a2a480a664866ddc5eaf75087b364ca28468 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 9 Jan 2024 21:59:53 +0400 Subject: [PATCH 18/28] Added KIWIX_RESPONSE_DATA to error response Now the data used to generate an error response can be made to be embedded in the response as a JS object KIWIX_RESPONSE_DATA. --- src/server/response.cpp | 50 ++++++++++++++++++++++++++++++++++--- src/server/response.h | 7 ++++-- static/templates/error.html | 4 ++- test/response.cpp | 12 +++++++-- 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 53584349b..efb72a9c9 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -195,6 +195,9 @@ class ContentResponseBlueprint::Data }; } + std::string asJSON() const; + void dumpJSON(std::ostream& os) const; + private: bool isString() const { return std::holds_alternative(data); } bool isList() const { return std::holds_alternative(data); } @@ -248,15 +251,51 @@ MustacheData ContentResponseBlueprint::Data::toMustache(const std::string& lang) } } +void ContentResponseBlueprint::Data::dumpJSON(std::ostream& os) const +{ + if ( this->isString() ) { + os << '"' << escapeForJSON(this->stringValue()) << '"'; + } else if ( this->isList() ) { + const char * sep = " "; + os << "["; + + for ( const auto& x : this->listValue() ) { + os << sep; + x.dumpJSON(os); + sep = ", "; + } + os << " ]"; + } else if ( this->isObject() ) { + const char * sep = " "; + os << "{"; + for ( const auto& kv : this->objectValue() ) { + os << sep << '"' << kv.first << "\" : "; + kv.second.dumpJSON(os); + sep = ", "; + } + os << " }"; + } else { + os << (this->boolValue() ? "true" : "false"); + } +} + +std::string ContentResponseBlueprint::Data::asJSON() const +{ + std::ostringstream oss; + this->dumpJSON(oss); + return oss.str(); +} ContentResponseBlueprint::ContentResponseBlueprint(const RequestContext* request, int httpStatusCode, const std::string& mimeType, - const std::string& templateStr) + const std::string& templateStr, + bool includeKiwixResponseData) : m_request(*request) , m_httpStatusCode(httpStatusCode) , m_mimeType(mimeType) , m_template(templateStr) + , m_includeKiwixResponseData(includeKiwixResponseData) , m_data(new Data) {} @@ -265,6 +304,9 @@ ContentResponseBlueprint::~ContentResponseBlueprint() = default; std::unique_ptr ContentResponseBlueprint::generateResponseObject() const { kainjow::mustache::data d = m_data->toMustache(m_request.get_user_language()); + if ( m_includeKiwixResponseData ) { + d.set("KIWIX_RESPONSE_DATA", m_data->asJSON()); + } auto r = ContentResponse::build(m_template, d, m_mimeType); r->set_code(m_httpStatusCode); return r; @@ -274,11 +316,13 @@ HTTPErrorResponse::HTTPErrorResponse(const RequestContext& request, int httpStatusCode, const std::string& pageTitleMsgId, const std::string& headingMsgId, - const std::string& cssUrl) + const std::string& cssUrl, + bool includeKiwixResponseData) : ContentResponseBlueprint(&request, httpStatusCode, request.get_requested_format() == "html" ? "text/html; charset=utf-8" : "application/xml; charset=utf-8", - request.get_requested_format() == "html" ? RESOURCE::templates::error_html : RESOURCE::templates::error_xml) + request.get_requested_format() == "html" ? RESOURCE::templates::error_html : RESOURCE::templates::error_xml, + includeKiwixResponseData) { Data::List emptyList; *this->m_data = Data(Data::Object{ diff --git a/src/server/response.h b/src/server/response.h index e239173df..11808f0da 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -121,7 +121,8 @@ class ContentResponseBlueprint ContentResponseBlueprint(const RequestContext* request, int httpStatusCode, const std::string& mimeType, - const std::string& templateStr); + const std::string& templateStr, + bool includeKiwixResponseData = false); ~ContentResponseBlueprint(); @@ -140,6 +141,7 @@ class ContentResponseBlueprint const int m_httpStatusCode; const std::string m_mimeType; const std::string m_template; + const bool m_includeKiwixResponseData; std::unique_ptr m_data; }; @@ -149,7 +151,8 @@ struct HTTPErrorResponse : ContentResponseBlueprint int httpStatusCode, const std::string& pageTitleMsgId, const std::string& headingMsgId, - const std::string& cssUrl = ""); + const std::string& cssUrl = "", + bool includeKiwixResponseData = false); HTTPErrorResponse& operator+(const ParameterizedMessage& errorDetails); HTTPErrorResponse& operator+=(const ParameterizedMessage& errorDetails); diff --git a/static/templates/error.html b/static/templates/error.html index 711082096..717e56b27 100644 --- a/static/templates/error.html +++ b/static/templates/error.html @@ -5,7 +5,9 @@ {{PAGE_TITLE}} {{#CSS_URL}} -{{/CSS_URL}} +{{/CSS_URL}}{{#KIWIX_RESPONSE_DATA}} {{/KIWIX_RESPONSE_DATA}}

{{PAGE_HEADING}}

diff --git a/test/response.cpp b/test/response.cpp index cbfbfd3d7..5b9d8c92a 100644 --- a/test/response.cpp +++ b/test/response.cpp @@ -29,7 +29,8 @@ TEST(HTTPErrorResponse, shouldBeInEnglishByDefault) { HTTPErrorResponse errResp(req, MHD_HTTP_NOT_FOUND, "404-page-title", "404-page-heading", - "/css/error.css"); + "/css/error.css", + /*includeKiwixResponseData=*/true); EXPECT_EQ(getResponseContent(errResp), R"( @@ -38,6 +39,9 @@ R"( Content not found +

Not Found

@@ -56,7 +60,8 @@ TEST(HTTPErrorResponse, shouldBeTranslatable) { HTTPErrorResponse errResp(req, MHD_HTTP_NOT_FOUND, "404-page-title", "404-page-heading", - "/css/error.css"); + "/css/error.css", + /*includeKiwixResponseData=*/true); EXPECT_EQ(getResponseContent(errResp), R"( @@ -65,6 +70,9 @@ R"( [I18N TESTING] Not Found - Try Again +

[I18N TESTING] Content not found, but at least the server is alive

From d2fedf9123f91dac04a28d388a7cec663cbbd352 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 5 Jan 2024 18:42:42 +0400 Subject: [PATCH 19/28] Added error details in testing of error responses --- test/response.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/test/response.cpp b/test/response.cpp index 5b9d8c92a..5762f8319 100644 --- a/test/response.cpp +++ b/test/response.cpp @@ -32,6 +32,12 @@ TEST(HTTPErrorResponse, shouldBeInEnglishByDefault) { "/css/error.css", /*includeKiwixResponseData=*/true); + errResp += ParameterizedMessage("suggest-search", + { + { "PATTERN", "asdf" }, + { "SEARCH_URL", "/search?q=asdf" } + }); + EXPECT_EQ(getResponseContent(errResp), R"( @@ -40,12 +46,14 @@ R"( Content not found

Not Found

- +

+ Make a full text search for asdf +

)"); @@ -63,6 +71,12 @@ TEST(HTTPErrorResponse, shouldBeTranslatable) { "/css/error.css", /*includeKiwixResponseData=*/true); + errResp += ParameterizedMessage("suggest-search", + { + { "PATTERN", "asdf" }, + { "SEARCH_URL", "/search?q=asdf" } + }); + EXPECT_EQ(getResponseContent(errResp), R"( @@ -71,12 +85,14 @@ R"( [I18N TESTING] Not Found - Try Again

[I18N TESTING] Content not found, but at least the server is alive

- +

+ [I18N TESTING] Make a full text search for asdf +

)"); From e14de692710bfcb18f3eff662839de14c2404563 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 6 Jan 2024 16:35:23 +0400 Subject: [PATCH 20/28] The page template is embedded in the error response This is a shortcut change since it doesn't make sense to send the error page template with every error response (the viewer can fetch it from the server once but that's slightly more work). --- src/server/response.cpp | 1 + static/templates/error.html | 1 + test/response.cpp | 2 ++ 3 files changed, 4 insertions(+) diff --git a/src/server/response.cpp b/src/server/response.cpp index efb72a9c9..df3dc10ef 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -305,6 +305,7 @@ std::unique_ptr ContentResponseBlueprint::generateResponseObjec { kainjow::mustache::data d = m_data->toMustache(m_request.get_user_language()); if ( m_includeKiwixResponseData ) { + d.set("KIWIX_RESPONSE_TEMPLATE", escapeForJSON(m_template)); d.set("KIWIX_RESPONSE_DATA", m_data->asJSON()); } auto r = ContentResponse::build(m_template, d, m_mimeType); diff --git a/static/templates/error.html b/static/templates/error.html index 717e56b27..66486b6ba 100644 --- a/static/templates/error.html +++ b/static/templates/error.html @@ -6,6 +6,7 @@ {{#CSS_URL}} {{/CSS_URL}}{{#KIWIX_RESPONSE_DATA}} {{/KIWIX_RESPONSE_DATA}} diff --git a/test/response.cpp b/test/response.cpp index 5762f8319..61f92ab75 100644 --- a/test/response.cpp +++ b/test/response.cpp @@ -46,6 +46,7 @@ R"( Content not found {{/KIWIX_RESPONSE_DATA}}\n \n \n

{{PAGE_HEADING}}

\n{{#details}}\n

\n {{{p}}}\n

\n{{/details}}\n \n\n"; const KIWIX_RESPONSE_DATA = { "CSS_URL" : "/css/error.css", "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "asdf", "SEARCH_URL" : "/search?q=asdf" } } } ] }; @@ -85,6 +86,7 @@ R"( [I18N TESTING] Not Found - Try Again {{/KIWIX_RESPONSE_DATA}}\n \n \n

{{PAGE_HEADING}}

\n{{#details}}\n

\n {{{p}}}\n

\n{{/details}}\n \n\n"; const KIWIX_RESPONSE_DATA = { "CSS_URL" : "/css/error.css", "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "asdf", "SEARCH_URL" : "/search?q=asdf" } } } ] }; From bceba4da066b16683a56423c52615d4d7e7ff957 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 6 Jan 2024 18:49:02 +0400 Subject: [PATCH 21/28] HTML-template data is HTML-encoded Non-HTML-encoded HTML-template data causes problems in HTML even when it appears inside JS string (resulting in the appearing inside a JS string). Besides, the KIWIX_RESPONSE_DATA and KIWIX_RESPONSE_TEMPLATE variables are set on the window object so that they can be accessed from the top context. This commit eliminates the need for the `escapeQuote` parameter in `escapeForJSON()` (that was introduced earlier in this PR) since now it is set to false in all call contexts. However from the consistency point of view, the default and intuitive behaviour of `escapeForJSON()` should be to escape the quote symbols, which justifies the existence of that parameter. --- src/server/response.cpp | 2 +- static/templates/error.html | 4 ++-- test/response.cpp | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index df3dc10ef..09dec8f7d 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -305,7 +305,7 @@ std::unique_ptr ContentResponseBlueprint::generateResponseObjec { kainjow::mustache::data d = m_data->toMustache(m_request.get_user_language()); if ( m_includeKiwixResponseData ) { - d.set("KIWIX_RESPONSE_TEMPLATE", escapeForJSON(m_template)); + d.set("KIWIX_RESPONSE_TEMPLATE", escapeForJSON(m_template, false)); d.set("KIWIX_RESPONSE_DATA", m_data->asJSON()); } auto r = ContentResponse::build(m_template, d, m_mimeType); diff --git a/static/templates/error.html b/static/templates/error.html index 66486b6ba..13fadc24c 100644 --- a/static/templates/error.html +++ b/static/templates/error.html @@ -6,8 +6,8 @@ {{#CSS_URL}} {{/CSS_URL}}{{#KIWIX_RESPONSE_DATA}} {{/KIWIX_RESPONSE_DATA}} diff --git a/test/response.cpp b/test/response.cpp index 61f92ab75..ca0ece39a 100644 --- a/test/response.cpp +++ b/test/response.cpp @@ -46,8 +46,8 @@ R"( Content not found {{/KIWIX_RESPONSE_DATA}}\n \n \n

{{PAGE_HEADING}}

\n{{#details}}\n

\n {{{p}}}\n

\n{{/details}}\n \n\n"; - const KIWIX_RESPONSE_DATA = { "CSS_URL" : "/css/error.css", "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "asdf", "SEARCH_URL" : "/search?q=asdf" } } } ] }; + window.KIWIX_RESPONSE_TEMPLATE = "<!DOCTYPE html>\n<html xmlns="http://www.w3.org/1999/xhtml">\n <head>\n <meta content="text/html;charset=UTF-8" http-equiv="content-type" />\n <title>{{PAGE_TITLE}}</title>\n{{#CSS_URL}}\n <link type="text/css" href="{{{CSS_URL}}}" rel="Stylesheet" />\n{{/CSS_URL}}{{#KIWIX_RESPONSE_DATA}} <script>\n window.KIWIX_RESPONSE_TEMPLATE = "{{KIWIX_RESPONSE_TEMPLATE}}";\n window.KIWIX_RESPONSE_DATA = {{{KIWIX_RESPONSE_DATA}}};\n </script>{{/KIWIX_RESPONSE_DATA}}\n </head>\n <body>\n <h1>{{PAGE_HEADING}}</h1>\n{{#details}}\n <p>\n {{{p}}}\n </p>\n{{/details}}\n </body>\n</html>\n"; + window.KIWIX_RESPONSE_DATA = { "CSS_URL" : "/css/error.css", "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "asdf", "SEARCH_URL" : "/search?q=asdf" } } } ] }; @@ -86,8 +86,8 @@ R"( [I18N TESTING] Not Found - Try Again {{/KIWIX_RESPONSE_DATA}}\n \n \n

{{PAGE_HEADING}}

\n{{#details}}\n

\n {{{p}}}\n

\n{{/details}}\n \n\n"; - const KIWIX_RESPONSE_DATA = { "CSS_URL" : "/css/error.css", "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "asdf", "SEARCH_URL" : "/search?q=asdf" } } } ] }; + window.KIWIX_RESPONSE_TEMPLATE = "<!DOCTYPE html>\n<html xmlns="http://www.w3.org/1999/xhtml">\n <head>\n <meta content="text/html;charset=UTF-8" http-equiv="content-type" />\n <title>{{PAGE_TITLE}}</title>\n{{#CSS_URL}}\n <link type="text/css" href="{{{CSS_URL}}}" rel="Stylesheet" />\n{{/CSS_URL}}{{#KIWIX_RESPONSE_DATA}} <script>\n window.KIWIX_RESPONSE_TEMPLATE = "{{KIWIX_RESPONSE_TEMPLATE}}";\n window.KIWIX_RESPONSE_DATA = {{{KIWIX_RESPONSE_DATA}}};\n </script>{{/KIWIX_RESPONSE_DATA}}\n </head>\n <body>\n <h1>{{PAGE_HEADING}}</h1>\n{{#details}}\n <p>\n {{{p}}}\n </p>\n{{/details}}\n </body>\n</html>\n"; + window.KIWIX_RESPONSE_DATA = { "CSS_URL" : "/css/error.css", "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "asdf", "SEARCH_URL" : "/search?q=asdf" } } } ] }; From 103a4516db93187b2c569b5b2a94d74f87beec96 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 6 Jan 2024 18:57:03 +0400 Subject: [PATCH 22/28] Demo of error page translation This commit demonstrates front-end-side translation of an error page for a URL like /viewer#INVALIDBOOK/whatever (where INVALIDBOOK should be a book name NOT present in the library). Known issues: - This change breaks a couple of subtests in the ServerTest.Http404HtmlError unit test. - Changing the UI language while an error page is displayed in the viewer doesn't retranslate it. --- src/server/internalServer.cpp | 2 +- src/server/response.cpp | 12 ++++++++---- src/server/response.h | 6 ++++-- static/skin/i18n.js | 32 ++++++++++++++++++++++++++++++++ static/skin/viewer.js | 21 +++++++++++++++++++++ test/server.cpp | 10 +++++----- 6 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index dcf6f170b..df709d008 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -1133,7 +1133,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r if (archive == nullptr) { const std::string searchURL = m_root + "/search?pattern=" + kiwix::urlEncode(pattern); - return UrlNotFoundResponse(request) + return UrlNotFoundResponse(request, true) + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } diff --git a/src/server/response.cpp b/src/server/response.cpp index 09dec8f7d..8d42ea715 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -334,16 +334,20 @@ HTTPErrorResponse::HTTPErrorResponse(const RequestContext& request, }); } -HTTP404Response::HTTP404Response(const RequestContext& request) +HTTP404Response::HTTP404Response(const RequestContext& request, + bool includeKiwixResponseData) : HTTPErrorResponse(request, MHD_HTTP_NOT_FOUND, "404-page-title", - "404-page-heading") + "404-page-heading", + std::string(), + includeKiwixResponseData) { } -UrlNotFoundResponse::UrlNotFoundResponse(const RequestContext& request) - : HTTP404Response(request) +UrlNotFoundResponse::UrlNotFoundResponse(const RequestContext& request, + bool includeKiwixResponseData) + : HTTP404Response(request, includeKiwixResponseData) { const std::string requestUrl = urlDecode(m_request.get_full_url(), false); *this += ParameterizedMessage("url-not-found", {{"url", requestUrl}}); diff --git a/src/server/response.h b/src/server/response.h index 11808f0da..a695efa84 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -160,12 +160,14 @@ struct HTTPErrorResponse : ContentResponseBlueprint struct HTTP404Response : HTTPErrorResponse { - explicit HTTP404Response(const RequestContext& request); + explicit HTTP404Response(const RequestContext& request, + bool includeKiwixResponseData = false); }; struct UrlNotFoundResponse : HTTP404Response { - explicit UrlNotFoundResponse(const RequestContext& request); + explicit UrlNotFoundResponse(const RequestContext& request, + bool includeKiwixResponseData = false); }; struct HTTP400Response : HTTPErrorResponse diff --git a/static/skin/i18n.js b/static/skin/i18n.js index 3ccf85c32..2a44cc500 100644 --- a/static/skin/i18n.js +++ b/static/skin/i18n.js @@ -69,6 +69,37 @@ function $t(msgId, params={}) { } } +const I18n = { + instantiateParameterizedMessages: function(data) { + if ( data.__proto__ == Array.prototype ) { + const result = []; + for ( const x of data ) { + result.push(this.instantiateParameterizedMessages(x)); + } + return result; + } else if ( data.__proto__ == Object.prototype ) { + const msgId = data.msgid; + const msgParams = data.params; + if ( msgId && msgId.__proto__ == String.prototype && msgParams && msgParams.__proto__ == Object.prototype ) { + return $t(msgId, msgParams); + } else { + const result = {}; + for ( const p in data ) { + result[p] = this.instantiateParameterizedMessages(data[p]); + } + return result; + } + } else { + return data; + } + }, + + render: function (template, params) { + params = this.instantiateParameterizedMessages(params); + return mustache.render(template, params); + } +} + const DEFAULT_UI_LANGUAGE = 'en'; Translations.load(DEFAULT_UI_LANGUAGE, /*asDefault=*/true); @@ -145,3 +176,4 @@ window.$t = $t; window.getUserLanguage = getUserLanguage; window.setUserLanguage = setUserLanguage; window.initUILanguageSelector = initUILanguageSelector; +window.I18n = I18n; diff --git a/static/skin/viewer.js b/static/skin/viewer.js index 23507109c..a311552db 100644 --- a/static/skin/viewer.js +++ b/static/skin/viewer.js @@ -249,6 +249,25 @@ function handle_location_hash_change() { history.replaceState(viewerState, null); } +function translateErrorPageIfNeeded() { + const cw = contentIframe.contentWindow; + if ( cw.KIWIX_RESPONSE_TEMPLATE && cw.KIWIX_RESPONSE_DATA ) { + const template = htmlDecode(cw.KIWIX_RESPONSE_TEMPLATE); + + // cw.KIWIX_RESPONSE_DATA belongs to the iframe context and running + // I18n.render() on it directly in the top context doesn't work correctly + // because the type checks (obj.__proto__ == ???.prototype) in + // I18n.instantiateParameterizedMessages() always fail (String.prototype + // refers to different objects in different contexts). + // Work arround that issue by copying the object into our context. + const params = JSON.parse(JSON.stringify(cw.KIWIX_RESPONSE_DATA)); + + const html = I18n.render(template, params); + const htmlDoc = new DOMParser().parseFromString(html, "text/html"); + cw.document.documentElement.innerHTML = htmlDoc.documentElement.innerHTML; + } +} + function handle_content_url_change() { const iframeLocation = contentIframe.contentWindow.location; console.log('handle_content_url_change: ' + iframeLocation.href); @@ -258,6 +277,7 @@ function handle_content_url_change() { const newHash = iframeUrl2UserUrl(iframeContentUrl, iframeContentQuery); history.replaceState(viewerState, null, makeURL(location.search, newHash)); updateCurrentBookIfNeeded(newHash); + translateErrorPageIfNeeded(); }; //////////////////////////////////////////////////////////////////////////////// @@ -496,6 +516,7 @@ function changeUILanguage() { viewerState.uiLanguage = lang; setUserLanguage(lang, () => { updateUIText(); + translateErrorPageIfNeeded(); history.pushState(viewerState, null); }); } diff --git a/test/server.cpp b/test/server.cpp index f9d09e509..5e7f11b65 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -59,7 +59,7 @@ const ResourceCollection resources200Compressible{ { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/autoComplete/css/autoComplete.css" }, { STATIC_CONTENT, "/ROOT%23%3F/skin/autoComplete/css/autoComplete.css?cacheid=ef30cd42" }, { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/i18n.js" }, - { STATIC_CONTENT, "/ROOT%23%3F/skin/i18n.js?cacheid=6a8c6fb2" }, + { STATIC_CONTENT, "/ROOT%23%3F/skin/i18n.js?cacheid=4ab55b42" }, { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/index.css" }, { STATIC_CONTENT, "/ROOT%23%3F/skin/index.css?cacheid=1e78e7cf" }, { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/index.js" }, @@ -75,7 +75,7 @@ const ResourceCollection resources200Compressible{ { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/taskbar.css" }, { STATIC_CONTENT, "/ROOT%23%3F/skin/taskbar.css?cacheid=e014a885" }, { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/viewer.js" }, - { STATIC_CONTENT, "/ROOT%23%3F/skin/viewer.js?cacheid=948df083" }, + { STATIC_CONTENT, "/ROOT%23%3F/skin/viewer.js?cacheid=e9c025f2" }, { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/fonts/Poppins.ttf" }, { STATIC_CONTENT, "/ROOT%23%3F/skin/fonts/Poppins.ttf?cacheid=af705837" }, { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/fonts/Roboto.ttf" }, @@ -285,7 +285,7 @@ R"EXPECTEDRESULT( href="/ROOT%23%3F/skin/kiwix.css?cacheid=2158fad9" - + @@ -318,9 +318,9 @@ R"EXPECTEDRESULT( - + - + const blankPageUrl = root + "/skin/blank.html?cacheid=6b1fa032"; From e1f067c086de26343aa5d8f3f3a0cfdb3bac503e Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 7 Jan 2024 17:33:41 +0400 Subject: [PATCH 23/28] Undid the demo of frontend-side error page translation This undoes frontend-side translation of the demo case with the purpose of having "clean" unit tests to support further work on this PR. --- src/server/internalServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index df709d008..dcf6f170b 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -1133,7 +1133,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r if (archive == nullptr) { const std::string searchURL = m_root + "/search?pattern=" + kiwix::urlEncode(pattern); - return UrlNotFoundResponse(request, true) + return UrlNotFoundResponse(request) + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } From bb1a730253671c8e1a36128eab815ee39edd9d0f Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 13 Jan 2024 18:26:17 +0400 Subject: [PATCH 24/28] Workaround for missing support for of std::variant std::variant is not supported by the old version of gcc used under aarch64. --- src/server/response.cpp | 73 +++++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 13 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 8d42ea715..b8605b681 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -34,7 +34,6 @@ #include #include #include -#include // This is somehow a magic value. // If this value is too small, we will compress (and lost cpu time) too much @@ -156,6 +155,44 @@ std::unique_ptr Response::build_304(const ETag& etag) return response; } + +namespace +{ + +// This class was introduced in order to work around the missing support +// for std::variant (and std::optional) under some of the current build +// platforms. +template +class Optional +{ +public: // functions + Optional() {} + Optional(const T& t) : ptr(new T(t)) {} + Optional(const Optional& o) : ptr(o.has_value() ? new T(*o) : nullptr) {} + Optional(Optional&& o) : ptr(std::move(o.ptr)) {} + + Optional& operator=(const Optional& o) + { + *this = Optional(o); + return *this; + } + + Optional& operator=(Optional&& o) + { + ptr = std::move(o.ptr); + return *this; + } + + bool has_value() const { return ptr.get() != nullptr; } + const T& operator*() const { return *ptr; } + T& operator*() { return *ptr; } + +private: // data + std::unique_ptr ptr; +}; + +} // unnamed namespace + class ContentResponseBlueprint::Data { public: @@ -163,20 +200,30 @@ class ContentResponseBlueprint::Data typedef std::map Object; private: - std::variant data; + // std::variant data; + // XXX: libkiwix is compiled on platforms where std::variant + // XXX: is not yet supported. Hence this hack. Only one + // XXX: of the below data members is expected to contain a value. + Optional m_stringValue; + Optional m_boolValue; + Optional m_listValue; + Optional m_objectValue; public: Data() {} - template Data(const T& t) : data(t) {} + Data(const std::string& s) : m_stringValue(s) {} + Data(bool b) : m_boolValue(b) {} + Data(const List& l) : m_listValue(l) {} + Data(const Object& o) : m_objectValue(o) {} MustacheData toMustache(const std::string& lang) const; Data& operator[](const std::string& key) { - return std::get(data)[key]; + return (*m_objectValue)[key]; } - void push_back(const Data& d) { std::get(data).push_back(d); } + void push_back(const Data& d) { (*m_listValue).push_back(d); } static Data onlyAsNonEmptyValue(const std::string& s) { @@ -199,14 +246,14 @@ class ContentResponseBlueprint::Data void dumpJSON(std::ostream& os) const; private: - bool isString() const { return std::holds_alternative(data); } - bool isList() const { return std::holds_alternative(data); } - bool isObject() const { return std::holds_alternative(data); } - - const std::string& stringValue() const { return std::get(data); } - bool boolValue() const { return std::get(data); } - const List& listValue() const { return std::get(data); } - const Object& objectValue() const { return std::get(data); } + bool isString() const { return m_stringValue.has_value(); } + bool isList() const { return m_listValue.has_value(); } + bool isObject() const { return m_objectValue.has_value(); } + + const std::string& stringValue() const { return *m_stringValue; } + bool boolValue() const { return *m_boolValue; } + const List& listValue() const { return *m_listValue; } + const Object& objectValue() const { return *m_objectValue; } const Data* get(const std::string& key) const { From 13a6863183b4f798cc9cefd8a573fcab772b6bdb Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 17 Jan 2024 16:49:02 +0400 Subject: [PATCH 25/28] Enabled frontend-side translation of 500 error page --- src/server/response.cpp | 4 +++- test/server.cpp | 5 ++++- test/server_testing_tools.h | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index b8605b681..8e4d9c71d 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -431,7 +431,9 @@ HTTP500Response::HTTP500Response(const RequestContext& request) : HTTPErrorResponse(request, MHD_HTTP_INTERNAL_SERVER_ERROR, "500-page-title", - "500-page-heading") + "500-page-heading", + std::string(), + /*includeKiwixResponseData=*/true) { *this += nonParameterizedMessage("500-page-text"); } diff --git a/test/server.cpp b/test/server.cpp index 5e7f11b65..fa57aee4e 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -1036,7 +1036,10 @@ TEST_F(ServerTest, 500) Internal Server Error - +

Internal Server Error

diff --git a/test/server_testing_tools.h b/test/server_testing_tools.h index 3ea10ab57..c7adbc565 100644 --- a/test/server_testing_tools.h +++ b/test/server_testing_tools.h @@ -190,3 +190,5 @@ class ServerTest : public ::testing::Test zfs1_.reset(); } }; + +static const std::string ERROR_HTML_TEMPLATE_JS_STRING = R"("<!DOCTYPE html>\n<html xmlns="http://www.w3.org/1999/xhtml">\n <head>\n <meta content="text/html;charset=UTF-8" http-equiv="content-type" />\n <title>{{PAGE_TITLE}}</title>\n{{#CSS_URL}}\n <link type="text/css" href="{{{CSS_URL}}}" rel="Stylesheet" />\n{{/CSS_URL}}{{#KIWIX_RESPONSE_DATA}} <script>\n window.KIWIX_RESPONSE_TEMPLATE = "{{KIWIX_RESPONSE_TEMPLATE}}";\n window.KIWIX_RESPONSE_DATA = {{{KIWIX_RESPONSE_DATA}}};\n </script>{{/KIWIX_RESPONSE_DATA}}\n </head>\n <body>\n <h1>{{PAGE_HEADING}}</h1>\n{{#details}}\n <p>\n {{{p}}}\n </p>\n{{/details}}\n </body>\n</html>\n")"; From 30b3f05497704b95d044115a5c894ae3b953d89f Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 17 Jan 2024 18:35:52 +0400 Subject: [PATCH 26/28] All kiwix-serve errors are now frontend-translatable But the question is do we need all of them to be translatable in the frontend? Maybe only responses to /random, /content and /search endpoints (that are displayed in the viewer) should be translatable? Also, the test cases against vulnerabilities in kiwix-serve seem to suggest that KIWIX_RESPONSE_DATA should be HTML-encoded too. --- src/server/internalServer.cpp | 3 +- src/server/response.cpp | 14 ++++---- src/server/response.h | 6 ++-- test/server.cpp | 63 ++++++++++++++++++++++++++++++----- test/server_search.cpp | 5 ++- 5 files changed, 70 insertions(+), 21 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index dcf6f170b..2c1ea518c 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -942,7 +942,8 @@ std::unique_ptr InternalServer::handle_search_request(const RequestCon HTTPErrorResponse response(request, MHD_HTTP_NOT_FOUND, "fulltext-search-unavailable", "404-page-heading", - cssUrl); + cssUrl, + /*includeKiwixResponseData=*/true); response += nonParameterizedMessage("no-search-results"); // XXX: Now this has to be handled by the iframe-based viewer which // XXX: has to resolve if the book selection resulted in a single book. diff --git a/src/server/response.cpp b/src/server/response.cpp index 8e4d9c71d..8ccf8c52a 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -381,20 +381,18 @@ HTTPErrorResponse::HTTPErrorResponse(const RequestContext& request, }); } -HTTP404Response::HTTP404Response(const RequestContext& request, - bool includeKiwixResponseData) +HTTP404Response::HTTP404Response(const RequestContext& request) : HTTPErrorResponse(request, MHD_HTTP_NOT_FOUND, "404-page-title", "404-page-heading", std::string(), - includeKiwixResponseData) + /*includeKiwixResponseData=*/true) { } -UrlNotFoundResponse::UrlNotFoundResponse(const RequestContext& request, - bool includeKiwixResponseData) - : HTTP404Response(request, includeKiwixResponseData) +UrlNotFoundResponse::UrlNotFoundResponse(const RequestContext& request) + : HTTP404Response(request) { const std::string requestUrl = urlDecode(m_request.get_full_url(), false); *this += ParameterizedMessage("url-not-found", {{"url", requestUrl}}); @@ -417,7 +415,9 @@ HTTP400Response::HTTP400Response(const RequestContext& request) : HTTPErrorResponse(request, MHD_HTTP_BAD_REQUEST, "400-page-title", - "400-page-heading") + "400-page-heading", + std::string(), + /*includeKiwixResponseData=*/true) { std::string requestUrl = urlDecode(m_request.get_full_url(), false); const auto query = m_request.get_query(); diff --git a/src/server/response.h b/src/server/response.h index a695efa84..11808f0da 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -160,14 +160,12 @@ struct HTTPErrorResponse : ContentResponseBlueprint struct HTTP404Response : HTTPErrorResponse { - explicit HTTP404Response(const RequestContext& request, - bool includeKiwixResponseData = false); + explicit HTTP404Response(const RequestContext& request); }; struct UrlNotFoundResponse : HTTP404Response { - explicit UrlNotFoundResponse(const RequestContext& request, - bool includeKiwixResponseData = false); + explicit UrlNotFoundResponse(const RequestContext& request); }; struct HTTP400Response : HTTPErrorResponse diff --git a/test/server.cpp b/test/server.cpp index fa57aee4e..cf7e0a46f 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -337,6 +337,7 @@ R"EXPECTEDRESULT( + window.KIWIX_RESPONSE_DATA = { "CSS_URL" : "/ROOT%23%3F/skin/search_results.css?cacheid=76d39c84", "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "fulltext-search-unavailable", "params" : { } }, "details" : [ { "p" : { "msgid" : "no-search-results", "params" : { } } } ] }; )EXPECTEDRESULT" }, }; @@ -535,6 +536,7 @@ struct ExpectedResponseData { const std::string expectedPageTitle; const std::string expectedCssUrl; + const std::string expectedKiwixResponseData; const std::string bookName; const std::string bookTitle; const std::string expectedBody; @@ -544,6 +546,7 @@ enum ExpectedResponseDataType { expected_page_title, expected_css_url, + expected_kiwix_response_data, book_name, book_title, expected_body @@ -556,11 +559,13 @@ ExpectedResponseData operator==(ExpectedResponseDataType t, std::string s) { switch (t) { - case expected_page_title: return ExpectedResponseData{s, "", "", "", ""}; - case expected_css_url: return ExpectedResponseData{"", s, "", "", ""}; - case book_name: return ExpectedResponseData{"", "", s, "", ""}; - case book_title: return ExpectedResponseData{"", "", "", s, ""}; - case expected_body: return ExpectedResponseData{"", "", "", "", s}; + case expected_page_title: return ExpectedResponseData{s, "", "", "", "", ""}; + case expected_css_url: return ExpectedResponseData{"", s, "", "", "", ""}; + case expected_kiwix_response_data: + return ExpectedResponseData{"", "", s, "", "", ""}; + case book_name: return ExpectedResponseData{"", "", "", s, "", ""}; + case book_title: return ExpectedResponseData{"", "", "", "", s, ""}; + case expected_body: return ExpectedResponseData{"", "", "", "", "", s}; default: assert(false); return ExpectedResponseData{}; } } @@ -579,6 +584,7 @@ ExpectedResponseData operator&&(const ExpectedResponseData& a, return ExpectedResponseData{ selectNonEmpty(a.expectedPageTitle, b.expectedPageTitle), selectNonEmpty(a.expectedCssUrl, b.expectedCssUrl), + selectNonEmpty(a.expectedKiwixResponseData, b.expectedKiwixResponseData), selectNonEmpty(a.bookName, b.bookName), selectNonEmpty(a.bookTitle, b.bookTitle), selectNonEmpty(a.expectedBody, b.expectedBody) @@ -607,19 +613,29 @@ class TestContentIn404HtmlResponse : public ExpectedResponseData std::string TestContentIn404HtmlResponse::expectedResponse() const { const std::string frag[] = { + // frag[0] R"FRAG( )FRAG", + // frag[1] R"FRAG( )FRAG", - R"FRAG( + // frag[2] + R"( )FRAG", + // frag[4] R"FRAG( )FRAG" @@ -630,8 +646,10 @@ std::string TestContentIn404HtmlResponse::expectedResponse() const + frag[1] + pageCssLink() + frag[2] + + expectedKiwixResponseData + + frag[3] + expectedBody - + frag[3]; + + frag[4]; } std::string TestContentIn404HtmlResponse::pageTitle() const @@ -648,7 +666,8 @@ std::string TestContentIn404HtmlResponse::pageCssLink() const return R"( )"; + + R"(" rel="Stylesheet" />)" + + "\n"; } class TestContentIn400HtmlResponse : public TestContentIn404HtmlResponse @@ -676,6 +695,7 @@ TEST_F(ServerTest, Http404HtmlError) using namespace TestingOfHtmlResponses; const std::vector testData{ { /* url */ "/ROOT%23%3F/random?content=non-existent-book", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "no-such-book", "params" : { "BOOK_NAME" : "non-existent-book" } } } ] })" && expected_body==R"(

Not Found

@@ -685,6 +705,7 @@ TEST_F(ServerTest, Http404HtmlError) { /* url */ "/ROOT%23%3F/random?content=non-existent-book&userlang=test", expected_page_title=="[I18N TESTING] Not Found - Try Again" && + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "no-such-book", "params" : { "BOOK_NAME" : "non-existent-book" } } } ] })" && expected_body==R"(

[I18N TESTING] Content not found, but at least the server is alive

@@ -693,6 +714,7 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ "/ROOT%23%3F/suggest?content=no-such-book&term=whatever", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "no-such-book", "params" : { "BOOK_NAME" : "no-such-book" } } } ] })" && expected_body==R"(

Not Found

@@ -701,6 +723,7 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ "/ROOT%23%3F/catalog/", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/catalog/" } } } ] })" && expected_body==R"(

Not Found

@@ -710,6 +733,7 @@ TEST_F(ServerTest, Http404HtmlError) { /* url */ "/ROOT%23%3F/catalog/?userlang=test", expected_page_title=="[I18N TESTING] Not Found - Try Again" && + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/catalog/" } } } ] })" && expected_body==R"(

[I18N TESTING] Content not found, but at least the server is alive

@@ -718,6 +742,7 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ "/ROOT%23%3F/catalog/invalid_endpoint", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/catalog/invalid_endpoint" } } } ] })" && expected_body==R"(

Not Found

@@ -727,6 +752,7 @@ TEST_F(ServerTest, Http404HtmlError) { /* url */ "/ROOT%23%3F/catalog/invalid_endpoint?userlang=test", expected_page_title=="[I18N TESTING] Not Found - Try Again" && + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/catalog/invalid_endpoint" } } } ] })" && expected_body==R"(

[I18N TESTING] Content not found, but at least the server is alive

@@ -735,6 +761,7 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ "/ROOT%23%3F/content/invalid-book/whatever", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/invalid-book/whatever" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "whatever", "SEARCH_URL" : "/ROOT%23%3F/search?pattern=whatever" } } } ] })" && expected_body==R"(

Not Found

@@ -748,6 +775,7 @@ TEST_F(ServerTest, Http404HtmlError) { /* url */ "/ROOT%23%3F/content/zimfile/invalid-article", book_name=="zimfile" && book_title=="Ray Charles" && + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/zimfile/invalid-article" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "invalid-article", "SEARCH_URL" : "/ROOT%23%3F/search?content=zimfile&pattern=invalid-article" } } } ] })" && expected_body==R"(

Not Found

@@ -759,6 +787,9 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ R"(/ROOT%23%3F/content/">)", + // XXX: This test case suggests that KIWIX_RESPONSE_DATA + // XXX: must be HTML-encoded, too + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/\">" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "\">", "SEARCH_URL" : "/ROOT%23%3F/search?pattern=%22%3E%3Csvg%20onload%3Dalert(1)%3E" } } } ] })" && expected_body==R"(

Not Found

@@ -772,6 +803,9 @@ TEST_F(ServerTest, Http404HtmlError) { /* url */ R"(/ROOT%23%3F/content/zimfile/">)", book_name=="zimfile" && book_title=="Ray Charles" && + // XXX: This test case suggests that KIWIX_RESPONSE_DATA + // XXX: must be HTML-encoded, too + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/zimfile/\">" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "\">", "SEARCH_URL" : "/ROOT%23%3F/search?content=zimfile&pattern=%22%3E%3Csvg%20onload%3Dalert(1)%3E" } } } ] })" && expected_body==R"(

Not Found

@@ -786,6 +820,7 @@ TEST_F(ServerTest, Http404HtmlError) expected_page_title=="[I18N TESTING] Not Found - Try Again" && book_name=="zimfile" && book_title=="Ray Charles" && + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/zimfile/invalid-article" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "invalid-article", "SEARCH_URL" : "/ROOT%23%3F/search?content=zimfile&pattern=invalid-article" } } } ] })" && expected_body==R"(

[I18N TESTING] Content not found, but at least the server is alive

@@ -797,6 +832,7 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ "/ROOT%23%3F/raw/no-such-book/meta/Title", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/raw/no-such-book/meta/Title" } } }, { "p" : { "msgid" : "no-such-book", "params" : { "BOOK_NAME" : "no-such-book" } } } ] })" && expected_body==R"(

Not Found

@@ -808,6 +844,7 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ "/ROOT%23%3F/raw/zimfile/XYZ", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/raw/zimfile/XYZ" } } }, { "p" : { "msgid" : "invalid-raw-data-type", "params" : { "DATATYPE" : "XYZ" } } } ] })" && expected_body==R"(

Not Found

@@ -819,6 +856,7 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ "/ROOT%23%3F/raw/zimfile/meta/invalid-metadata", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/raw/zimfile/meta/invalid-metadata" } } }, { "p" : { "msgid" : "raw-entry-not-found", "params" : { "DATATYPE" : "meta", "ENTRY" : "invalid-metadata" } } } ] })" && expected_body==R"(

Not Found

@@ -830,6 +868,7 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ "/ROOT%23%3F/raw/zimfile/content/invalid-article", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/raw/zimfile/content/invalid-article" } } }, { "p" : { "msgid" : "raw-entry-not-found", "params" : { "DATATYPE" : "content", "ENTRY" : "invalid-article" } } } ] })" && expected_body==R"(

Not Found

@@ -845,6 +884,7 @@ TEST_F(ServerTest, Http404HtmlError) expected_css_url=="/ROOT%23%3F/skin/search_results.css?cacheid=76d39c84" && book_name=="poor" && book_title=="poor" && + expected_kiwix_response_data==R"({ "CSS_URL" : "/ROOT%23%3F/skin/search_results.css?cacheid=76d39c84", "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "fulltext-search-unavailable", "params" : { } }, "details" : [ { "p" : { "msgid" : "no-search-results", "params" : { } } } ] })" && expected_body==R"(

Not Found

@@ -866,6 +906,7 @@ TEST_F(ServerTest, Http400HtmlError) using namespace TestingOfHtmlResponses; const std::vector testData{ { /* url */ "/ROOT%23%3F/search", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "400-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "400-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "invalid-request", "params" : { "url" : "/ROOT%23%3F/search" } } }, { "p" : { "msgid" : "too-many-books", "params" : { "LIMIT" : "3", "NB_BOOKS" : "4" } } } ] })" && expected_body== R"(

Invalid request

@@ -876,6 +917,7 @@ TEST_F(ServerTest, Http400HtmlError)

)" }, { /* url */ "/ROOT%23%3F/search?content=zimfile", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "400-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "400-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "invalid-request", "params" : { "url" : "/ROOT%23%3F/search?content=zimfile" } } }, { "p" : { "msgid" : "no-query", "params" : { } } } ] })" && expected_body==R"(

Invalid request

@@ -886,6 +928,7 @@ TEST_F(ServerTest, Http400HtmlError)

)" }, { /* url */ "/ROOT%23%3F/search?content=non-existing-book&pattern=asdfqwerty", + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "400-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "400-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "invalid-request", "params" : { "url" : "/ROOT%23%3F/search?content=non-existing-book&pattern=asdfqwerty" } } }, { "p" : { "msgid" : "no-such-book", "params" : { "BOOK_NAME" : "non-existing-book" } } } ] })" && expected_body==R"(

Invalid request

@@ -896,6 +939,7 @@ TEST_F(ServerTest, Http400HtmlError)

)" }, { /* url */ "/ROOT%23%3F/search?content=non-existing-book&pattern=a\"

Invalid request

From 1f9026f29501ce8557e35c952edf2b78dbf8a515 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 25 Jan 2024 15:02:43 +0400 Subject: [PATCH 27/28] "" inside KIWIX_RESPONSE_DATA is bad Added a test case demonstrating how a bad error response could be generated if appears inside KIWIX_RESPONSE_DATA. That seems to be the only problematic interaction between HTML-like syntax inside javascript code (hence the deleted XXX comments on the other two test cases). --- test/server.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/test/server.cpp b/test/server.cpp index cf7e0a46f..4a188619d 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -787,8 +787,6 @@ TEST_F(ServerTest, Http404HtmlError) )" }, { /* url */ R"(/ROOT%23%3F/content/">)", - // XXX: This test case suggests that KIWIX_RESPONSE_DATA - // XXX: must be HTML-encoded, too expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/\">" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "\">", "SEARCH_URL" : "/ROOT%23%3F/search?pattern=%22%3E%3Csvg%20onload%3Dalert(1)%3E" } } } ] })" && expected_body==R"(

Not Found

@@ -803,8 +801,6 @@ TEST_F(ServerTest, Http404HtmlError) { /* url */ R"(/ROOT%23%3F/content/zimfile/">)", book_name=="zimfile" && book_title=="Ray Charles" && - // XXX: This test case suggests that KIWIX_RESPONSE_DATA - // XXX: must be HTML-encoded, too expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/zimfile/\">" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "\">", "SEARCH_URL" : "/ROOT%23%3F/search?content=zimfile&pattern=%22%3E%3Csvg%20onload%3Dalert(1)%3E" } } } ] })" && expected_body==R"(

Not Found

@@ -816,6 +812,22 @@ TEST_F(ServerTest, Http404HtmlError)

)" }, + // XXX: This test case is against a "" string appearing inside + // XXX: javascript code that will confuse the HTML parser + { /* url */ R"(/ROOT%23%3F/content/zimfile/)", + book_name=="zimfile" && + book_title=="Ray Charles" && + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/zimfile/" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "script>", "SEARCH_URL" : "/ROOT%23%3F/search?content=zimfile&pattern=script%3E" } } } ] })" && + expected_body==R"( +

Not Found

+

+ The requested URL "/ROOT%23%3F/content/zimfile/</script>" was not found on this server. +

+

+ Make a full text search for script> +

+)" }, + { /* url */ "/ROOT%23%3F/content/zimfile/invalid-article?userlang=test", expected_page_title=="[I18N TESTING] Not Found - Try Again" && book_name=="zimfile" && From dc3960c5f89a5bb8711d184fadccc167cd2f6ba8 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 25 Jan 2024 15:37:31 +0400 Subject: [PATCH 28/28] Fix against a malicious "" in KIWIX_RESPONSE_DATA --- src/server/response.cpp | 10 +++++++++- test/server.cpp | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 8ccf8c52a..7520ab1cd 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -34,6 +34,7 @@ #include #include #include +#include // This is somehow a magic value. // If this value is too small, we will compress (and lost cpu time) too much @@ -330,7 +331,14 @@ std::string ContentResponseBlueprint::Data::asJSON() const { std::ostringstream oss; this->dumpJSON(oss); - return oss.str(); + + // This JSON is going to be used in HTML inside a tag. + // If it contains "" (or "") as a substring, then the HTML + // parser will be confused. Since for a valid JSON that may happen only inside + // a JSON string, we can safely take advantage of the answers to + // https://stackoverflow.com/questions/28259389/how-to-put-script-in-a-javascript-string + // and work around the issue by inserting an otherwise harmless backslash. + return std::regex_replace(oss.str(), std::regex(")", book_name=="zimfile" && book_title=="Ray Charles" && - expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/zimfile/" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "script>", "SEARCH_URL" : "/ROOT%23%3F/search?content=zimfile&pattern=script%3E" } } } ] })" && + expected_kiwix_response_data==R"({ "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "404-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "404-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "url-not-found", "params" : { "url" : "/ROOT%23%3F/content/zimfile/" } } }, { "p" : { "msgid" : "suggest-search", "params" : { "PATTERN" : "script>", "SEARCH_URL" : "/ROOT%23%3F/search?content=zimfile&pattern=script%3E" } } } ] })" && expected_body==R"(

Not Found