Handling of If-None-Match conditional requests

src/server.cpp

@@ -111,10 +111,11 @@ class InternalServer {
     bool start();
     void stop();
 
-  private:
+  private: // functions
     Response handle_request(const RequestContext& request);
     Response build_500(const std::string& msg);
     Response build_404(const RequestContext& request, const std::string& zimName);
+    Response build_304(const RequestContext& request, const ETag& etag) const;
     Response build_redirect(const std::string& bookName, const kiwix::Entry& entry) const;
     Response build_homepage(const RequestContext& request);
     Response handle_skin(const RequestContext& request);
@@ -132,6 +133,7 @@ class InternalServer {
 
     std::shared_ptr<Reader> get_reader(const std::string& bookName) const;
     bool etag_not_needed(const RequestContext& r) const;
+    ETag get_matching_if_none_match_etag(const RequestContext& request) const;
 
   private: // data
     std::string m_addr;
@@ -337,6 +339,14 @@ int InternalServer::handlerCallback(struct MHD_Connection* connection,
   return ret;
 }
 
+Response InternalServer::build_304(const RequestContext& request, const ETag& etag) const
+{
+  auto response = get_default_response();
+  response.set_code(MHD_HTTP_NOT_MODIFIED);
+  response.set_etag(etag);
+  response.set_content("");
+  return response;
+}
 
 Response InternalServer::handle_request(const RequestContext& request)
 {
@@ -344,6 +354,10 @@ Response InternalServer::handle_request(const RequestContext& request)
     if (! request.is_valid_url())
       return build_404(request, "");
 
+    const ETag etag = get_matching_if_none_match_etag(request);
+    if ( etag )
+      return build_304(request, etag);
+
     if (kiwix::startsWith(request.get_url(), "/skin/"))
       return handle_skin(request);
 
@@ -446,6 +460,17 @@ bool InternalServer::etag_not_needed(const RequestContext& request) const
       || url == "/catch/external";
 }
 
+ETag
+InternalServer::get_matching_if_none_match_etag(const RequestContext& r) const
+{
+  try {
+    const std::string etag_list = r.get_header(MHD_HTTP_HEADER_IF_NONE_MATCH);
+    return ETag::match(etag_list, m_server_id);
+  } catch (const std::out_of_range&) {
+    return ETag();
+  }
+}
+
 Response InternalServer::build_homepage(const RequestContext& request)
 {
   auto response = get_default_response();

src/server/etag.cpp

@@ -20,7 +20,10 @@
 
 #include "etag.h"
 
+#include "tools/stringTools.h"
+
 #include <algorithm>
+#include <sstream>
 
 namespace kiwix {
 
@@ -32,10 +35,35 @@ namespace {
 // this file). However it is better to have some mnemonics in the option names,
 // hence below variable: all_options[opt] corresponds to the character going
 // into the ETag for ETag::Option opt.
+// IMPORTANT: The characters in all_options must come in sorted order (so that
+// IMPORTANT: isValidOptionsString() works correctly).
 const char all_options[] = "cz";
 
 static_assert(ETag::OPTION_COUNT == sizeof(all_options) - 1, "");
 
+bool isValidServerId(const std::string& s)
+{
+  return !s.empty() && s.find_first_of("\"/") == std::string::npos;
+}
+
+bool isSubsequenceOf(const std::string& s, const std::string& sortedString)
+{
+  std::string::size_type i = 0;
+  for ( const char c : s )
+  {
+    const std::string::size_type j = sortedString.find(c, i);
+    if ( j == std::string::npos )
+      return false;
+    i = j+1;
+  }
+  return true;
+}
+
+bool isValidOptionsString(const std::string& s)
+{
+  return isSubsequenceOf(s, all_options);
+}
+
 } // namespace
 
 
@@ -61,4 +89,47 @@ std::string ETag::get_etag() const
   return "\"" + m_serverId + "/" + m_options + "\"";
 }
 
+ETag::ETag(const std::string& serverId, const std::string& options)
+{
+  if ( isValidServerId(serverId) && isValidOptionsString(options) )
+  {
+    m_serverId = serverId;
+    m_options = options;
+  }
+}
+
+ETag ETag::parse(std::string s)
+{
+  if ( kiwix::startsWith("W/", s) )
+    s = s.substr(2);
+
+  if ( s.front() != '"' || s.back() != '"' )
+    return ETag();
+
+  s = s.substr(1, s.size()-2);
+
+  const std::string::size_type i = s.find('/');
+  if ( i == std::string::npos )
+    return ETag();
+
+  return ETag(s.substr(0, i), s.substr(i+1));
+}
+
+ETag ETag::match(const std::string& etags, const std::string& server_id)
+{
+  std::istringstream ss(etags);
+  std::string etag_str;
+  while ( ss >> etag_str )
+  {
+    if ( etag_str.back() == ',' )
+      etag_str.pop_back();
+
+    const ETag etag = parse(etag_str);
+    if ( etag && etag.m_serverId == server_id )
+      return etag;
+  }
+
+  return ETag();
+}
+
 } // namespace kiwix

src/server/etag.h

@@ -67,6 +67,14 @@ class ETag
     bool get_option(Option opt) const;
     std::string get_etag() const;
 
+
+    static ETag match(const std::string& etags, const std::string& server_id);
+
+  private: // functions
+    ETag(const std::string& serverId, const std::string& options);
+
+    static ETag parse(std::string s);
+
   private: // data
     std::string m_serverId;
     std::string m_options;

test/server.cpp

@@ -6,6 +6,22 @@
 
 #include "./httplib.h"
 
+using TestContextImpl = std::vector<std::pair<std::string, std::string> >;
+struct TestContext : TestContextImpl {
+  TestContext(const std::initializer_list<value_type>& il)
+    : TestContextImpl(il)
+  {}
+};
+
+std::ostream& operator<<(std::ostream& out, const TestContext& ctx)
+{
+  out << "Test context:\n";
+  for ( const auto& kv : ctx )
+    out << "\t" << kv.first << ": " << kv.second << "\n";
+  out << std::endl;
+  return out;
+}
+
 bool is_valid_etag(const std::string& etag)
 {
   return etag.size() >= 2 &&
@@ -323,3 +339,70 @@ TEST_F(ServerTest, ETagOfUncompressibleContentIsNotAffectedByAcceptEncoding)
     EXPECT_EQ(etag, g3->get_header_value("ETag")) << res;
   }
 }
+
+// Pick from the response those headers that are required to be present in the
+// 304 (Not Modified) response if they would be set in the 200 (OK) response.
+// NOTE: The "Date" header (which should belong to that list as required
+// NOTE: by RFC 7232) is not included (since the result of this function
+// NOTE: will be used to check the equality of headers from the 200 and 304
+// NOTe: responses).
+Headers special304Headers(const httplib::Response& r)
+{
+  Headers result;
+  std::copy_if(
+    r.headers.begin(), r.headers.end(),
+    std::inserter(result, result.end()),
+    [](const Headers::value_type& x) {
+       return x.first == "Cache-Control"
+           || x.first == "Content-Location"
+           || x.first == "ETag"
+           || x.first == "Expires"
+           || x.first == "Vary";
+  });
+  return result;
+}
+
+// make a list of three etags with the given one in the middle
+std::string make_etag_list(const std::string& etag)
+{
+  return "\"x" + etag.substr(1) + ", "
+       + etag + ", "
+       + etag.substr(0, etag.size()-2) + "\"";
+}
+
+TEST_F(ServerTest, IfNoneMatchRequestsWithMatchingETagResultIn304Responses)
+{
+  const char* const encodings[] = { "", "deflate" };
+  for ( const Resource& res : all200Resources() ) {
+    for ( const char* enc: encodings ) {
+      if ( ! res.etag_expected ) continue;
+      const TestContext ctx{ {"url", res.url}, {"encoding", enc} };
+
+      const auto g = zfs1_->GET(res.url, { {"Accept-Encoding", enc} });
+      const auto etag = g->get_header_value("ETag");
+
+      const std::string etags = make_etag_list(etag);
+      const Headers headers{{"If-None-Match", etags}, {"Accept-Encoding", enc}};
+      const auto g2 = zfs1_->GET(res.url, headers );
+      const auto h = zfs1_->HEAD(res.url, headers );
+      EXPECT_EQ(304, h->status) << ctx;
+      EXPECT_EQ(304, g2->status) << ctx;
+      EXPECT_EQ(special304Headers(*g), special304Headers(*g2)) << ctx;
+      EXPECT_EQ(special304Headers(*g2), special304Headers(*h)) << ctx;
+      EXPECT_TRUE(g2->body.empty()) << ctx;
+    }
+  }
+}
+
+TEST_F(ServerTest, IfNoneMatchRequestsWithMismatchingETagResultIn200Responses)
+{
+  for ( const Resource& res : all200Resources() ) {
+    const auto g = zfs1_->GET(res.url);
+    const auto etag = g->get_header_value("ETag");
+    const auto etag2 = etag.substr(0, etag.size() - 1) + "x\"";
+    const auto h = zfs1_->HEAD(res.url, { {"If-None-Match", etag2} } );
+    const auto g2 = zfs1_->GET(res.url, { {"If-None-Match", etag2} } );
+    EXPECT_EQ(200, h->status);
+    EXPECT_EQ(200, g2->status);
+  }
+}