diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index baffc6738..0c45580ad 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -109,6 +109,54 @@ unsigned int getCacheLength(const char* name, unsigned int defaultVal) { } } // unnamed namespace +SearchInfo::SearchInfo(const std::string& pattern) + : pattern(pattern), + geoQuery() +{} + +SearchInfo::SearchInfo(const std::string& pattern, GeoQuery geoQuery) + : pattern(pattern), + geoQuery(geoQuery) +{} + +SearchInfo::SearchInfo(const RequestContext& request) + : pattern(request.get_optional_param("pattern", "")), + geoQuery(), + bookName(request.get_optional_param("content", "")) +{ + /* Retrive geo search */ + try { + auto latitude = request.get_argument("latitude"); + auto longitude = request.get_argument("longitude"); + auto distance = request.get_argument("distance"); + geoQuery = GeoQuery(latitude, longitude, distance); + } catch(const std::out_of_range&) {} + catch(const std::invalid_argument&) {} + + if (!geoQuery && pattern.empty()) { + throw std::invalid_argument("No query provided."); + } +} + +zim::Query SearchInfo::getZimQuery(bool verbose) const { + zim::Query query; + if (verbose) { + std::cout << "Performing query '" << pattern<< "'"; + } + query.setQuery(pattern); + if (geoQuery) { + if (verbose) { + std::cout << " with geo query '" << geoQuery.distance << "&(" << geoQuery.latitude << ";" << geoQuery.longitude << ")'"; + } + query.setGeorange(geoQuery.latitude, geoQuery.longitude, geoQuery.distance); + } + if (verbose) { + std::cout << std::endl; + } + return query; +} + + static IdNameMapper defaultNameMapper; static MHD_Result staticHandlerCallback(void* cls, @@ -499,111 +547,86 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re printf("** running handle_search\n"); } - std::string patternString; try { - patternString = request.get_argument("pattern"); - } catch (const std::out_of_range&) {} + auto searchInfo = SearchInfo(request); - /* Retrive geo search */ - bool has_geo_query = false; - float latitude = 0; - float longitude = 0; - float distance = 0; - try { - latitude = request.get_argument("latitude"); - longitude = request.get_argument("longitude"); - distance = request.get_argument("distance"); - has_geo_query = true; - } catch(const std::out_of_range&) {} - catch(const std::invalid_argument&) {} - - std::string bookName, bookId; - std::shared_ptr archive; - try { - bookName = request.get_argument("content"); - bookId = mp_nameMapper->getIdForName(bookName); - archive = mp_library->getArchiveById(bookId); - } catch (const std::out_of_range&) {} - - /* Make the search */ - if ( (!archive && !bookName.empty()) - || (patternString.empty() && ! has_geo_query) ) { - auto data = get_default_data(); - data.set("pattern", encodeDiples(patternString)); - data.set("root", m_root); - auto response = ContentResponse::build(*this, RESOURCE::templates::no_search_result_html, data, "text/html; charset=utf-8"); - response->set_code(MHD_HTTP_NOT_FOUND); - return withTaskbarInfo(bookName, archive.get(), std::move(response)); - } - - std::shared_ptr searcher; - if (archive) { - searcher = searcherCache.getOrPut(bookId, [=](){ return std::make_shared(*archive);}); - } else { - for (auto& bookId: mp_library->filter(kiwix::Filter().local(true).valid(true))) { - auto currentArchive = mp_library->getArchiveById(bookId); - if (currentArchive) { - if (! searcher) { - searcher = std::make_shared(*currentArchive); - } else { - searcher->addArchive(*currentArchive); - } + std::string bookId; + std::shared_ptr archive; + if (!searchInfo.bookName.empty()) { + try { + bookId = mp_nameMapper->getIdForName(searchInfo.bookName); + archive = mp_library->getArchiveById(bookId); + } catch (const std::out_of_range&) { + throw std::invalid_argument("The requested book doesn't exist."); } } - } - auto start = 0; - try { - start = request.get_argument("start"); - } catch (const std::exception&) {} - - auto pageLength = 25; - try { - pageLength = request.get_argument("pageLength"); - } catch (const std::exception&) {} - if (pageLength > MAX_SEARCH_LEN) { - pageLength = MAX_SEARCH_LEN; - } - if (pageLength == 0) { - pageLength = 25; - } - - /* Get the results */ - std::string queryString; - try { - zim::Query query; - if (patternString.empty()) { - // Execute geo-search - if (m_verbose.load()) { - cout << "Performing geo query `" << distance << "&(" << latitude << ";" << longitude << ")'" << endl; - } - - query.setQuery(""); - queryString = "GEO:" + to_string(latitude) + to_string(longitude) + to_string(distance); - query.setGeorange(latitude, longitude, distance); - } else { - // Execute Ft search - if (m_verbose.load()) { - cout << "Performing query `" << patternString << "'" << endl; - } - - queryString = "FT:" + removeAccents(patternString); - query.setQuery(queryString); - } - queryString = bookId + queryString; + /* Make the search */ + // Try to get a search from the searchInfo, else build it std::shared_ptr search; - search = searchCache.getOrPut(queryString, [=](){ return make_shared(searcher->search(query));}); + try { + search = searchCache.getOrPut(searchInfo, + [=](){ + std::shared_ptr searcher; + if (archive) { + searcher = searcherCache.getOrPut(bookId, [=](){ return std::make_shared(*archive);}); + } else { + for (auto& bookId: mp_library->filter(kiwix::Filter().local(true).valid(true))) { + auto currentArchive = mp_library->getArchiveById(bookId); + if (currentArchive) { + if (! searcher) { + searcher = std::make_shared(*currentArchive); + } else { + searcher->addArchive(*currentArchive); + } + } + } + } + return make_shared(searcher->search(searchInfo.getZimQuery(m_verbose.load()))); + } + ); + } catch(std::runtime_error& e) { + // 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) + auto data = get_default_data(); + data.set("pattern", encodeDiples(searchInfo.pattern)); + auto response = ContentResponse::build(*this, RESOURCE::templates::no_search_result_html, data, "text/html; charset=utf-8"); + response->set_code(MHD_HTTP_NOT_FOUND); + return withTaskbarInfo(searchInfo.bookName, archive.get(), std::move(response)); + } + + auto start = 0; + try { + start = request.get_argument("start"); + } catch (const std::exception&) {} + + auto pageLength = 25; + try { + pageLength = request.get_argument("pageLength"); + } catch (const std::exception&) {} + if (pageLength > MAX_SEARCH_LEN) { + pageLength = MAX_SEARCH_LEN; + } + if (pageLength == 0) { + pageLength = 25; + } + + /* Get the results */ SearchRenderer renderer(search->getResults(start, pageLength), mp_nameMapper, mp_library, start, search->getEstimatedMatches()); - renderer.setSearchPattern(patternString); - renderer.setSearchContent(bookName); + renderer.setSearchPattern(searchInfo.pattern); + renderer.setSearchContent(searchInfo.bookName); renderer.setProtocolPrefix(m_root + "/"); renderer.setSearchProtocolPrefix(m_root + "/search?"); renderer.setPageLength(pageLength); auto response = ContentResponse::build(*this, renderer.getHtml(), "text/html; charset=utf-8"); - return withTaskbarInfo(bookName, archive.get(), std::move(response)); + return withTaskbarInfo(searchInfo.bookName, archive.get(), std::move(response)); + } catch (const std::invalid_argument& e) { + return HTTP400HtmlResponse(*this, request) + + invalidUrlMsg + + std::string(e.what()); } catch (const std::exception& e) { std::cerr << e.what() << std::endl; return Response::build_500(*this, e.what()); diff --git a/src/server/internalServer.h b/src/server/internalServer.h index 6ccde9f8f..eb8dcc5cf 100644 --- a/src/server/internalServer.h +++ b/src/server/internalServer.h @@ -43,9 +43,53 @@ extern "C" { namespace kiwix { +struct GeoQuery { + GeoQuery() + : GeoQuery(0, 0, -1) + {} + + GeoQuery(float latitude, float longitude, float distance) + : latitude(latitude), longitude(longitude), distance(distance) + {} + float latitude; + float longitude; + float distance; + + explicit operator bool() const { + return distance >= 0; + } + + friend bool operator<(const GeoQuery& l, const GeoQuery& r) + { + return std::tie(l.latitude, l.longitude, l.distance) + < std::tie(r.latitude, r.longitude, r.distance); // keep the same order + } +}; + +class SearchInfo { + public: + SearchInfo(const std::string& pattern); + SearchInfo(const std::string& pattern, GeoQuery geoQuery); + SearchInfo(const RequestContext& request); + + zim::Query getZimQuery(bool verbose) const; + + friend bool operator<(const SearchInfo& l, const SearchInfo& r) + { + return std::tie(l.bookName, l.pattern, l.geoQuery) + < std::tie(r.bookName, r.pattern, r.geoQuery); // keep the same order + } + + public: //data + std::string pattern; + GeoQuery geoQuery; + std::string bookName; +}; + + typedef kainjow::mustache::data MustacheData; typedef ConcurrentCache> SearcherCache; -typedef ConcurrentCache> SearchCache; +typedef ConcurrentCache> SearchCache; typedef ConcurrentCache> SuggestionSearcherCache; class Entry; @@ -77,7 +121,7 @@ class InternalServer { bool start(); void stop(); std::string getAddress() { return m_addr; } - int getPort() { return m_port; } + int getPort() { return m_port; } private: // functions std::unique_ptr handle_request(const RequestContext& request); diff --git a/src/server/response.cpp b/src/server/response.cpp index 50a66c344..4b82659c8 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -110,6 +110,7 @@ std::unique_ptr Response::build_404(const InternalServer& serve } extern const UrlNotFoundMsg urlNotFoundMsg; +extern const InvalidUrlMsg invalidUrlMsg; std::unique_ptr ContentResponseBlueprint::generateResponseObject() const { @@ -145,6 +146,36 @@ HTTP404HtmlResponse& HTTP404HtmlResponse::operator+(const std::string& msg) return *this; } +HTTP400HtmlResponse::HTTP400HtmlResponse(const InternalServer& server, + const RequestContext& request) + : ContentResponseBlueprint(&server, + &request, + MHD_HTTP_BAD_REQUEST, + "text/html", + RESOURCE::templates::_400_html) +{ + kainjow::mustache::list emptyList; + this->m_data = kainjow::mustache::object{{"details", emptyList}}; +} + +HTTP400HtmlResponse& HTTP400HtmlResponse::operator+(InvalidUrlMsg /*unused*/) +{ + std::string requestUrl = m_request.get_full_url(); + const auto query = m_request.get_query(); + 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}); +} + +HTTP400HtmlResponse& HTTP400HtmlResponse::operator+(const std::string& msg) +{ + m_data["details"].push_back({"p", msg}); + return *this; +} + + ContentResponseBlueprint& ContentResponseBlueprint::operator+(const TaskbarInfo& taskbarInfo) { this->m_taskbarInfo.reset(new TaskbarInfo(taskbarInfo)); diff --git a/src/server/response.h b/src/server/response.h index b10524bc8..d46898923 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -207,6 +207,20 @@ struct HTTP404HtmlResponse : ContentResponseBlueprint HTTP404HtmlResponse& operator+(const std::string& errorDetails); }; +class InvalidUrlMsg {}; + +extern const InvalidUrlMsg invalidUrlMsg; + +struct HTTP400HtmlResponse : ContentResponseBlueprint +{ + HTTP400HtmlResponse(const InternalServer& server, + const RequestContext& request); + + using ContentResponseBlueprint::operator+; + HTTP400HtmlResponse& operator+(InvalidUrlMsg /*unused*/); + HTTP400HtmlResponse& operator+(const std::string& errorDetails); +}; + class ItemResponse : public Response { public: ItemResponse(bool verbose, const zim::Item& item, const std::string& mimetype, const ByteRange& byterange); diff --git a/static/resources_list.txt b/static/resources_list.txt index 5587111c0..ab583d7a6 100644 --- a/static/resources_list.txt +++ b/static/resources_list.txt @@ -35,6 +35,7 @@ skin/block_external.js skin/search_results.css templates/search_result.html templates/no_search_result.html +templates/400.html templates/404.html templates/500.html templates/index.html diff --git a/static/templates/400.html b/static/templates/400.html new file mode 100644 index 000000000..5f4ecf42f --- /dev/null +++ b/static/templates/400.html @@ -0,0 +1,15 @@ + + + + + Invalid request + + +

Invalid request

+{{#details}} +

+ {{{p}}} +

+{{/details}} + + diff --git a/static/templates/search_result.html b/static/templates/search_result.html index b52c9c3d3..c0ec3b02e 100644 --- a/static/templates/search_result.html +++ b/static/templates/search_result.html @@ -107,11 +107,11 @@ of {{count}} for - {{searchPattern}} + "{{{searchPattern}}}" {{/hasResults}} {{^hasResults}} - No results were found for {{searchPattern}} + No results were found for "{{{searchPattern}}}" {{/hasResults}} diff --git a/test/server.cpp b/test/server.cpp index cf57826e6..59ed29bb4 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -287,6 +287,22 @@ TEST_F(ServerTest, UncompressibleContentIsNotCompressed) } } +const char* urls400[] = { + "/ROOT/search", + "/ROOT/search?content=zimfile", + "/ROOT/search?content=non-existing-book&pattern=asdfqwerty", + "/ROOT/search?content=non-existing-book&pattern=asdGET(url)->status) << "url: " << url; + } +} + const char* urls404[] = { "/", "/zimfile", @@ -302,8 +318,6 @@ const char* urls404[] = { "/ROOT/meta?content=non-existent-book&name=title", "/ROOT/random", "/ROOT/random?content=non-existent-book", - "/ROOT/search", - "/ROOT/search?content=non-existing-book&pattern=asdfqwerty", "/ROOT/suggest", "/ROOT/suggest?content=non-existent-book&term=abcd", "/ROOT/catch/external", @@ -319,8 +333,9 @@ const char* urls404[] = { TEST_F(ServerTest, 404) { - for ( const char* url : urls404 ) + for ( const char* url : urls404 ) { EXPECT_EQ(404, zfs1_->GET(url)->status) << "url: " << url; + } } namespace TestingOfHtmlResponses @@ -388,13 +403,14 @@ public: : ExpectedResponseData(erd) , url(url) {} + virtual ~TestContentIn404HtmlResponse() = default; const std::string url; std::string expectedResponse() const; private: - std::string pageTitle() const; + virtual std::string pageTitle() const; std::string pageCssLink() const; std::string hiddenBookNameInput() const; std::string searchPatternInput() const; @@ -521,6 +537,25 @@ std::string TestContentIn404HtmlResponse::taskbarLinks() const + R"(">)"; } +class TestContentIn400HtmlResponse : public TestContentIn404HtmlResponse +{ +public: + TestContentIn400HtmlResponse(const std::string& url, + const ExpectedResponseData& erd) + : TestContentIn404HtmlResponse(url, erd) + {} + +private: + std::string pageTitle() const; +}; + +std::string TestContentIn400HtmlResponse::pageTitle() const { + return expectedPageTitle.empty() + ? "Invalid request" + : expectedPageTitle; +} + + } // namespace TestingOfHtmlResponses TEST_F(ServerTest, 404WithBodyTesting) @@ -650,28 +685,70 @@ TEST_F(ServerTest, 404WithBodyTesting) Cannot find content entry invalid-article

)" }, + }; - { /* url */ "/ROOT/search?content=zimfile", - expected_page_title=="Fulltext search unavailable" && - expected_css_url=="/ROOT/skin/search_results.css" && - book_name=="zimfile" && - book_title=="Ray Charles" && - expected_body==R"( -
Not found
+ for ( const auto& t : testData ) { + const TestContext ctx{ {"url", t.url} }; + const auto r = zfs1_->GET(t.url.c_str()); + EXPECT_EQ(r->status, 404) << ctx; + EXPECT_EQ(r->body, t.expectedResponse()) << ctx; + } +} + +TEST_F(ServerTest, 400WithBodyTesting) +{ + using namespace TestingOfHtmlResponses; + const std::vector testData{ + { /* url */ "/ROOT/search", + expected_body== R"( +

Invalid request

- There is no article with the title "" - and the fulltext search engine is not available for this content. + The requested URL "/ROOT/search" is not a valid request. +

+

+ No query provided.

)" }, - - { /* url */ "/ROOT/search?content=non-existent-book&pattern=asdfqwerty", - expected_page_title=="Fulltext search unavailable" && - expected_css_url=="/ROOT/skin/search_results.css" && + { /* url */ "/ROOT/search?content=zimfile", expected_body==R"( -
Not found
+

Invalid request

- There is no article with the title "asdfqwerty" - and the fulltext search engine is not available for this content. + The requested URL "/ROOT/search?content=zimfile" is not a valid request. +

+

+ No query provided. +

+)" }, + { /* url */ "/ROOT/search?content=non-existing-book&pattern=asdfqwerty", + expected_body==R"( +

Invalid request

+

+ The requested URL "/ROOT/search?content=non-existing-book&pattern=asdfqwerty" is not a valid request. +

+

+ The requested book doesn't exist. +

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