From 54f5dbbd3515ede50219a13799f378c67c30e7bb Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 14 May 2020 16:46:19 +0400 Subject: [PATCH] Handling of If-None-Match conditional requests --- src/server.cpp | 27 ++++++++++++++- src/server/etag.cpp | 71 ++++++++++++++++++++++++++++++++++++++ src/server/etag.h | 8 +++++ test/server.cpp | 83 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 188 insertions(+), 1 deletion(-) diff --git a/src/server.cpp b/src/server.cpp index ddd057838..c9372e899 100644 --- a/src/server.cpp +++ b/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 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(); diff --git a/src/server/etag.cpp b/src/server/etag.cpp index bf64348f3..1ba89470f 100644 --- a/src/server/etag.cpp +++ b/src/server/etag.cpp @@ -20,7 +20,10 @@ #include "etag.h" +#include "tools/stringTools.h" + #include +#include 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 diff --git a/src/server/etag.h b/src/server/etag.h index ec3b0077e..49ff0e724 100644 --- a/src/server/etag.h +++ b/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; diff --git a/test/server.cpp b/test/server.cpp index cc157992e..9d5dcd834 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -6,6 +6,22 @@ #include "./httplib.h" +using TestContextImpl = std::vector >; +struct TestContext : TestContextImpl { + TestContext(const std::initializer_list& 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); + } +}