From 2a20e873412f10d12142e41442384298b565f2ca Mon Sep 17 00:00:00 2001
From: Veloman Yunkan <veloman.yunkan@gmail.com>
Date: Wed, 30 Mar 2022 20:05:37 +0400
Subject: [PATCH] Got rid of Response::build_500()

This change is not tested (mostly due to the difficulties of triggering
an internal server error).
---
 src/server/internalServer.cpp |  9 ++++---
 src/server/internalServer.h   |  2 --
 src/server/response.cpp       | 44 +++++++++++++++++++----------------
 src/server/response.h         | 12 +++++++++-
 static/resources_list.txt     |  1 -
 static/templates/500.html     | 16 -------------
 6 files changed, 41 insertions(+), 43 deletions(-)
 delete mode 100644 static/templates/500.html

diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp
index dfa6a2403..05c2d559c 100644
--- a/src/server/internalServer.cpp
+++ b/src/server/internalServer.cpp
@@ -359,10 +359,12 @@ std::unique_ptr<Response> InternalServer::handle_request(const RequestContext& r
     return handle_content(request);
   } catch (std::exception& e) {
     fprintf(stderr, "===== Unhandled error : %s\n", e.what());
-    return Response::build_500(*this, e.what());
+    return HTTP500HtmlResponse(*this, request)
+         + e.what();
   } catch (...) {
     fprintf(stderr, "===== Unhandled unknown error\n");
-    return Response::build_500(*this, "Unknown error");
+    return HTTP500HtmlResponse(*this, request)
+         + "Unknown error";
   }
 }
 
@@ -631,7 +633,8 @@ std::unique_ptr<Response> InternalServer::handle_search(const RequestContext& re
       + std::string(e.what());
   } catch (const std::exception& e) {
     std::cerr << e.what() << std::endl;
-    return Response::build_500(*this, e.what());
+    return HTTP500HtmlResponse(*this, request)
+         + e.what();
   }
 }
 
diff --git a/src/server/internalServer.h b/src/server/internalServer.h
index eb8dcc5cf..69e8166c4 100644
--- a/src/server/internalServer.h
+++ b/src/server/internalServer.h
@@ -177,8 +177,6 @@ class InternalServer {
     friend std::unique_ptr<Response> Response::build(const InternalServer& server);
     friend std::unique_ptr<ContentResponse> ContentResponse::build(const InternalServer& server, const std::string& content, const std::string& mimetype, bool isHomePage, bool raw);
     friend std::unique_ptr<Response> ItemResponse::build(const InternalServer& server, const RequestContext& request, const zim::Item& item, bool raw);
-    friend std::unique_ptr<Response> Response::build_500(const InternalServer& server, const std::string& msg);
-
 };
 
 }
diff --git a/src/server/response.cpp b/src/server/response.cpp
index 01b8bf97d..aefa0dc71 100644
--- a/src/server/response.cpp
+++ b/src/server/response.cpp
@@ -160,6 +160,30 @@ HTTPErrorHtmlResponse& HTTP400HtmlResponse::operator+(InvalidUrlMsg /*unused*/)
   return *this + msgTmpl.render({"url", requestUrl});
 }
 
+HTTP500HtmlResponse::HTTP500HtmlResponse(const InternalServer& server,
+                                         const RequestContext& request)
+  : HTTPErrorHtmlResponse(server,
+                          request,
+                          MHD_HTTP_INTERNAL_SERVER_ERROR,
+                          "Internal Server Error",
+                          "Internal Server Error")
+{
+  // operator+() is a state-modifying operator (akin to operator+=)
+  *this + "An internal server error occured. We are sorry about that :/";
+}
+
+std::unique_ptr<ContentResponse> HTTP500HtmlResponse::generateResponseObject() const
+{
+  // We want a 500 response to be a minimalistic one (so that the server doesn't
+  // have to provide additional resources required for its proper rendering)
+  // ";raw=true" in the MIME-type below disables response decoration
+  // (see ContentResponse::contentDecorationAllowed())
+  const std::string mimeType = "text/html;charset=utf-8;raw=true";
+  auto r = ContentResponse::build(m_server, m_template, m_data, mimeType);
+  r->set_code(m_httpStatusCode);
+  return r;
+}
+
 ContentResponseBlueprint& ContentResponseBlueprint::operator+(const TaskbarInfo& taskbarInfo)
 {
   this->m_taskbarInfo.reset(new TaskbarInfo(taskbarInfo));
@@ -179,26 +203,6 @@ std::unique_ptr<Response> Response::build_416(const InternalServer& server, size
   return response;
 }
 
-std::unique_ptr<Response> Response::build_500(const InternalServer& server, const std::string& msg)
-{
-  MustacheData data;
-  data.set("error", msg);
-  auto content = render_template(RESOURCE::templates::_500_html, data);
-  std::unique_ptr<Response> response (
-      new ContentResponse(
-        server.m_root, //root
-        true, //verbose
-        true, //raw
-        false, //withTaskbar
-        false, //withLibraryButton
-        false, //blockExternalLinks
-        content, //content
-        "text/html" //mimetype
-  ));
-  response->set_code(MHD_HTTP_INTERNAL_SERVER_ERROR);
-  return response;
-}
-
 
 std::unique_ptr<Response> Response::build_redirect(const InternalServer& server, const std::string& redirectUrl)
 {
diff --git a/src/server/response.h b/src/server/response.h
index f7546f269..710e2bc0f 100644
--- a/src/server/response.h
+++ b/src/server/response.h
@@ -50,7 +50,6 @@ class Response {
     static std::unique_ptr<Response> build(const InternalServer& server);
     static std::unique_ptr<Response> build_304(const InternalServer& server, const ETag& etag);
     static std::unique_ptr<Response> build_416(const InternalServer& server, size_t resourceLength);
-    static std::unique_ptr<Response> build_500(const InternalServer& server, const std::string& msg);
     static std::unique_ptr<Response> build_redirect(const InternalServer& server, const std::string& redirectUrl);
 
     MHD_Result send(const RequestContext& request, MHD_Connection* connection);
@@ -217,6 +216,17 @@ struct HTTP400HtmlResponse : HTTPErrorHtmlResponse
   HTTPErrorHtmlResponse& operator+(InvalidUrlMsg /*unused*/);
 };
 
+struct HTTP500HtmlResponse : HTTPErrorHtmlResponse
+{
+  HTTP500HtmlResponse(const InternalServer& server,
+                      const RequestContext& request);
+
+private: // overrides
+  // generateResponseObject() is overriden in order to produce a minimal
+  // response without any need for additional resources from the server
+  std::unique_ptr<ContentResponse> generateResponseObject() const override;
+};
+
 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 258553a2a..7444c48ae 100644
--- a/static/resources_list.txt
+++ b/static/resources_list.txt
@@ -36,7 +36,6 @@ skin/search_results.css
 templates/search_result.html
 templates/no_search_result.html
 templates/error.html
-templates/500.html
 templates/index.html
 templates/suggestion.json
 templates/head_taskbar.html
diff --git a/static/templates/500.html b/static/templates/500.html
deleted file mode 100644
index 1b2692d5f..000000000
--- a/static/templates/500.html
+++ /dev/null
@@ -1,16 +0,0 @@
-<!DOCTYPE html>
-<html>
-  <head>
-    <meta content="text/html;charset=UTF-8" http-equiv="content-type" />
-    <title>Internal Server Error</title>
-  </head>
-  <body>
-    <h1>Internal Server Error</h1>
-    <p>
-      An internal server error occured. We are sorry about that :/
-    </p>
-    <p>
-      {{ error }}
-    </p>
-  </body>
-</html>