Do not allow SearchRendered to work on a delete nameMapper/Library.

By moving the nameMapper/library arguments in `getHtml`/`getXml` we avoid
any potential "use after free" of name mapper or library as they are not
stored.

include/search_renderer.h

@@ -34,32 +34,14 @@ class NameMapper;
 class SearchRenderer
 {
  public:
-  /**
-   * Construct a SearchRenderer from a SearchResultSet.
-   *
-   * The constructed version of the SearchRenderer will not introduce
-   * the book name for each result. It is better to use the other constructor
-   * with a Library pointer to have a better html page.
-   *
-   * @param srs The `SearchResultSet` to render.
-   * @param mapper The `NameMapper` to use to do the rendering.
-   * @param start The start offset used for the srs.
-   * @param estimatedResultCount The estimatedResultCount of the whole search
-   */
-  SearchRenderer(zim::SearchResultSet srs, const NameMapper* mapper,
-                 unsigned int start, unsigned int estimatedResultCount);
-
   /**
    * Construct a SearchRenderer from a SearchResultSet.
    *
    * @param srs The `SearchResultSet` to render.
-   * @param mapper The `NameMapper` to use to do the rendering.
-   * @param library The `Library` to use to look up book details for search results.
    * @param start The start offset used for the srs.
    * @param estimatedResultCount The estimatedResultCount of the whole search
    */
-  SearchRenderer(zim::SearchResultSet srs, const NameMapper* mapper, Library* library,
-                 unsigned int start, unsigned int estimatedResultCount);
+  SearchRenderer(zim::SearchResultSet srs, unsigned int start, unsigned int estimatedResultCount);
 
   ~SearchRenderer();
 
@@ -90,24 +72,32 @@ class SearchRenderer
     this->pageLength  = pageLength;
   }
 
-  std::string renderTemplate(const std::string& tmpl_str);
-
   /**
    * Generate the html page with the resutls of the search.
+   *
+   * @param mapper The `NameMapper` to use to do the rendering.
+   * @param library The `Library` to use to look up book details for search results.
+                    May be nullptr. In this case, bookName is not set in the rendered string.
+   * @return The html string
    */
-  std::string getHtml();
+  std::string getHtml(const NameMapper* mapper, const Library* library);
 
-    /**
+  /**
    * Generate the xml page with the resutls of the search.
+   *
+   * @param mapper The `NameMapper` to use to do the rendering.
+   * @param library The `Library` to use to look up book details for search results.
+                    May be nullptr. In this case, bookName is not set in the rendered string.
+   * @return The xml string
    */
-  std::string getXml();
+  std::string getXml(const NameMapper* mapper, const Library* library);
 
+ protected: // function
+  std::string renderTemplate(const std::string& tmpl_str, const NameMapper* mapper, const Library *library);
 
  protected:
   std::string beautifyInteger(const unsigned int number);
   zim::SearchResultSet m_srs;
-  const NameMapper* mp_nameMapper;
-  Library* mp_library;
   std::string searchBookQuery;
   std::string searchPattern;
   std::string protocolPrefix;

src/search_renderer.cpp

@@ -36,16 +36,9 @@ namespace kiwix
 {
 
 /* Constructor */
-SearchRenderer::SearchRenderer(zim::SearchResultSet srs, const NameMapper* mapper,
-                      unsigned int start, unsigned int estimatedResultCount)
-    : SearchRenderer(srs, mapper, nullptr, start, estimatedResultCount)
-{}
-
-SearchRenderer::SearchRenderer(zim::SearchResultSet srs, const NameMapper* mapper, Library* library,
+SearchRenderer::SearchRenderer(zim::SearchResultSet srs,
                       unsigned int start, unsigned int estimatedResultCount)
     : m_srs(srs),
-      mp_nameMapper(mapper),
-      mp_library(library),
       protocolPrefix("zim://"),
       searchProtocolPrefix("search://"),
       estimatedResultCount(estimatedResultCount),
@@ -164,20 +157,20 @@ kainjow::mustache::data buildPagination(
   return pagination;
 }
 
-std::string SearchRenderer::renderTemplate(const std::string& tmpl_str)
+std::string SearchRenderer::renderTemplate(const std::string& tmpl_str, const NameMapper* nameMapper, const Library* library)
 {
   const std::string absPathPrefix = protocolPrefix;
   // Build the results list
   kainjow::mustache::data items{kainjow::mustache::data::type::list};
   for (auto it = m_srs.begin(); it != m_srs.end(); it++) {
     kainjow::mustache::data result;
     const std::string zim_id(it.getZimId());
-    const auto path = mp_nameMapper->getNameForId(zim_id) + "/" + it.getPath();
+    const auto path = nameMapper->getNameForId(zim_id) + "/" + it.getPath();
     result.set("title", it.getTitle());
     result.set("absolutePath", absPathPrefix + urlEncode(path));
     result.set("snippet", it.getSnippet());
-    if (mp_library) {
-      result.set("bookTitle", mp_library->getBookById(zim_id).getTitle());
+    if (library) {
+      result.set("bookTitle", library->getBookById(zim_id).getTitle());
     }
     if (it.getWordCount() >= 0) {
       result.set("wordCount", kiwix::beautifyInteger(it.getWordCount()));
@@ -222,14 +215,14 @@ std::string SearchRenderer::renderTemplate(const std::string& tmpl_str)
   return ss.str();
 }
 
-std::string SearchRenderer::getHtml()
+std::string SearchRenderer::getHtml(const NameMapper* mapper, Library* library)
 {
-  return renderTemplate(RESOURCE::templates::search_result_html);
+  return renderTemplate(RESOURCE::templates::search_result_html, mapper, library);
 }
 
-std::string SearchRenderer::getXml()
+std::string SearchRenderer::getXml(const NameMapper* mapper, Library* library)
 {
-  return renderTemplate(RESOURCE::templates::search_result_xml);
+  return renderTemplate(RESOURCE::templates::search_result_xml, mapper, library);
 }
 
 

src/server/internalServer.cpp

@@ -963,17 +963,25 @@ std::unique_ptr<Response> InternalServer::handle_search_request(const RequestCon
   const auto pageLength = getSearchPageSize(request);
 
   /* Get the results */
-  SearchRenderer renderer(search->getResults(start-1, pageLength), mp_nameMapper.get(), mp_library.get(), start,
+  SearchRenderer renderer(search->getResults(start-1, pageLength), start,
                           search->getEstimatedMatches());
   renderer.setSearchPattern(searchInfo.pattern);
   renderer.setSearchBookQuery(searchInfo.bookFilterQuery);
   renderer.setProtocolPrefix(m_root + "/content/");
   renderer.setSearchProtocolPrefix(m_root + "/search");
   renderer.setPageLength(pageLength);
   if (request.get_requested_format() == "xml") {
-    return ContentResponse::build(*this, renderer.getXml(), "application/rss+xml; charset=utf-8");
+    return ContentResponse::build(
+      *this,
+      renderer.getXml(mp_nameMapper.get(), mp_library.get()),
+      "application/rss+xml; charset=utf-8"
+    );
   }
-  auto response = ContentResponse::build(*this, renderer.getHtml(), "text/html; charset=utf-8");
+  auto response = ContentResponse::build(
+    *this,
+    renderer.getHtml(mp_nameMapper.get(), mp_library.get()),
+    "text/html; charset=utf-8"
+  );
   // XXX: Now this has to be handled by the iframe-based viewer which
   // XXX: has to resolve if the book selection resulted in a single book.
   /*