From 3188b0afe63014bddfc53978a0f3bf48ccd88fed Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 14:18:06 +0400 Subject: [PATCH 01/13] Translated a hard-coded error message --- src/server/response.cpp | 3 +-- static/skin/i18n/en.json | 1 + static/skin/i18n/qqq.json | 1 + static/skin/i18n/test.json | 2 ++ test/server.cpp | 13 +++++++++++++ 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index bc8c85e51..5399cfc58 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -239,8 +239,7 @@ HTTPErrorResponse& HTTP400Response::operator+(InvalidUrlMsg /*unused*/) if (!query.empty()) { requestUrl += "?" + encodeDiples(query); } - kainjow::mustache::mustache msgTmpl(R"(The requested URL "{{{url}}}" is not a valid request.)"); - return *this + msgTmpl.render({"url", requestUrl}); + return *this + ParameterizedMessage("invalid-request", {{"url", requestUrl}}); } HTTP500Response::HTTP500Response(const InternalServer& server, diff --git a/static/skin/i18n/en.json b/static/skin/i18n/en.json index aaea04978..97a71d558 100644 --- a/static/skin/i18n/en.json +++ b/static/skin/i18n/en.json @@ -12,6 +12,7 @@ , "suggest-search" : "Make a full text search for {{PATTERN}}" , "random-article-failure" : "Oops! Failed to pick a random article :(" , "invalid-raw-data-type" : "{{DATATYPE}} is not a valid request for raw content." + , "invalid-request" : "The requested URL \"{{{url}}}\" is not a valid request." , "no-value-for-arg": "No value provided for argument {{ARGUMENT}}" , "no-query" : "No query provided." , "raw-entry-not-found" : "Cannot find {{DATATYPE}} entry {{ENTRY}}" diff --git a/static/skin/i18n/qqq.json b/static/skin/i18n/qqq.json index 18c97a609..d647a75e4 100644 --- a/static/skin/i18n/qqq.json +++ b/static/skin/i18n/qqq.json @@ -15,6 +15,7 @@ "suggest-search": "Suggest a search when the URL points to a non existing article", "random-article-failure": "Failure of the random article selection procedure", "invalid-raw-data-type": "Invalid DATATYPE was used with the /raw endpoint (/raw//DATATYPE/...); allowed values are 'meta' and 'content'", + "invalid-request" : "Error text for malformed URLs.", "no-value-for-arg": "Error text when no value has been provided for ARGUMENT in the request's query string", "no-query": "Error text when no query has been provided for fulltext search", "raw-entry-not-found": "Entry requested via the /raw endpoint was not found", diff --git a/static/skin/i18n/test.json b/static/skin/i18n/test.json index 7686471ab..29def8eac 100644 --- a/static/skin/i18n/test.json +++ b/static/skin/i18n/test.json @@ -40,4 +40,6 @@ , "download-links-heading": "[I18N] Download links for {{BOOK_TITLE}} [TESTING]" , "download-links-title": "[I18N TESTING]Download book" , "preview-book": "[I18N] Preview [TESTING]" + , "no-query" : "[I18N TESTING] Kiwix can read your thoughts but it is against GDPR. Please provide your query explicitly." + , "invalid-request" : "[I18N TESTING] Invalid URL: \"{{{url}}}\"" } diff --git a/test/server.cpp b/test/server.cpp index 73ba7b1ff..86171cd4a 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -927,6 +927,19 @@ TEST_F(ServerTest, Http400HtmlError) Too many books requested (4) where limit is 3

)" }, + + // Testing of translation + { /* url */ "/ROOT%23%3F/search?content=zimfile&userlang=test", + expected_page_title=="[I18N TESTING] Invalid request ($400 fine must be paid)" && + expected_body==R"( +

[I18N TESTING] -400 karma for an invalid request

+

+ [I18N TESTING] Invalid URL: "/ROOT%23%3F/search?content=zimfile&userlang=test" +

+

+ [I18N TESTING] Kiwix can read your thoughts but it is against GDPR. Please provide your query explicitly. +

+)" }, }; for ( const auto& t : testData ) { From 41f25083dae9f8316d68261122922f6e07057689 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 14:31:38 +0400 Subject: [PATCH 02/13] Replaced UrlNotFoundMsg with UrlNotFoundResponse --- src/server/internalServer.cpp | 45 +++++++++------------------ src/server/internalServer_catalog.cpp | 18 ++++------- src/server/response.cpp | 7 +++-- src/server/response.h | 11 ++++--- 4 files changed, 31 insertions(+), 50 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index c8446cb17..cdfa4debe 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -593,8 +593,7 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r { try { if (! request.is_valid_url()) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } if ( request.get_url() == "" ) { @@ -697,8 +696,7 @@ std::unique_ptr InternalServer::handle_suggest(const RequestContext& r } if ( startsWith(request.get_url(), "/suggest/") ) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } std::string bookName, bookId; @@ -817,12 +815,10 @@ std::unique_ptr InternalServer::handle_no_js(const RequestContext& req const auto bookId = mp_nameMapper->getIdForName(urlParts[2]); content = getNoJSDownloadPageHTML(bookId, userLang); } catch (const std::out_of_range&) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } } else { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } return ContentResponse::build( @@ -873,8 +869,7 @@ std::unique_ptr InternalServer::handle_skin(const RequestContext& requ response->set_kind(accessType); return std::move(response); } catch (const ResourceNotFound& e) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } } @@ -892,8 +887,7 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re get_default_data(), "application/opensearchdescription+xml"); } - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } try { @@ -1001,8 +995,7 @@ std::unique_ptr InternalServer::handle_random(const RequestContext& re } if ( startsWith(request.get_url(), "/random/") ) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } std::string bookName; @@ -1037,8 +1030,7 @@ std::unique_ptr InternalServer::handle_captured_external(const Request } catch (const std::out_of_range& e) {} if (source.empty()) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } auto data = get_default_data(); @@ -1056,8 +1048,7 @@ std::unique_ptr InternalServer::handle_catch(const RequestContext& req return handle_captured_external(request); } - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } std::vector @@ -1141,8 +1132,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 HTTP404Response(*this, request) - + urlNotFoundMsg + return UrlNotFoundResponse(*this, request) + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } @@ -1189,8 +1179,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r printf("Failed to find %s\n", urlStr.c_str()); std::string searchURL = m_root + "/search?content=" + bookName + "&pattern=" + kiwix::urlEncode(pattern); - return HTTP404Response(*this, request) - + urlNotFoundMsg + return UrlNotFoundResponse(*this, request) + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } } @@ -1208,13 +1197,11 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque bookName = request.get_url_part(1); kind = request.get_url_part(2); } catch (const std::out_of_range& e) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } if (kind != "meta" && kind!= "content") { - return HTTP404Response(*this, request) - + urlNotFoundMsg + return UrlNotFoundResponse(*this, request) + invalidRawAccessMsg(kind); } @@ -1225,8 +1212,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque } catch (const std::out_of_range& e) {} if (archive == nullptr) { - return HTTP404Response(*this, request) - + urlNotFoundMsg + return UrlNotFoundResponse(*this, request) + noSuchBookErrorMsg(bookName); } @@ -1260,8 +1246,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque if (m_verbose.load()) { printf("Failed to find %s\n", itemPath.c_str()); } - return HTTP404Response(*this, request) - + urlNotFoundMsg + return UrlNotFoundResponse(*this, request) + rawEntryNotFoundMsg(kind, itemPath); } } diff --git a/src/server/internalServer_catalog.cpp b/src/server/internalServer_catalog.cpp index fa50450a9..7bda62792 100644 --- a/src/server/internalServer_catalog.cpp +++ b/src/server/internalServer_catalog.cpp @@ -63,8 +63,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r host = request.get_header("Host"); url = request.get_url_part(1); } catch (const std::out_of_range&) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } if (url == "v2") { @@ -72,8 +71,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r } if (url != "searchdescription.xml" && url != "root.xml" && url != "search") { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } if (url == "searchdescription.xml") { @@ -111,8 +109,7 @@ std::unique_ptr InternalServer::handle_catalog_v2(const RequestContext try { url = request.get_url_part(2); } catch (const std::out_of_range&) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } if (url == "root.xml") { @@ -138,8 +135,7 @@ std::unique_ptr InternalServer::handle_catalog_v2(const RequestContext } else if (url == "illustration") { return handle_catalog_v2_illustration(request); } else { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } } @@ -181,8 +177,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const try { mp_library->getBookById(entryId); } catch (const std::out_of_range&) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } OPDSDumper opdsDumper(mp_library.get(), mp_nameMapper.get()); @@ -233,8 +228,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_illustration(const R illustration->mimeType ); } catch(...) { - return HTTP404Response(*this, request) - + urlNotFoundMsg; + return UrlNotFoundResponse(*this, request); } } diff --git a/src/server/response.cpp b/src/server/response.cpp index 5399cfc58..f03ee5fcc 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -152,7 +152,6 @@ std::unique_ptr Response::build_304(const InternalServer& server, cons return response; } -const UrlNotFoundMsg urlNotFoundMsg; const InvalidUrlMsg invalidUrlMsg; std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const @@ -198,10 +197,12 @@ HTTP404Response::HTTP404Response(const InternalServer& server, { } -HTTPErrorResponse& HTTP404Response::operator+(UrlNotFoundMsg /*unused*/) +UrlNotFoundResponse::UrlNotFoundResponse(const InternalServer& server, + const RequestContext& request) + : HTTP404Response(server, request) { const std::string requestUrl = urlDecode(m_request.get_full_url(), false); - return *this + ParameterizedMessage("url-not-found", {{"url", requestUrl}}); + *this += ParameterizedMessage("url-not-found", {{"url", requestUrl}}); } HTTPErrorResponse& HTTPErrorResponse::operator+(const std::string& msg) diff --git a/src/server/response.h b/src/server/response.h index 4ed07e628..32702de45 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -174,17 +174,18 @@ struct HTTPErrorResponse : ContentResponseBlueprint HTTPErrorResponse& operator+=(const ParameterizedMessage& errorDetails); }; -class UrlNotFoundMsg {}; - -extern const UrlNotFoundMsg urlNotFoundMsg; - struct HTTP404Response : HTTPErrorResponse { HTTP404Response(const InternalServer& server, const RequestContext& request); using HTTPErrorResponse::operator+; - HTTPErrorResponse& operator+(UrlNotFoundMsg /*unused*/); +}; + +struct UrlNotFoundResponse : HTTP404Response +{ + UrlNotFoundResponse(const InternalServer& server, + const RequestContext& request); }; class InvalidUrlMsg {}; From a7ea908bcd47516f754a03e3eb36ac9dd0199075 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 15:29:23 +0400 Subject: [PATCH 03/13] HTTPErrorResponse no longer accepts std::strings --- src/server/i18n.h | 6 ++++++ src/server/internalServer.cpp | 10 ++-------- src/server/response.cpp | 13 ++++--------- src/server/response.h | 1 - static/skin/i18n/en.json | 3 +++ static/skin/i18n/qqq.json | 5 ++++- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/server/i18n.h b/src/server/i18n.h index e1cbcc68f..79005b721 100644 --- a/src/server/i18n.h +++ b/src/server/i18n.h @@ -111,6 +111,12 @@ private: // data const Parameters params; }; +inline ParameterizedMessage nonParameterizedMessage(const std::string& msgId) +{ + const ParameterizedMessage::Parameters noParams; + return ParameterizedMessage(msgId, noParams); +} + struct LangPreference { const std::string lang; diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index cdfa4debe..6f98b4095 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -190,12 +190,6 @@ ParameterizedMessage tooManyBooksMsg(size_t nbBooks, size_t limit) ); } -ParameterizedMessage nonParameterizedMessage(const std::string& msgId) -{ - const ParameterizedMessage::Parameters noParams; - return ParameterizedMessage(msgId, noParams); -} - struct Error : public std::runtime_error { explicit Error(const ParameterizedMessage& message) : std::runtime_error("Error while handling request"), @@ -650,11 +644,11 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r } catch (std::exception& e) { fprintf(stderr, "===== Unhandled error : %s\n", e.what()); return HTTP500Response(*this, request) - + e.what(); + + ParameterizedMessage("non-translated-text", {{"MSG", e.what()}}); } catch (...) { fprintf(stderr, "===== Unhandled unknown error\n"); return HTTP500Response(*this, request) - + "Unknown error"; + + nonParameterizedMessage("unknown-error"); } } diff --git a/src/server/response.cpp b/src/server/response.cpp index f03ee5fcc..36a2da244 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -205,15 +205,11 @@ UrlNotFoundResponse::UrlNotFoundResponse(const InternalServer& server, *this += ParameterizedMessage("url-not-found", {{"url", requestUrl}}); } -HTTPErrorResponse& HTTPErrorResponse::operator+(const std::string& msg) -{ - m_data["details"].push_back({"p", msg}); - return *this; -} - HTTPErrorResponse& HTTPErrorResponse::operator+(const ParameterizedMessage& details) { - return *this + details.getText(m_request.get_user_language()); + const std::string msg = details.getText(m_request.get_user_language()); + m_data["details"].push_back({"p", msg}); + return *this; } HTTPErrorResponse& HTTPErrorResponse::operator+=(const ParameterizedMessage& details) @@ -251,8 +247,7 @@ HTTP500Response::HTTP500Response(const InternalServer& server, "500-page-title", "500-page-heading") { - // operator+() is a state-modifying operator (akin to operator+=) - *this + "An internal server error occured. We are sorry about that :/"; + *this += nonParameterizedMessage("500-page-text"); } std::unique_ptr HTTP500Response::generateResponseObject() const diff --git a/src/server/response.h b/src/server/response.h index 32702de45..46ed4775f 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -169,7 +169,6 @@ struct HTTPErrorResponse : ContentResponseBlueprint const std::string& headingMsgId, const std::string& cssUrl = ""); - HTTPErrorResponse& operator+(const std::string& msg); HTTPErrorResponse& operator+(const ParameterizedMessage& errorDetails); HTTPErrorResponse& operator+=(const ParameterizedMessage& errorDetails); }; diff --git a/static/skin/i18n/en.json b/static/skin/i18n/en.json index 97a71d558..8c4a6b0bc 100644 --- a/static/skin/i18n/en.json +++ b/static/skin/i18n/en.json @@ -22,6 +22,7 @@ , "404-page-heading" : "Not Found" , "500-page-title" : "Internal Server Error" , "500-page-heading" : "Internal Server Error" + , "500-page-text": "An internal server error occured. We are sorry about that :/" , "fulltext-search-unavailable" : "Fulltext search unavailable" , "no-search-results": "The fulltext search engine is not available for this content." , "library-button-text": "Go to welcome page" @@ -52,4 +53,6 @@ , "download-links-heading": "Download links for {{BOOK_TITLE}}" , "download-links-title": "Download book" , "preview-book": "Preview" + , "non-translated-text": "{{MSG}}" + , "unknown-error": "Unknown error" } diff --git a/static/skin/i18n/qqq.json b/static/skin/i18n/qqq.json index d647a75e4..4fa2ef94e 100644 --- a/static/skin/i18n/qqq.json +++ b/static/skin/i18n/qqq.json @@ -25,6 +25,7 @@ "404-page-heading": "Heading of the 404 error page", "500-page-title": "Title of the 500 error page", "500-page-heading": "Heading of the 500 error page", + "500-page-text": "Text of the 500 error page", "fulltext-search-unavailable": "Title of the error page returned when search is attempted in a book without fulltext search database", "no-search-results": "Text of the error page returned when search is attempted in a book without fulltext search database", "library-button-text": "Tooltip of the button leading to the welcome page", @@ -53,5 +54,7 @@ "welcome-to-kiwix-server": "Title shown in browser's title bar/page tab", "download-links-heading": "Heading for no-js download page", "download-links-title": "Title for no-js download page", - "preview-book": "Tooltip of book-tile leading to the book" + "preview-book": "Tooltip of book-tile leading to the book", + "non-translated-text": "Used to display text that is generated at runtime and cannot be translated. Nothing to translate about this one.", + "unknown-error": "Unknown error" } From e470c97f74859c18b730e1fdbfd4bdad5fd836c2 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 15:42:21 +0400 Subject: [PATCH 04/13] Got rid of InvalidUrlMsg --- src/server/internalServer.cpp | 1 - src/server/response.cpp | 8 +------- src/server/response.h | 9 --------- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 6f98b4095..a2fa89fe8 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -888,7 +888,6 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re return handle_search_request(request); } catch (const Error& e) { return HTTP400Response(*this, request) - + invalidUrlMsg + e.message(); } } diff --git a/src/server/response.cpp b/src/server/response.cpp index 36a2da244..e3c501798 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -152,8 +152,6 @@ std::unique_ptr Response::build_304(const InternalServer& server, cons return response; } -const InvalidUrlMsg invalidUrlMsg; - std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const { return getTranslatedString(m_request.get_user_language(), msgId); @@ -226,17 +224,13 @@ HTTP400Response::HTTP400Response(const InternalServer& server, MHD_HTTP_BAD_REQUEST, "400-page-title", "400-page-heading") -{ -} - -HTTPErrorResponse& HTTP400Response::operator+(InvalidUrlMsg /*unused*/) { std::string requestUrl = urlDecode(m_request.get_full_url(), false); const auto query = m_request.get_query(); if (!query.empty()) { requestUrl += "?" + encodeDiples(query); } - return *this + ParameterizedMessage("invalid-request", {{"url", requestUrl}}); + *this += ParameterizedMessage("invalid-request", {{"url", requestUrl}}); } HTTP500Response::HTTP500Response(const InternalServer& server, diff --git a/src/server/response.h b/src/server/response.h index 46ed4775f..931bfb2f7 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -177,8 +177,6 @@ struct HTTP404Response : HTTPErrorResponse { HTTP404Response(const InternalServer& server, const RequestContext& request); - - using HTTPErrorResponse::operator+; }; struct UrlNotFoundResponse : HTTP404Response @@ -187,17 +185,10 @@ struct UrlNotFoundResponse : HTTP404Response const RequestContext& request); }; -class InvalidUrlMsg {}; - -extern const InvalidUrlMsg invalidUrlMsg; - struct HTTP400Response : HTTPErrorResponse { HTTP400Response(const InternalServer& server, const RequestContext& request); - - using HTTPErrorResponse::operator+; - HTTPErrorResponse& operator+(InvalidUrlMsg /*unused*/); }; struct HTTP500Response : HTTPErrorResponse From 3dce025f470cb958cb1d38c69dee77fd3c3578a1 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 17:16:23 +0400 Subject: [PATCH 05/13] Deleted an unused function --- src/server/response.h | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/server/response.h b/src/server/response.h index 931bfb2f7..6cde15f5b 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -136,14 +136,9 @@ public: // functions virtual ~ContentResponseBlueprint() = default; - operator std::unique_ptr() const - { - return generateResponseObject(); - } - operator std::unique_ptr() const { - return operator std::unique_ptr(); + return generateResponseObject(); } From f81a5a1a4ba98efaad2a1ed72f94bc24b3106b2a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 17:39:16 +0400 Subject: [PATCH 06/13] Moved verbosity control to Response::send() It makes little sense to pass the verbosity control to the `Response` constructor if it is used only in `Response::send()`. --- src/server/internalServer.cpp | 2 +- src/server/internalServer.h | 2 -- src/server/response.cpp | 21 +++++++++------------ src/server/response.h | 8 +++----- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index a2fa89fe8..8cf8d71c5 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -558,7 +558,7 @@ MHD_Result InternalServer::handlerCallback(struct MHD_Connection* connection, response->set_etag_body(getLibraryId()); } - auto ret = response->send(request, connection); + auto ret = response->send(request, m_verbose.load(), connection); auto end_time = std::chrono::steady_clock::now(); auto time_span = std::chrono::duration_cast>(end_time - start_time); if (m_verbose.load()) { diff --git a/src/server/internalServer.h b/src/server/internalServer.h index cdd7cebe2..5abd643bf 100644 --- a/src/server/internalServer.h +++ b/src/server/internalServer.h @@ -189,9 +189,7 @@ class InternalServer { class CustomizedResources; std::unique_ptr m_customizedResources; - friend std::unique_ptr Response::build(const InternalServer& server); friend std::unique_ptr ContentResponse::build(const InternalServer& server, const std::string& content, const std::string& mimetype); - friend std::unique_ptr ItemResponse::build(const InternalServer& server, const RequestContext& request, const zim::Item& item); }; } diff --git a/src/server/response.cpp b/src/server/response.cpp index e3c501798..f48de5d8a 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -119,9 +119,8 @@ const char* getCacheControlHeader(Response::Kind k) } // unnamed namespace -Response::Response(bool verbose) - : m_verbose(verbose), - m_returnCode(MHD_HTTP_OK) +Response::Response() + : m_returnCode(MHD_HTTP_OK) { add_header(MHD_HTTP_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN, "*"); } @@ -135,7 +134,7 @@ void Response::set_kind(Kind k) std::unique_ptr Response::build(const InternalServer& server) { - return std::unique_ptr(new Response(server.m_verbose.load())); + return std::unique_ptr(new Response()); } std::unique_ptr Response::build_304(const InternalServer& server, const ETag& etag) @@ -363,7 +362,7 @@ ContentResponse::create_mhd_response(const RequestContext& request) return response; } -MHD_Result Response::send(const RequestContext& request, MHD_Connection* connection) +MHD_Result Response::send(const RequestContext& request, bool verbose, MHD_Connection* connection) { MHD_Response* response = create_mhd_response(request); @@ -379,7 +378,7 @@ MHD_Result Response::send(const RequestContext& request, MHD_Connection* connect if (m_returnCode == MHD_HTTP_OK && m_byteRange.kind() == ByteRange::RESOLVED_PARTIAL_CONTENT) m_returnCode = MHD_HTTP_PARTIAL_CONTENT; - if (m_verbose) + if (verbose) print_response_info(m_returnCode, response); auto ret = MHD_queue_response(connection, m_returnCode, response); @@ -387,8 +386,8 @@ MHD_Result Response::send(const RequestContext& request, MHD_Connection* connect return ret; } -ContentResponse::ContentResponse(const std::string& root, bool verbose, const std::string& content, const std::string& mimetype) : - Response(verbose), +ContentResponse::ContentResponse(const std::string& root, const std::string& content, const std::string& mimetype) : + Response(), m_root(root), m_content(content), m_mimeType(mimetype) @@ -403,7 +402,6 @@ std::unique_ptr ContentResponse::build( { return std::unique_ptr(new ContentResponse( server.m_root, - server.m_verbose.load(), content, mimetype)); } @@ -418,8 +416,8 @@ std::unique_ptr ContentResponse::build( return ContentResponse::build(server, content, mimetype); } -ItemResponse::ItemResponse(bool verbose, const zim::Item& item, const std::string& mimetype, const ByteRange& byterange) : - Response(verbose), +ItemResponse::ItemResponse(const zim::Item& item, const std::string& mimetype, const ByteRange& byterange) : + Response(), m_item(item), m_mimeType(mimetype) { @@ -448,7 +446,6 @@ std::unique_ptr ItemResponse::build(const InternalServer& server, cons } return std::unique_ptr(new ItemResponse( - server.m_verbose.load(), item, mimetype, byteRange)); diff --git a/src/server/response.h b/src/server/response.h index 6cde15f5b..3adae56b1 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -54,7 +54,7 @@ class Response { }; public: - Response(bool verbose); + Response(); virtual ~Response() = default; static std::unique_ptr build(const InternalServer& server); @@ -62,7 +62,7 @@ class Response { static std::unique_ptr build_416(const InternalServer& server, size_t resourceLength); static std::unique_ptr build_redirect(const InternalServer& server, const std::string& redirectUrl); - MHD_Result send(const RequestContext& request, MHD_Connection* connection); + MHD_Result send(const RequestContext& request, bool verbose, MHD_Connection* connection); void set_code(int code) { m_returnCode = code; } void set_kind(Kind k); @@ -78,7 +78,6 @@ class Response { protected: // data Kind m_kind = DYNAMIC_CONTENT; - bool m_verbose; int m_returnCode; ByteRange m_byteRange; ETag m_etag; @@ -92,7 +91,6 @@ class ContentResponse : public Response { public: ContentResponse( const std::string& root, - bool verbose, const std::string& content, const std::string& mimetype); @@ -199,7 +197,7 @@ private: // overrides class ItemResponse : public Response { public: - ItemResponse(bool verbose, const zim::Item& item, const std::string& mimetype, const ByteRange& byterange); + ItemResponse(const zim::Item& item, const std::string& mimetype, const ByteRange& byterange); static std::unique_ptr build(const InternalServer& server, const RequestContext& request, const zim::Item& item); private: From 2d132d701e661a5ba1f66ec5fbc5860552c47661 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 17:48:05 +0400 Subject: [PATCH 07/13] Dropped the server param from Response::build*() --- src/server/internalServer.cpp | 14 +++++++------- src/server/response.cpp | 16 ++++++++-------- src/server/response.h | 8 ++++---- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 8cf8d71c5..f495447cd 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -594,12 +594,12 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r // Redirect /ROOT_LOCATION to /ROOT_LOCATION/ (note the added slash) // so that relative URLs are resolved correctly const std::string query = getSearchComponent(request); - return Response::build_redirect(*this, m_root + "/" + query); + return Response::build_redirect(m_root + "/" + query); } const ETag etag = get_matching_if_none_match_etag(request, getLibraryId()); if ( etag ) - return Response::build_304(*this, etag); + return Response::build_304(etag); const auto url = request.get_url(); if ( isLocallyCustomizedResource(url) ) @@ -640,7 +640,7 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r const std::string contentUrl = m_root + "/content" + urlEncode(url); const std::string query = getSearchComponent(request); - return Response::build_redirect(*this, contentUrl + query); + return Response::build_redirect(contentUrl + query); } catch (std::exception& e) { fprintf(stderr, "===== Unhandled error : %s\n", e.what()); return HTTP500Response(*this, request) @@ -1101,7 +1101,7 @@ InternalServer::build_redirect(const std::string& bookName, const zim::Item& ite { const auto contentPath = "/content/" + bookName + "/" + item.getPath(); const auto url = m_root + kiwix::urlEncode(contentPath); - return Response::build_redirect(*this, url); + return Response::build_redirect(url); } std::unique_ptr InternalServer::handle_content(const RequestContext& request) @@ -1132,7 +1132,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r const std::string archiveUuid(archive->getUuid()); const ETag etag = get_matching_if_none_match_etag(request, archiveUuid); if ( etag ) - return Response::build_304(*this, etag); + return Response::build_304(etag); auto urlStr = url.substr(prefixLength + bookName.size()); if (urlStr[0] == '/') { @@ -1212,7 +1212,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque const std::string archiveUuid(archive->getUuid()); const ETag etag = get_matching_if_none_match_etag(request, archiveUuid); if ( etag ) - return Response::build_304(*this, etag); + return Response::build_304(etag); // Remove the beggining of the path: // /raw///foo @@ -1264,7 +1264,7 @@ std::unique_ptr InternalServer::handle_locally_customized_resource(con auto byteRange = request.get_range().resolve(resourceData.size()); if (byteRange.kind() != ByteRange::RESOLVED_FULL_CONTENT) { - return Response::build_416(*this, resourceData.size()); + return Response::build_416(resourceData.size()); } return ContentResponse::build(*this, diff --git a/src/server/response.cpp b/src/server/response.cpp index f48de5d8a..c0ecabc79 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -132,14 +132,14 @@ void Response::set_kind(Kind k) m_etag.set_option(ETag::ZIM_CONTENT); } -std::unique_ptr Response::build(const InternalServer& server) +std::unique_ptr Response::build() { return std::unique_ptr(new Response()); } -std::unique_ptr Response::build_304(const InternalServer& server, const ETag& etag) +std::unique_ptr Response::build_304(const ETag& etag) { - auto response = Response::build(server); + auto response = Response::build(); response->set_code(MHD_HTTP_NOT_MODIFIED); response->m_etag = etag; if ( etag.get_option(ETag::ZIM_CONTENT) ) { @@ -251,9 +251,9 @@ std::unique_ptr HTTP500Response::generateResponseObject() const return r; } -std::unique_ptr Response::build_416(const InternalServer& server, size_t resourceLength) +std::unique_ptr Response::build_416(size_t resourceLength) { - auto response = Response::build(server); + auto response = Response::build(); // [FIXME] (compile with recent enough version of libmicrohttpd) // response->set_code(MHD_HTTP_RANGE_NOT_SATISFIABLE); response->set_code(416); @@ -265,9 +265,9 @@ std::unique_ptr Response::build_416(const InternalServer& server, size } -std::unique_ptr Response::build_redirect(const InternalServer& server, const std::string& redirectUrl) +std::unique_ptr Response::build_redirect(const std::string& redirectUrl) { - auto response = Response::build(server); + auto response = Response::build(); response->m_returnCode = MHD_HTTP_FOUND; response->add_header(MHD_HTTP_HEADER_LOCATION, redirectUrl); return response; @@ -440,7 +440,7 @@ std::unique_ptr ItemResponse::build(const InternalServer& server, cons } if (byteRange.kind() == ByteRange::RESOLVED_UNSATISFIABLE) { - auto response = Response::build_416(server, item.getSize()); + auto response = Response::build_416(item.getSize()); response->set_kind(Response::ZIM_CONTENT); return response; } diff --git a/src/server/response.h b/src/server/response.h index 3adae56b1..197c3145b 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -57,10 +57,10 @@ class Response { Response(); virtual ~Response() = default; - static std::unique_ptr build(const InternalServer& server); - static std::unique_ptr build_304(const InternalServer& server, const ETag& etag); - static std::unique_ptr build_416(const InternalServer& server, size_t resourceLength); - static std::unique_ptr build_redirect(const InternalServer& server, const std::string& redirectUrl); + static std::unique_ptr build(); + static std::unique_ptr build_304(const ETag& etag); + static std::unique_ptr build_416(size_t resourceLength); + static std::unique_ptr build_redirect(const std::string& redirectUrl); MHD_Result send(const RequestContext& request, bool verbose, MHD_Connection* connection); From 22ea3106c59d85ce634a612ae6bec488ce5043ba Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 18:04:21 +0400 Subject: [PATCH 08/13] Passing only root location instead of the entire server --- src/server/internalServer.cpp | 70 +++++++++++++-------------- src/server/internalServer.h | 2 - src/server/internalServer_catalog.cpp | 30 ++++++------ src/server/response.cpp | 36 +++++++------- src/server/response.h | 23 +++++---- 5 files changed, 79 insertions(+), 82 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index f495447cd..4fb138e82 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -587,7 +587,7 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r { try { if (! request.is_valid_url()) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } if ( request.get_url() == "" ) { @@ -643,11 +643,11 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r return Response::build_redirect(contentUrl + query); } catch (std::exception& e) { fprintf(stderr, "===== Unhandled error : %s\n", e.what()); - return HTTP500Response(*this, request) + return HTTP500Response(m_root, request) + ParameterizedMessage("non-translated-text", {{"MSG", e.what()}}); } catch (...) { fprintf(stderr, "===== Unhandled unknown error\n"); - return HTTP500Response(*this, request) + return HTTP500Response(m_root, request) + nonParameterizedMessage("unknown-error"); } } @@ -661,7 +661,7 @@ MustacheData InternalServer::get_default_data() const std::unique_ptr InternalServer::build_homepage(const RequestContext& request) { - return ContentResponse::build(*this, m_indexTemplateString, get_default_data(), "text/html; charset=utf-8"); + return ContentResponse::build(m_root, m_indexTemplateString, get_default_data(), "text/html; charset=utf-8"); } /** @@ -690,7 +690,7 @@ std::unique_ptr InternalServer::handle_suggest(const RequestContext& r } if ( startsWith(request.get_url(), "/suggest/") ) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } std::string bookName, bookId; @@ -704,7 +704,7 @@ std::unique_ptr InternalServer::handle_suggest(const RequestContext& r } if (archive == nullptr) { - return HTTP404Response(*this, request) + return HTTP404Response(m_root, request) + noSuchBookErrorMsg(bookName); } @@ -739,7 +739,7 @@ std::unique_ptr InternalServer::handle_suggest(const RequestContext& r results.addFTSearchSuggestion(request.get_user_language(), queryString); } - return ContentResponse::build(*this, results.getJSON(), "application/json; charset=utf-8"); + return ContentResponse::build(m_root, results.getJSON(), "application/json; charset=utf-8"); } std::unique_ptr InternalServer::handle_viewer_settings(const RequestContext& request) @@ -754,7 +754,7 @@ std::unique_ptr InternalServer::handle_viewer_settings(const RequestCo {"enable_library_button", m_withLibraryButton ? "true" : "false" }, {"default_user_language", request.get_user_language() } }; - return ContentResponse::build(*this, RESOURCE::templates::viewer_settings_js, data, "application/javascript; charset=utf-8"); + return ContentResponse::build(m_root, RESOURCE::templates::viewer_settings_js, data, "application/javascript; charset=utf-8"); } std::string InternalServer::getNoJSDownloadPageHTML(const std::string& bookId, const std::string& userLang) const @@ -809,14 +809,14 @@ std::unique_ptr InternalServer::handle_no_js(const RequestContext& req const auto bookId = mp_nameMapper->getIdForName(urlParts[2]); content = getNoJSDownloadPageHTML(bookId, userLang); } catch (const std::out_of_range&) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } } else { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } return ContentResponse::build( - *this, + m_root, content, "text/html; charset=utf-8" ); @@ -857,13 +857,13 @@ std::unique_ptr InternalServer::handle_skin(const RequestContext& requ try { const auto accessType = staticResourceAccessType(request, resourceCacheId); auto response = ContentResponse::build( - *this, + m_root, getResource(resourceName), getMimeTypeForFile(resourceName)); response->set_kind(accessType); return std::move(response); } catch (const ResourceNotFound& e) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } } @@ -876,18 +876,18 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re if ( startsWith(request.get_url(), "/search/") ) { if (request.get_url() == "/search/searchdescription.xml") { return ContentResponse::build( - *this, + m_root, RESOURCE::ft_opensearchdescription_xml, get_default_data(), "application/opensearchdescription+xml"); } - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } try { return handle_search_request(request); } catch (const Error& e) { - return HTTP400Response(*this, request) + return HTTP400Response(m_root, request) + e.message(); } } @@ -929,7 +929,7 @@ std::unique_ptr InternalServer::handle_search_request(const RequestCon // Searcher->search will throw a runtime error if there is no valid xapian database to do the search. // (in case of zim file not containing a index) const auto cssUrl = renderUrl(m_root, RESOURCE::templates::url_of_search_results_css); - HTTPErrorResponse response(*this, request, MHD_HTTP_NOT_FOUND, + HTTPErrorResponse response(m_root, request, MHD_HTTP_NOT_FOUND, "fulltext-search-unavailable", "404-page-heading", cssUrl); @@ -959,13 +959,13 @@ std::unique_ptr InternalServer::handle_search_request(const RequestCon renderer.setPageLength(pageLength); if (request.get_requested_format() == "xml") { return ContentResponse::build( - *this, + m_root, renderer.getXml(*mp_nameMapper, mp_library.get()), "application/rss+xml; charset=utf-8" ); } auto response = ContentResponse::build( - *this, + m_root, renderer.getHtml(*mp_nameMapper, mp_library.get()), "text/html; charset=utf-8" ); @@ -988,7 +988,7 @@ std::unique_ptr InternalServer::handle_random(const RequestContext& re } if ( startsWith(request.get_url(), "/random/") ) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } std::string bookName; @@ -1002,7 +1002,7 @@ std::unique_ptr InternalServer::handle_random(const RequestContext& re } if (archive == nullptr) { - return HTTP404Response(*this, request) + return HTTP404Response(m_root, request) + noSuchBookErrorMsg(bookName); } @@ -1010,7 +1010,7 @@ std::unique_ptr InternalServer::handle_random(const RequestContext& re auto entry = archive->getRandomEntry(); return build_redirect(bookName, getFinalItem(*archive, entry)); } catch(zim::EntryNotFound& e) { - return HTTP404Response(*this, request) + return HTTP404Response(m_root, request) + nonParameterizedMessage("random-article-failure"); } } @@ -1023,12 +1023,12 @@ std::unique_ptr InternalServer::handle_captured_external(const Request } catch (const std::out_of_range& e) {} if (source.empty()) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } auto data = get_default_data(); data.set("source", source); - return ContentResponse::build(*this, RESOURCE::templates::captured_external_html, data, "text/html; charset=utf-8"); + return ContentResponse::build(m_root, RESOURCE::templates::captured_external_html, data, "text/html; charset=utf-8"); } std::unique_ptr InternalServer::handle_catch(const RequestContext& request) @@ -1041,7 +1041,7 @@ std::unique_ptr InternalServer::handle_catch(const RequestContext& req return handle_captured_external(request); } - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } std::vector @@ -1125,7 +1125,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(*this, request) + return UrlNotFoundResponse(m_root, request) + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } @@ -1151,7 +1151,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r // '-' namespaces, in which case that resource is returned instead. return build_redirect(bookName, getFinalItem(*archive, entry)); } - auto response = ItemResponse::build(*this, request, entry.getItem()); + auto response = ItemResponse::build(m_root, request, entry.getItem()); response->set_etag_body(archiveUuid); if ( !startsWith(entry.getItem().getMimetype(), "application/pdf") ) { @@ -1172,7 +1172,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r printf("Failed to find %s\n", urlStr.c_str()); std::string searchURL = m_root + "/search?content=" + bookName + "&pattern=" + kiwix::urlEncode(pattern); - return UrlNotFoundResponse(*this, request) + return UrlNotFoundResponse(m_root, request) + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } } @@ -1190,11 +1190,11 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque bookName = request.get_url_part(1); kind = request.get_url_part(2); } catch (const std::out_of_range& e) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } if (kind != "meta" && kind!= "content") { - return UrlNotFoundResponse(*this, request) + return UrlNotFoundResponse(m_root, request) + invalidRawAccessMsg(kind); } @@ -1205,7 +1205,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque } catch (const std::out_of_range& e) {} if (archive == nullptr) { - return UrlNotFoundResponse(*this, request) + return UrlNotFoundResponse(m_root, request) + noSuchBookErrorMsg(bookName); } @@ -1223,7 +1223,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque try { if (kind == "meta") { auto item = archive->getMetadataItem(itemPath); - auto response = ItemResponse::build(*this, request, item); + auto response = ItemResponse::build(m_root, request, item); response->set_etag_body(archiveUuid); return response; } else { @@ -1231,7 +1231,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque if (entry.isRedirect()) { return build_redirect(bookName, entry.getItem(true)); } - auto response = ItemResponse::build(*this, request, entry.getItem()); + auto response = ItemResponse::build(m_root, request, entry.getItem()); response->set_etag_body(archiveUuid); return response; } @@ -1239,7 +1239,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque if (m_verbose.load()) { printf("Failed to find %s\n", itemPath.c_str()); } - return UrlNotFoundResponse(*this, request) + return UrlNotFoundResponse(m_root, request) + rawEntryNotFoundMsg(kind, itemPath); } } @@ -1267,7 +1267,7 @@ std::unique_ptr InternalServer::handle_locally_customized_resource(con return Response::build_416(resourceData.size()); } - return ContentResponse::build(*this, + return ContentResponse::build(m_root, resourceData, crd.mimeType); } diff --git a/src/server/internalServer.h b/src/server/internalServer.h index 5abd643bf..53b39d986 100644 --- a/src/server/internalServer.h +++ b/src/server/internalServer.h @@ -188,8 +188,6 @@ class InternalServer { class CustomizedResources; std::unique_ptr m_customizedResources; - - friend std::unique_ptr ContentResponse::build(const InternalServer& server, const std::string& content, const std::string& mimetype); }; } diff --git a/src/server/internalServer_catalog.cpp b/src/server/internalServer_catalog.cpp index 7bda62792..b44e86b46 100644 --- a/src/server/internalServer_catalog.cpp +++ b/src/server/internalServer_catalog.cpp @@ -63,7 +63,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r host = request.get_header("Host"); url = request.get_url_part(1); } catch (const std::out_of_range&) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } if (url == "v2") { @@ -71,11 +71,11 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r } if (url != "searchdescription.xml" && url != "root.xml" && url != "search") { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } if (url == "searchdescription.xml") { - auto response = ContentResponse::build(*this, RESOURCE::opensearchdescription_xml, get_default_data(), "application/opensearchdescription+xml"); + auto response = ContentResponse::build(m_root, RESOURCE::opensearchdescription_xml, get_default_data(), "application/opensearchdescription+xml"); return std::move(response); } @@ -93,7 +93,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r } auto response = ContentResponse::build( - *this, + m_root, opdsDumper.dumpOPDSFeed(bookIdsToDump, request.get_query()), opdsMimeType[OPDS_ACQUISITION_FEED]); return std::move(response); @@ -109,14 +109,14 @@ std::unique_ptr InternalServer::handle_catalog_v2(const RequestContext try { url = request.get_url_part(2); } catch (const std::out_of_range&) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } if (url == "root.xml") { return handle_catalog_v2_root(request); } else if (url == "searchdescription.xml") { const std::string endpoint_root = m_root + "/catalog/v2"; - return ContentResponse::build(*this, + return ContentResponse::build(m_root, RESOURCE::catalog_v2_searchdescription_xml, kainjow::mustache::object({{"endpoint_root", endpoint_root}}), "application/opensearchdescription+xml" @@ -135,7 +135,7 @@ std::unique_ptr InternalServer::handle_catalog_v2(const RequestContext } else if (url == "illustration") { return handle_catalog_v2_illustration(request); } else { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } } @@ -143,7 +143,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_root(const RequestCo { const std::string libraryId = getLibraryId(); return ContentResponse::build( - *this, + m_root, RESOURCE::templates::catalog_v2_root_xml, kainjow::mustache::object{ {"date", gen_date_str()}, @@ -166,7 +166,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_entries(const Reques const auto bookIds = search_catalog(request, opdsDumper); const auto opdsFeed = opdsDumper.dumpOPDSFeedV2(bookIds, request.get_query(), partial); return ContentResponse::build( - *this, + m_root, opdsFeed, opdsMimeType[OPDS_ACQUISITION_FEED] ); @@ -177,7 +177,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const try { mp_library->getBookById(entryId); } catch (const std::out_of_range&) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } OPDSDumper opdsDumper(mp_library.get(), mp_nameMapper.get()); @@ -185,7 +185,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const opdsDumper.setLibraryId(getLibraryId()); const auto opdsFeed = opdsDumper.dumpOPDSCompleteEntry(entryId); return ContentResponse::build( - *this, + m_root, opdsFeed, opdsMimeType[OPDS_ENTRY] ); @@ -197,7 +197,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_categories(const Req opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); return ContentResponse::build( - *this, + m_root, opdsDumper.categoriesOPDSFeed(), opdsMimeType[OPDS_NAVIGATION_FEED] ); @@ -209,7 +209,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_languages(const Requ opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); return ContentResponse::build( - *this, + m_root, opdsDumper.languagesOPDSFeed(), opdsMimeType[OPDS_NAVIGATION_FEED] ); @@ -223,12 +223,12 @@ std::unique_ptr InternalServer::handle_catalog_v2_illustration(const R auto size = request.get_argument("size"); auto illustration = book.getIllustration(size); return ContentResponse::build( - *this, + m_root, illustration->getData(), illustration->mimeType ); } catch(...) { - return UrlNotFoundResponse(*this, request); + return UrlNotFoundResponse(m_root, request); } } diff --git a/src/server/response.cpp b/src/server/response.cpp index c0ecabc79..262bff890 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -158,18 +158,18 @@ std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const std::unique_ptr ContentResponseBlueprint::generateResponseObject() const { - auto r = ContentResponse::build(m_server, m_template, m_data, m_mimeType); + auto r = ContentResponse::build(m_root, m_template, m_data, m_mimeType); r->set_code(m_httpStatusCode); return r; } -HTTPErrorResponse::HTTPErrorResponse(const InternalServer& server, +HTTPErrorResponse::HTTPErrorResponse(const std::string& root, const RequestContext& request, int httpStatusCode, const std::string& pageTitleMsgId, const std::string& headingMsgId, const std::string& cssUrl) - : ContentResponseBlueprint(&server, + : ContentResponseBlueprint(root, &request, httpStatusCode, request.get_requested_format() == "html" ? "text/html; charset=utf-8" : "application/xml; charset=utf-8", @@ -184,9 +184,9 @@ HTTPErrorResponse::HTTPErrorResponse(const InternalServer& server, }; } -HTTP404Response::HTTP404Response(const InternalServer& server, +HTTP404Response::HTTP404Response(const std::string& root, const RequestContext& request) - : HTTPErrorResponse(server, + : HTTPErrorResponse(root, request, MHD_HTTP_NOT_FOUND, "404-page-title", @@ -194,9 +194,9 @@ HTTP404Response::HTTP404Response(const InternalServer& server, { } -UrlNotFoundResponse::UrlNotFoundResponse(const InternalServer& server, +UrlNotFoundResponse::UrlNotFoundResponse(const std::string& root, const RequestContext& request) - : HTTP404Response(server, request) + : HTTP404Response(root, request) { const std::string requestUrl = urlDecode(m_request.get_full_url(), false); *this += ParameterizedMessage("url-not-found", {{"url", requestUrl}}); @@ -216,9 +216,9 @@ HTTPErrorResponse& HTTPErrorResponse::operator+=(const ParameterizedMessage& det } -HTTP400Response::HTTP400Response(const InternalServer& server, +HTTP400Response::HTTP400Response(const std::string& root, const RequestContext& request) - : HTTPErrorResponse(server, + : HTTPErrorResponse(root, request, MHD_HTTP_BAD_REQUEST, "400-page-title", @@ -232,9 +232,9 @@ HTTP400Response::HTTP400Response(const InternalServer& server, *this += ParameterizedMessage("invalid-request", {{"url", requestUrl}}); } -HTTP500Response::HTTP500Response(const InternalServer& server, +HTTP500Response::HTTP500Response(const std::string& root, const RequestContext& request) - : HTTPErrorResponse(server, + : HTTPErrorResponse(root, request, MHD_HTTP_INTERNAL_SERVER_ERROR, "500-page-title", @@ -246,7 +246,7 @@ HTTP500Response::HTTP500Response(const InternalServer& server, std::unique_ptr HTTP500Response::generateResponseObject() const { const std::string mimeType = "text/html;charset=utf-8"; - auto r = ContentResponse::build(m_server, m_template, m_data, mimeType); + auto r = ContentResponse::build(m_root, m_template, m_data, mimeType); r->set_code(m_httpStatusCode); return r; } @@ -396,24 +396,24 @@ ContentResponse::ContentResponse(const std::string& root, const std::string& con } std::unique_ptr ContentResponse::build( - const InternalServer& server, + const std::string& root, const std::string& content, const std::string& mimetype) { return std::unique_ptr(new ContentResponse( - server.m_root, + root, content, mimetype)); } std::unique_ptr ContentResponse::build( - const InternalServer& server, + const std::string& root, const std::string& template_str, kainjow::mustache::data data, const std::string& mimetype) { auto content = render_template(template_str, data); - return ContentResponse::build(server, content, mimetype); + return ContentResponse::build(root, content, mimetype); } ItemResponse::ItemResponse(const zim::Item& item, const std::string& mimetype, const ByteRange& byterange) : @@ -426,14 +426,14 @@ ItemResponse::ItemResponse(const zim::Item& item, const std::string& mimetype, c add_header(MHD_HTTP_HEADER_CONTENT_TYPE, m_mimeType); } -std::unique_ptr ItemResponse::build(const InternalServer& server, const RequestContext& request, const zim::Item& item) +std::unique_ptr ItemResponse::build(const std::string& root, const RequestContext& request, const zim::Item& item) { const std::string mimetype = get_mime_type(item); auto byteRange = request.get_range().resolve(item.getSize()); const bool noRange = byteRange.kind() == ByteRange::RESOLVED_FULL_CONTENT; if (noRange && is_compressible_mime_type(mimetype)) { // Return a contentResponse - auto response = ContentResponse::build(server, item.getData(), mimetype); + auto response = ContentResponse::build(root, item.getData(), mimetype); response->set_kind(Response::ZIM_CONTENT); response->m_byteRange = byteRange; return std::move(response); diff --git a/src/server/response.h b/src/server/response.h index 197c3145b..428e65392 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -41,7 +41,6 @@ class Archive; namespace kiwix { -class InternalServer; class RequestContext; class Response { @@ -95,12 +94,12 @@ class ContentResponse : public Response { const std::string& mimetype); static std::unique_ptr build( - const InternalServer& server, + const std::string& root, const std::string& content, const std::string& mimetype); static std::unique_ptr build( - const InternalServer& server, + const std::string& root, const std::string& template_str, kainjow::mustache::data data, const std::string& mimetype); @@ -120,12 +119,12 @@ class ContentResponse : public Response { class ContentResponseBlueprint { public: // functions - ContentResponseBlueprint(const InternalServer* server, + ContentResponseBlueprint(const std::string& root, const RequestContext* request, int httpStatusCode, const std::string& mimeType, const std::string& templateStr) - : m_server(*server) + : m_root(root) , m_request(*request) , m_httpStatusCode(httpStatusCode) , m_mimeType(mimeType) @@ -145,7 +144,7 @@ protected: // functions virtual std::unique_ptr generateResponseObject() const; public: //data - const InternalServer& m_server; + const std::string m_root; const RequestContext& m_request; const int m_httpStatusCode; const std::string m_mimeType; @@ -155,7 +154,7 @@ public: //data struct HTTPErrorResponse : ContentResponseBlueprint { - HTTPErrorResponse(const InternalServer& server, + HTTPErrorResponse(const std::string& root, const RequestContext& request, int httpStatusCode, const std::string& pageTitleMsgId, @@ -168,25 +167,25 @@ struct HTTPErrorResponse : ContentResponseBlueprint struct HTTP404Response : HTTPErrorResponse { - HTTP404Response(const InternalServer& server, + HTTP404Response(const std::string& root, const RequestContext& request); }; struct UrlNotFoundResponse : HTTP404Response { - UrlNotFoundResponse(const InternalServer& server, + UrlNotFoundResponse(const std::string& root, const RequestContext& request); }; struct HTTP400Response : HTTPErrorResponse { - HTTP400Response(const InternalServer& server, + HTTP400Response(const std::string& root, const RequestContext& request); }; struct HTTP500Response : HTTPErrorResponse { - HTTP500Response(const InternalServer& server, + HTTP500Response(const std::string& root, const RequestContext& request); private: // overrides @@ -198,7 +197,7 @@ private: // overrides class ItemResponse : public Response { public: ItemResponse(const zim::Item& item, const std::string& mimetype, const ByteRange& byterange); - static std::unique_ptr build(const InternalServer& server, const RequestContext& request, const zim::Item& item); + static std::unique_ptr build(const std::string& root, const RequestContext& request, const zim::Item& item); private: MHD_Response* create_mhd_response(const RequestContext& request); From 6a651e04e53bad79516f6800ed80f694cf23817a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 18:19:33 +0400 Subject: [PATCH 09/13] 1st step in removing root from ContentResponse It turned out that ContentResponse::m_root is no longer used. At this point, the root parameter is dropped only from the 3-ary variant of ContentResponse::build(), so that its all call sites are automatically discovered by the compiler (and updated manually). Including the other (4-ary) variant of ContentResponse::build() in this change might result in the semantic change of expressions like `ContentResponse::build(x, y, z)` and failure to update them. --- src/server/internalServer.cpp | 15 +++------------ src/server/internalServer_catalog.cpp | 6 ------ src/server/response.cpp | 9 +++------ src/server/response.h | 3 --- 4 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 4fb138e82..4f4b986c6 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -739,7 +739,7 @@ std::unique_ptr InternalServer::handle_suggest(const RequestContext& r results.addFTSearchSuggestion(request.get_user_language(), queryString); } - return ContentResponse::build(m_root, results.getJSON(), "application/json; charset=utf-8"); + return ContentResponse::build(results.getJSON(), "application/json; charset=utf-8"); } std::unique_ptr InternalServer::handle_viewer_settings(const RequestContext& request) @@ -815,11 +815,7 @@ std::unique_ptr InternalServer::handle_no_js(const RequestContext& req return UrlNotFoundResponse(m_root, request); } - return ContentResponse::build( - m_root, - content, - "text/html; charset=utf-8" - ); + return ContentResponse::build(content, "text/html; charset=utf-8"); } namespace @@ -857,7 +853,6 @@ std::unique_ptr InternalServer::handle_skin(const RequestContext& requ try { const auto accessType = staticResourceAccessType(request, resourceCacheId); auto response = ContentResponse::build( - m_root, getResource(resourceName), getMimeTypeForFile(resourceName)); response->set_kind(accessType); @@ -959,13 +954,11 @@ std::unique_ptr InternalServer::handle_search_request(const RequestCon renderer.setPageLength(pageLength); if (request.get_requested_format() == "xml") { return ContentResponse::build( - m_root, renderer.getXml(*mp_nameMapper, mp_library.get()), "application/rss+xml; charset=utf-8" ); } auto response = ContentResponse::build( - m_root, renderer.getHtml(*mp_nameMapper, mp_library.get()), "text/html; charset=utf-8" ); @@ -1267,9 +1260,7 @@ std::unique_ptr InternalServer::handle_locally_customized_resource(con return Response::build_416(resourceData.size()); } - return ContentResponse::build(m_root, - resourceData, - crd.mimeType); + return ContentResponse::build(resourceData, crd.mimeType); } } diff --git a/src/server/internalServer_catalog.cpp b/src/server/internalServer_catalog.cpp index b44e86b46..edb1b327c 100644 --- a/src/server/internalServer_catalog.cpp +++ b/src/server/internalServer_catalog.cpp @@ -93,7 +93,6 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r } auto response = ContentResponse::build( - m_root, opdsDumper.dumpOPDSFeed(bookIdsToDump, request.get_query()), opdsMimeType[OPDS_ACQUISITION_FEED]); return std::move(response); @@ -166,7 +165,6 @@ std::unique_ptr InternalServer::handle_catalog_v2_entries(const Reques const auto bookIds = search_catalog(request, opdsDumper); const auto opdsFeed = opdsDumper.dumpOPDSFeedV2(bookIds, request.get_query(), partial); return ContentResponse::build( - m_root, opdsFeed, opdsMimeType[OPDS_ACQUISITION_FEED] ); @@ -185,7 +183,6 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const opdsDumper.setLibraryId(getLibraryId()); const auto opdsFeed = opdsDumper.dumpOPDSCompleteEntry(entryId); return ContentResponse::build( - m_root, opdsFeed, opdsMimeType[OPDS_ENTRY] ); @@ -197,7 +194,6 @@ std::unique_ptr InternalServer::handle_catalog_v2_categories(const Req opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); return ContentResponse::build( - m_root, opdsDumper.categoriesOPDSFeed(), opdsMimeType[OPDS_NAVIGATION_FEED] ); @@ -209,7 +205,6 @@ std::unique_ptr InternalServer::handle_catalog_v2_languages(const Requ opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); return ContentResponse::build( - m_root, opdsDumper.languagesOPDSFeed(), opdsMimeType[OPDS_NAVIGATION_FEED] ); @@ -223,7 +218,6 @@ std::unique_ptr InternalServer::handle_catalog_v2_illustration(const R auto size = request.get_argument("size"); auto illustration = book.getIllustration(size); return ContentResponse::build( - m_root, illustration->getData(), illustration->mimeType ); diff --git a/src/server/response.cpp b/src/server/response.cpp index 262bff890..04d158652 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -386,9 +386,8 @@ MHD_Result Response::send(const RequestContext& request, bool verbose, MHD_Conne return ret; } -ContentResponse::ContentResponse(const std::string& root, const std::string& content, const std::string& mimetype) : +ContentResponse::ContentResponse(const std::string& content, const std::string& mimetype) : Response(), - m_root(root), m_content(content), m_mimeType(mimetype) { @@ -396,12 +395,10 @@ ContentResponse::ContentResponse(const std::string& root, const std::string& con } std::unique_ptr ContentResponse::build( - const std::string& root, const std::string& content, const std::string& mimetype) { return std::unique_ptr(new ContentResponse( - root, content, mimetype)); } @@ -413,7 +410,7 @@ std::unique_ptr ContentResponse::build( const std::string& mimetype) { auto content = render_template(template_str, data); - return ContentResponse::build(root, content, mimetype); + return ContentResponse::build(content, mimetype); } ItemResponse::ItemResponse(const zim::Item& item, const std::string& mimetype, const ByteRange& byterange) : @@ -433,7 +430,7 @@ std::unique_ptr ItemResponse::build(const std::string& root, const Req const bool noRange = byteRange.kind() == ByteRange::RESOLVED_FULL_CONTENT; if (noRange && is_compressible_mime_type(mimetype)) { // Return a contentResponse - auto response = ContentResponse::build(root, item.getData(), mimetype); + auto response = ContentResponse::build(item.getData(), mimetype); response->set_kind(Response::ZIM_CONTENT); response->m_byteRange = byteRange; return std::move(response); diff --git a/src/server/response.h b/src/server/response.h index 428e65392..7c3f2cf3f 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -89,12 +89,10 @@ class Response { class ContentResponse : public Response { public: ContentResponse( - const std::string& root, const std::string& content, const std::string& mimetype); static std::unique_ptr build( - const std::string& root, const std::string& content, const std::string& mimetype); @@ -111,7 +109,6 @@ class ContentResponse : public Response { private: - std::string m_root; std::string m_content; std::string m_mimeType; }; From db3b76247fc576d9c58fdc0cc1ab9968f49431df Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 18:32:18 +0400 Subject: [PATCH 10/13] Last step of removing root from ContentResponse --- src/server/internalServer.cpp | 7 +++---- src/server/internalServer_catalog.cpp | 5 ++--- src/server/response.cpp | 5 ++--- src/server/response.h | 1 - 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 4f4b986c6..7c710b488 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -661,7 +661,7 @@ MustacheData InternalServer::get_default_data() const std::unique_ptr InternalServer::build_homepage(const RequestContext& request) { - return ContentResponse::build(m_root, m_indexTemplateString, get_default_data(), "text/html; charset=utf-8"); + return ContentResponse::build(m_indexTemplateString, get_default_data(), "text/html; charset=utf-8"); } /** @@ -754,7 +754,7 @@ std::unique_ptr InternalServer::handle_viewer_settings(const RequestCo {"enable_library_button", m_withLibraryButton ? "true" : "false" }, {"default_user_language", request.get_user_language() } }; - return ContentResponse::build(m_root, RESOURCE::templates::viewer_settings_js, data, "application/javascript; charset=utf-8"); + return ContentResponse::build(RESOURCE::templates::viewer_settings_js, data, "application/javascript; charset=utf-8"); } std::string InternalServer::getNoJSDownloadPageHTML(const std::string& bookId, const std::string& userLang) const @@ -871,7 +871,6 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re if ( startsWith(request.get_url(), "/search/") ) { if (request.get_url() == "/search/searchdescription.xml") { return ContentResponse::build( - m_root, RESOURCE::ft_opensearchdescription_xml, get_default_data(), "application/opensearchdescription+xml"); @@ -1021,7 +1020,7 @@ std::unique_ptr InternalServer::handle_captured_external(const Request auto data = get_default_data(); data.set("source", source); - return ContentResponse::build(m_root, RESOURCE::templates::captured_external_html, data, "text/html; charset=utf-8"); + return ContentResponse::build(RESOURCE::templates::captured_external_html, data, "text/html; charset=utf-8"); } std::unique_ptr InternalServer::handle_catch(const RequestContext& request) diff --git a/src/server/internalServer_catalog.cpp b/src/server/internalServer_catalog.cpp index edb1b327c..a1ec11100 100644 --- a/src/server/internalServer_catalog.cpp +++ b/src/server/internalServer_catalog.cpp @@ -75,7 +75,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r } if (url == "searchdescription.xml") { - auto response = ContentResponse::build(m_root, RESOURCE::opensearchdescription_xml, get_default_data(), "application/opensearchdescription+xml"); + auto response = ContentResponse::build(RESOURCE::opensearchdescription_xml, get_default_data(), "application/opensearchdescription+xml"); return std::move(response); } @@ -115,7 +115,7 @@ std::unique_ptr InternalServer::handle_catalog_v2(const RequestContext return handle_catalog_v2_root(request); } else if (url == "searchdescription.xml") { const std::string endpoint_root = m_root + "/catalog/v2"; - return ContentResponse::build(m_root, + return ContentResponse::build( RESOURCE::catalog_v2_searchdescription_xml, kainjow::mustache::object({{"endpoint_root", endpoint_root}}), "application/opensearchdescription+xml" @@ -142,7 +142,6 @@ std::unique_ptr InternalServer::handle_catalog_v2_root(const RequestCo { const std::string libraryId = getLibraryId(); return ContentResponse::build( - m_root, RESOURCE::templates::catalog_v2_root_xml, kainjow::mustache::object{ {"date", gen_date_str()}, diff --git a/src/server/response.cpp b/src/server/response.cpp index 04d158652..f4d1ea17c 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -158,7 +158,7 @@ std::string ContentResponseBlueprint::getMessage(const std::string& msgId) const std::unique_ptr ContentResponseBlueprint::generateResponseObject() const { - auto r = ContentResponse::build(m_root, m_template, m_data, m_mimeType); + auto r = ContentResponse::build(m_template, m_data, m_mimeType); r->set_code(m_httpStatusCode); return r; } @@ -246,7 +246,7 @@ HTTP500Response::HTTP500Response(const std::string& root, std::unique_ptr HTTP500Response::generateResponseObject() const { const std::string mimeType = "text/html;charset=utf-8"; - auto r = ContentResponse::build(m_root, m_template, m_data, mimeType); + auto r = ContentResponse::build(m_template, m_data, mimeType); r->set_code(m_httpStatusCode); return r; } @@ -404,7 +404,6 @@ std::unique_ptr ContentResponse::build( } std::unique_ptr ContentResponse::build( - const std::string& root, const std::string& template_str, kainjow::mustache::data data, const std::string& mimetype) diff --git a/src/server/response.h b/src/server/response.h index 7c3f2cf3f..f57ab9345 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -97,7 +97,6 @@ class ContentResponse : public Response { const std::string& mimetype); static std::unique_ptr build( - const std::string& root, const std::string& template_str, kainjow::mustache::data data, const std::string& mimetype); From 6e2be481fd82e24571c691e015e5d6fd7a6a9c33 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 18:34:45 +0400 Subject: [PATCH 11/13] Dropped the root param from ItemResponse::build() --- src/server/internalServer.cpp | 6 +++--- src/server/response.cpp | 2 +- src/server/response.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 7c710b488..8cfcebe26 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -1143,7 +1143,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r // '-' namespaces, in which case that resource is returned instead. return build_redirect(bookName, getFinalItem(*archive, entry)); } - auto response = ItemResponse::build(m_root, request, entry.getItem()); + auto response = ItemResponse::build(request, entry.getItem()); response->set_etag_body(archiveUuid); if ( !startsWith(entry.getItem().getMimetype(), "application/pdf") ) { @@ -1215,7 +1215,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque try { if (kind == "meta") { auto item = archive->getMetadataItem(itemPath); - auto response = ItemResponse::build(m_root, request, item); + auto response = ItemResponse::build(request, item); response->set_etag_body(archiveUuid); return response; } else { @@ -1223,7 +1223,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque if (entry.isRedirect()) { return build_redirect(bookName, entry.getItem(true)); } - auto response = ItemResponse::build(m_root, request, entry.getItem()); + auto response = ItemResponse::build(request, entry.getItem()); response->set_etag_body(archiveUuid); return response; } diff --git a/src/server/response.cpp b/src/server/response.cpp index f4d1ea17c..304a1b7e3 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -422,7 +422,7 @@ ItemResponse::ItemResponse(const zim::Item& item, const std::string& mimetype, c add_header(MHD_HTTP_HEADER_CONTENT_TYPE, m_mimeType); } -std::unique_ptr ItemResponse::build(const std::string& root, const RequestContext& request, const zim::Item& item) +std::unique_ptr ItemResponse::build(const RequestContext& request, const zim::Item& item) { const std::string mimetype = get_mime_type(item); auto byteRange = request.get_range().resolve(item.getSize()); diff --git a/src/server/response.h b/src/server/response.h index f57ab9345..4707ed8ea 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -193,7 +193,7 @@ private: // overrides class ItemResponse : public Response { public: ItemResponse(const zim::Item& item, const std::string& mimetype, const ByteRange& byterange); - static std::unique_ptr build(const std::string& root, const RequestContext& request, const zim::Item& item); + static std::unique_ptr build(const RequestContext& request, const zim::Item& item); private: MHD_Response* create_mhd_response(const RequestContext& request); From 7a85c920257051c4ad76212f09995eca88bff5b5 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 18:43:58 +0400 Subject: [PATCH 12/13] Dropped root from HTTPErrorResponse & friends --- src/server/internalServer.cpp | 44 +++++++++++++-------------- src/server/internalServer_catalog.cpp | 12 ++++---- src/server/response.cpp | 29 ++++++------------ src/server/response.h | 22 +++++--------- 4 files changed, 45 insertions(+), 62 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 8cfcebe26..72b71b044 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -587,7 +587,7 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r { try { if (! request.is_valid_url()) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } if ( request.get_url() == "" ) { @@ -643,11 +643,11 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r return Response::build_redirect(contentUrl + query); } catch (std::exception& e) { fprintf(stderr, "===== Unhandled error : %s\n", e.what()); - return HTTP500Response(m_root, request) + return HTTP500Response(request) + ParameterizedMessage("non-translated-text", {{"MSG", e.what()}}); } catch (...) { fprintf(stderr, "===== Unhandled unknown error\n"); - return HTTP500Response(m_root, request) + return HTTP500Response(request) + nonParameterizedMessage("unknown-error"); } } @@ -690,7 +690,7 @@ std::unique_ptr InternalServer::handle_suggest(const RequestContext& r } if ( startsWith(request.get_url(), "/suggest/") ) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } std::string bookName, bookId; @@ -704,7 +704,7 @@ std::unique_ptr InternalServer::handle_suggest(const RequestContext& r } if (archive == nullptr) { - return HTTP404Response(m_root, request) + return HTTP404Response(request) + noSuchBookErrorMsg(bookName); } @@ -809,10 +809,10 @@ std::unique_ptr InternalServer::handle_no_js(const RequestContext& req const auto bookId = mp_nameMapper->getIdForName(urlParts[2]); content = getNoJSDownloadPageHTML(bookId, userLang); } catch (const std::out_of_range&) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } } else { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } return ContentResponse::build(content, "text/html; charset=utf-8"); @@ -858,7 +858,7 @@ std::unique_ptr InternalServer::handle_skin(const RequestContext& requ response->set_kind(accessType); return std::move(response); } catch (const ResourceNotFound& e) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } } @@ -875,13 +875,13 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re get_default_data(), "application/opensearchdescription+xml"); } - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } try { return handle_search_request(request); } catch (const Error& e) { - return HTTP400Response(m_root, request) + return HTTP400Response(request) + e.message(); } } @@ -923,7 +923,7 @@ std::unique_ptr InternalServer::handle_search_request(const RequestCon // Searcher->search will throw a runtime error if there is no valid xapian database to do the search. // (in case of zim file not containing a index) const auto cssUrl = renderUrl(m_root, RESOURCE::templates::url_of_search_results_css); - HTTPErrorResponse response(m_root, request, MHD_HTTP_NOT_FOUND, + HTTPErrorResponse response(request, MHD_HTTP_NOT_FOUND, "fulltext-search-unavailable", "404-page-heading", cssUrl); @@ -980,7 +980,7 @@ std::unique_ptr InternalServer::handle_random(const RequestContext& re } if ( startsWith(request.get_url(), "/random/") ) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } std::string bookName; @@ -994,7 +994,7 @@ std::unique_ptr InternalServer::handle_random(const RequestContext& re } if (archive == nullptr) { - return HTTP404Response(m_root, request) + return HTTP404Response(request) + noSuchBookErrorMsg(bookName); } @@ -1002,7 +1002,7 @@ std::unique_ptr InternalServer::handle_random(const RequestContext& re auto entry = archive->getRandomEntry(); return build_redirect(bookName, getFinalItem(*archive, entry)); } catch(zim::EntryNotFound& e) { - return HTTP404Response(m_root, request) + return HTTP404Response(request) + nonParameterizedMessage("random-article-failure"); } } @@ -1015,7 +1015,7 @@ std::unique_ptr InternalServer::handle_captured_external(const Request } catch (const std::out_of_range& e) {} if (source.empty()) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } auto data = get_default_data(); @@ -1033,7 +1033,7 @@ std::unique_ptr InternalServer::handle_catch(const RequestContext& req return handle_captured_external(request); } - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } std::vector @@ -1117,7 +1117,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(m_root, request) + return UrlNotFoundResponse(request) + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } @@ -1164,7 +1164,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r printf("Failed to find %s\n", urlStr.c_str()); std::string searchURL = m_root + "/search?content=" + bookName + "&pattern=" + kiwix::urlEncode(pattern); - return UrlNotFoundResponse(m_root, request) + return UrlNotFoundResponse(request) + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } } @@ -1182,11 +1182,11 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque bookName = request.get_url_part(1); kind = request.get_url_part(2); } catch (const std::out_of_range& e) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } if (kind != "meta" && kind!= "content") { - return UrlNotFoundResponse(m_root, request) + return UrlNotFoundResponse(request) + invalidRawAccessMsg(kind); } @@ -1197,7 +1197,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque } catch (const std::out_of_range& e) {} if (archive == nullptr) { - return UrlNotFoundResponse(m_root, request) + return UrlNotFoundResponse(request) + noSuchBookErrorMsg(bookName); } @@ -1231,7 +1231,7 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque if (m_verbose.load()) { printf("Failed to find %s\n", itemPath.c_str()); } - return UrlNotFoundResponse(m_root, request) + return UrlNotFoundResponse(request) + rawEntryNotFoundMsg(kind, itemPath); } } diff --git a/src/server/internalServer_catalog.cpp b/src/server/internalServer_catalog.cpp index a1ec11100..1c48117fc 100644 --- a/src/server/internalServer_catalog.cpp +++ b/src/server/internalServer_catalog.cpp @@ -63,7 +63,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r host = request.get_header("Host"); url = request.get_url_part(1); } catch (const std::out_of_range&) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } if (url == "v2") { @@ -71,7 +71,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r } if (url != "searchdescription.xml" && url != "root.xml" && url != "search") { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } if (url == "searchdescription.xml") { @@ -108,7 +108,7 @@ std::unique_ptr InternalServer::handle_catalog_v2(const RequestContext try { url = request.get_url_part(2); } catch (const std::out_of_range&) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } if (url == "root.xml") { @@ -134,7 +134,7 @@ std::unique_ptr InternalServer::handle_catalog_v2(const RequestContext } else if (url == "illustration") { return handle_catalog_v2_illustration(request); } else { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } } @@ -174,7 +174,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const try { mp_library->getBookById(entryId); } catch (const std::out_of_range&) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } OPDSDumper opdsDumper(mp_library.get(), mp_nameMapper.get()); @@ -221,7 +221,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_illustration(const R illustration->mimeType ); } catch(...) { - return UrlNotFoundResponse(m_root, request); + return UrlNotFoundResponse(request); } } diff --git a/src/server/response.cpp b/src/server/response.cpp index 304a1b7e3..0fcb65052 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -163,14 +163,12 @@ std::unique_ptr ContentResponseBlueprint::generateResponseObjec return r; } -HTTPErrorResponse::HTTPErrorResponse(const std::string& root, - const RequestContext& request, +HTTPErrorResponse::HTTPErrorResponse(const RequestContext& request, int httpStatusCode, const std::string& pageTitleMsgId, const std::string& headingMsgId, const std::string& cssUrl) - : ContentResponseBlueprint(root, - &request, + : 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) @@ -184,19 +182,16 @@ HTTPErrorResponse::HTTPErrorResponse(const std::string& root, }; } -HTTP404Response::HTTP404Response(const std::string& root, - const RequestContext& request) - : HTTPErrorResponse(root, - request, +HTTP404Response::HTTP404Response(const RequestContext& request) + : HTTPErrorResponse(request, MHD_HTTP_NOT_FOUND, "404-page-title", "404-page-heading") { } -UrlNotFoundResponse::UrlNotFoundResponse(const std::string& root, - const RequestContext& request) - : HTTP404Response(root, request) +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}}); @@ -216,10 +211,8 @@ HTTPErrorResponse& HTTPErrorResponse::operator+=(const ParameterizedMessage& det } -HTTP400Response::HTTP400Response(const std::string& root, - const RequestContext& request) - : HTTPErrorResponse(root, - request, +HTTP400Response::HTTP400Response(const RequestContext& request) + : HTTPErrorResponse(request, MHD_HTTP_BAD_REQUEST, "400-page-title", "400-page-heading") @@ -232,10 +225,8 @@ HTTP400Response::HTTP400Response(const std::string& root, *this += ParameterizedMessage("invalid-request", {{"url", requestUrl}}); } -HTTP500Response::HTTP500Response(const std::string& root, - const RequestContext& request) - : HTTPErrorResponse(root, - request, +HTTP500Response::HTTP500Response(const RequestContext& request) + : HTTPErrorResponse(request, MHD_HTTP_INTERNAL_SERVER_ERROR, "500-page-title", "500-page-heading") diff --git a/src/server/response.h b/src/server/response.h index 4707ed8ea..b1636fa9c 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -115,13 +115,11 @@ class ContentResponse : public Response { class ContentResponseBlueprint { public: // functions - ContentResponseBlueprint(const std::string& root, - const RequestContext* request, + ContentResponseBlueprint(const RequestContext* request, int httpStatusCode, const std::string& mimeType, const std::string& templateStr) - : m_root(root) - , m_request(*request) + : m_request(*request) , m_httpStatusCode(httpStatusCode) , m_mimeType(mimeType) , m_template(templateStr) @@ -140,7 +138,6 @@ protected: // functions virtual std::unique_ptr generateResponseObject() const; public: //data - const std::string m_root; const RequestContext& m_request; const int m_httpStatusCode; const std::string m_mimeType; @@ -150,8 +147,7 @@ public: //data struct HTTPErrorResponse : ContentResponseBlueprint { - HTTPErrorResponse(const std::string& root, - const RequestContext& request, + HTTPErrorResponse(const RequestContext& request, int httpStatusCode, const std::string& pageTitleMsgId, const std::string& headingMsgId, @@ -163,26 +159,22 @@ struct HTTPErrorResponse : ContentResponseBlueprint struct HTTP404Response : HTTPErrorResponse { - HTTP404Response(const std::string& root, - const RequestContext& request); + explicit HTTP404Response(const RequestContext& request); }; struct UrlNotFoundResponse : HTTP404Response { - UrlNotFoundResponse(const std::string& root, - const RequestContext& request); + explicit UrlNotFoundResponse(const RequestContext& request); }; struct HTTP400Response : HTTPErrorResponse { - HTTP400Response(const std::string& root, - const RequestContext& request); + explicit HTTP400Response(const RequestContext& request); }; struct HTTP500Response : HTTPErrorResponse { - HTTP500Response(const std::string& root, - const RequestContext& request); + explicit HTTP500Response(const RequestContext& request); private: // overrides // generateResponseObject() is overriden in order to produce a minimal From 5f27b4b651c14555279904cb44ed4f0344101aa8 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 29 Nov 2023 21:19:55 +0400 Subject: [PATCH 13/13] Taking advantage of std::make_unique() --- src/server/response.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/server/response.cpp b/src/server/response.cpp index 0fcb65052..8ccab1313 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -134,7 +134,7 @@ void Response::set_kind(Kind k) std::unique_ptr Response::build() { - return std::unique_ptr(new Response()); + return std::make_unique(); } std::unique_ptr Response::build_304(const ETag& etag) @@ -389,9 +389,7 @@ std::unique_ptr ContentResponse::build( const std::string& content, const std::string& mimetype) { - return std::unique_ptr(new ContentResponse( - content, - mimetype)); + return std::make_unique(content, mimetype); } std::unique_ptr ContentResponse::build( @@ -432,10 +430,7 @@ std::unique_ptr ItemResponse::build(const RequestContext& request, con return response; } - return std::unique_ptr(new ItemResponse( - item, - mimetype, - byteRange)); + return std::make_unique(item, mimetype, byteRange); } MHD_Response*