From 3b98987cb355683c8f256a85dfccea7037d1f593 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 5 Aug 2022 16:30:28 +0400 Subject: [PATCH] More robust handling of endpoint URLs The next goal is to redirect old-style /book/path/to/entry URLs to /content/book/path/to/entry, which seemed pretty trivial. However, given the current handling of some endpoint URLs, more work was required to ensure that invalid endpoint URLs (e.g. "/random/number" or "/suggest/fr") are not interpreted as content URLs. Previously, that was not a user-observable issue, since the result would be an immediate 404 error (except in certain edge cases, like handling the request for "/random/number" when there is a book with name "random" containing an article at path "/number"). With redirection of URLs that were assumed to refer to content a 404 error would be issued for the transformed URL ("/content/random/number") which may be confusing. Therefore this change is to ensure the correct routing of endpoint URL handling. --- src/server/internalServer.cpp | 77 ++++++++++++++++++++++++++--------- src/server/internalServer.h | 1 + test/server.cpp | 12 +++++- 3 files changed, 70 insertions(+), 20 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index d3d5aef42..f0a3750eb 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -524,6 +524,16 @@ MHD_Result InternalServer::handlerCallback(struct MHD_Connection* connection, return ret; } +namespace +{ + +bool isEndpointUrl(const std::string& url, const std::string& endpoint) +{ + return startsWith(url, "/" + endpoint + "/") || url == "/" + endpoint; +}; + +} // unnamed namespace + std::unique_ptr InternalServer::handle_request(const RequestContext& request) { try { @@ -536,43 +546,36 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r if ( etag ) return Response::build_304(*this, etag); - if ( isLocallyCustomizedResource(request.get_url()) ) + const auto url = request.get_url(); + if ( isLocallyCustomizedResource(url) ) return handle_locally_customized_resource(request); - if (request.get_url() == "/" ) + if (url == "/" ) return build_homepage(request); - if (startsWith(request.get_url(), "/skin/")) + if (isEndpointUrl(url, "skin")) return handle_skin(request); - if (startsWith(request.get_url(), "/content/")) + if (isEndpointUrl(url, "content")) return handle_content(request); - if (startsWith(request.get_url(), "/catalog/")) + if (isEndpointUrl(url, "catalog")) return handle_catalog(request); - if (startsWith(request.get_url(), "/raw/")) + if (isEndpointUrl(url, "raw")) return handle_raw(request); - if (request.get_url() == "/search") + if (isEndpointUrl(url, "search")) return handle_search(request); - if (request.get_url() == "/search/searchdescription.xml") { - return ContentResponse::build( - *this, - RESOURCE::ft_opensearchdescription_xml, - get_default_data(), - "application/opensearchdescription+xml"); - } - - if (request.get_url() == "/suggest") + if (isEndpointUrl(url, "suggest")) return handle_suggest(request); - if (request.get_url() == "/random") + if (isEndpointUrl(url, "random")) return handle_random(request); - if (request.get_url() == "/catch/external") - return handle_captured_external(request); + if (isEndpointUrl(url, "catch")) + return handle_catch(request); return handle_content(request); } catch (std::exception& e) { @@ -629,6 +632,11 @@ std::unique_ptr InternalServer::handle_suggest(const RequestContext& r printf("** running handle_suggest\n"); } + if ( startsWith(request.get_url(), "/suggest/") ) { + return HTTP404Response(*this, request) + + urlNotFoundMsg; + } + std::string bookName, bookId; std::shared_ptr archive; try { @@ -728,6 +736,18 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re printf("** running handle_search\n"); } + if ( startsWith(request.get_url(), "/search/") ) { + if (request.get_url() == "/search/searchdescription.xml") { + return ContentResponse::build( + *this, + RESOURCE::ft_opensearchdescription_xml, + get_default_data(), + "application/opensearchdescription+xml"); + } + return HTTP404Response(*this, request) + + urlNotFoundMsg; + } + try { auto searchInfo = getSearchInfo(request); auto bookIds = searchInfo.getBookIds(); @@ -811,6 +831,11 @@ std::unique_ptr InternalServer::handle_random(const RequestContext& re printf("** running handle_random\n"); } + if ( startsWith(request.get_url(), "/random/") ) { + return HTTP404Response(*this, request) + + urlNotFoundMsg; + } + std::string bookName; std::shared_ptr archive; try { @@ -854,6 +879,20 @@ std::unique_ptr InternalServer::handle_captured_external(const Request return ContentResponse::build(*this, RESOURCE::templates::captured_external_html, data, "text/html; charset=utf-8"); } +std::unique_ptr InternalServer::handle_catch(const RequestContext& request) +{ + if (m_verbose.load()) { + printf("** running handle_catch\n"); + } + + if ( request.get_url() == "/catch/external" ) { + return handle_captured_external(request); + } + + return HTTP404Response(*this, request) + + urlNotFoundMsg; +} + std::unique_ptr InternalServer::handle_catalog(const RequestContext& request) { if (m_verbose.load()) { diff --git a/src/server/internalServer.h b/src/server/internalServer.h index 011fce2e4..82574378c 100644 --- a/src/server/internalServer.h +++ b/src/server/internalServer.h @@ -138,6 +138,7 @@ class InternalServer { std::unique_ptr handle_search(const RequestContext& request); std::unique_ptr handle_suggest(const RequestContext& request); std::unique_ptr handle_random(const RequestContext& request); + std::unique_ptr handle_catch(const RequestContext& request); std::unique_ptr handle_captured_external(const RequestContext& request); std::unique_ptr handle_content(const RequestContext& request); std::unique_ptr handle_raw(const RequestContext& request); diff --git a/test/server.cpp b/test/server.cpp index b50dbe396..8b77f97d9 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -271,6 +271,7 @@ const char* urls404[] = { "/ROOT/non-existent-item", "/ROOT/skin/non-existent-skin-resource", "/ROOT/catalog", + "/ROOT/catalog/", "/ROOT/catalog/non-existent-item", "/ROOT/catalogBLABLABLA/root.xml", "/ROOT/catalog/v2/illustration/zimfile?size=48", @@ -281,9 +282,18 @@ const char* urls404[] = { "/ROOT/meta?content=non-existent-book&name=title", "/ROOT/random", "/ROOT/random?content=non-existent-book", + "/ROOT/random/", + "/ROOT/random/number", "/ROOT/suggest", "/ROOT/suggest?content=non-existent-book&term=abcd", - "/ROOT/catch/external", + "/ROOT/suggest/", + "/ROOT/suggest/fr", + "/ROOT/search/", + "/ROOT/search/anythingotherthansearchdescription.xml", + "/ROOT/catch/", + "/ROOT/catch/external", // missing ?source=URL + "/ROOT/catch/external?source=", + "/ROOT/catch/anythingotherthanexternal", "/ROOT/zimfile/A/non-existent-article", "/ROOT/content/zimfile/A/non-existent-article",