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

@@ -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<Response> InternalServer::handle_request(const RequestContext& request)
 {
   try {
@@ -536,43 +546,36 @@ std::unique_ptr<Response> 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") {
+    if (isEndpointUrl(url, "suggest"))
-      return ContentResponse::build(
-        *this,
-        RESOURCE::ft_opensearchdescription_xml,
-        get_default_data(),
-        "application/opensearchdescription+xml");
-    }
-
-    if (request.get_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")
+    if (isEndpointUrl(url, "catch"))
-      return handle_captured_external(request);
+      return handle_catch(request);
 
     return handle_content(request);
   } catch (std::exception& e) {
@@ -629,6 +632,11 @@ std::unique_ptr<Response> 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<zim::Archive> archive;
   try {
@@ -728,6 +736,18 @@ std::unique_ptr<Response> 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<Response> 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<zim::Archive> archive;
   try {
@@ -854,6 +879,20 @@ std::unique_ptr<Response> InternalServer::handle_captured_external(const Request
   return ContentResponse::build(*this, RESOURCE::templates::captured_external_html, data, "text/html; charset=utf-8");
 }
 
+std::unique_ptr<Response> 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<Response> InternalServer::handle_catalog(const RequestContext& request)
 {
   if (m_verbose.load()) {

src/server/internalServer.h

@@ -138,6 +138,7 @@ class InternalServer {
     std::unique_ptr<Response> handle_search(const RequestContext& request);
     std::unique_ptr<Response> handle_suggest(const RequestContext& request);
     std::unique_ptr<Response> handle_random(const RequestContext& request);
+    std::unique_ptr<Response> handle_catch(const RequestContext& request);
     std::unique_ptr<Response> handle_captured_external(const RequestContext& request);
     std::unique_ptr<Response> handle_content(const RequestContext& request);
     std::unique_ptr<Response> handle_raw(const RequestContext& request);

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",