From b249edee60d0a505e45bba47410d63c324fdaa9a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 9 Oct 2022 22:15:08 +0400 Subject: [PATCH] ETags for ZIM content use the ZIM file UUID --- src/server/internalServer.cpp | 31 ++++++++++++++++++++++++------- src/server/internalServer.h | 2 +- src/server/response.h | 3 ++- test/server.cpp | 15 +++++++++++++-- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 772b056e5..0583ba016 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -511,8 +511,10 @@ MHD_Result InternalServer::handlerCallback(struct MHD_Connection* connection, } } - if (response->getReturnCode() == MHD_HTTP_OK && !etag_not_needed(request)) - response->set_server_id(m_server_id); + if (response->getReturnCode() == MHD_HTTP_OK + && response->get_kind() == Response::DYNAMIC_CONTENT + && !etag_not_needed(request)) + response->set_etag_body(m_server_id); auto ret = response->send(request, connection); auto end_time = std::chrono::steady_clock::now(); @@ -542,7 +544,7 @@ std::unique_ptr InternalServer::handle_request(const RequestContext& r + urlNotFoundMsg; } - const ETag etag = get_matching_if_none_match_etag(request); + const ETag etag = get_matching_if_none_match_etag(request, m_server_id); if ( etag ) return Response::build_304(*this, etag); @@ -611,11 +613,11 @@ bool InternalServer::etag_not_needed(const RequestContext& request) const } ETag -InternalServer::get_matching_if_none_match_etag(const RequestContext& r) const +InternalServer::get_matching_if_none_match_etag(const RequestContext& r, const std::string& etagBody) const { try { const std::string etag_list = r.get_header(MHD_HTTP_HEADER_IF_NONE_MATCH); - return ETag::match(etag_list, m_server_id); + return ETag::match(etag_list, etagBody); } catch (const std::out_of_range&) { return ETag(); } @@ -1049,6 +1051,11 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r + suggestSearchMsg(searchURL, kiwix::urlDecode(pattern)); } + const std::string archiveUuid(archive->getUuid()); + const ETag etag = get_matching_if_none_match_etag(request, archiveUuid); + if ( etag ) + return Response::build_304(*this, etag); + auto urlStr = url.substr(prefixLength + bookName.size()); if (urlStr[0] == '/') { urlStr = urlStr.substr(1); @@ -1067,6 +1074,7 @@ std::unique_ptr InternalServer::handle_content(const RequestContext& r return build_redirect(bookName, getFinalItem(*archive, entry)); } auto response = ItemResponse::build(*this, request, entry.getItem()); + response->set_etag_body(archiveUuid); if (m_verbose.load()) { printf("Found %s\n", entry.getPath().c_str()); @@ -1120,6 +1128,11 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque + noSuchBookErrorMsg(bookName); } + const std::string archiveUuid(archive->getUuid()); + const ETag etag = get_matching_if_none_match_etag(request, archiveUuid); + if ( etag ) + return Response::build_304(*this, etag); + // Remove the beggining of the path: // /raw///foo // ^^^^^ ^ ^ @@ -1129,13 +1142,17 @@ std::unique_ptr InternalServer::handle_raw(const RequestContext& reque try { if (kind == "meta") { auto item = archive->getMetadataItem(itemPath); - return ItemResponse::build(*this, request, item); + auto response = ItemResponse::build(*this, request, item); + response->set_etag_body(archiveUuid); + return response; } else { auto entry = archive->getEntryByPath(itemPath); if (entry.isRedirect()) { return build_redirect(bookName, entry.getItem(true)); } - return ItemResponse::build(*this, request, entry.getItem()); + auto response = ItemResponse::build(*this, request, entry.getItem()); + response->set_etag_body(archiveUuid); + return response; } } catch (zim::EntryNotFound& e ) { if (m_verbose.load()) { diff --git a/src/server/internalServer.h b/src/server/internalServer.h index 6f523336e..48fab9c39 100644 --- a/src/server/internalServer.h +++ b/src/server/internalServer.h @@ -148,7 +148,7 @@ class InternalServer { MustacheData get_default_data() const; bool etag_not_needed(const RequestContext& r) const; - ETag get_matching_if_none_match_etag(const RequestContext& request) const; + ETag get_matching_if_none_match_etag(const RequestContext& request, const std::string& etagBody) const; std::pair selectBooks(const RequestContext& r) const; SearchInfo getSearchInfo(const RequestContext& r) const; diff --git a/src/server/response.h b/src/server/response.h index 31bcfc0da..4ed07e628 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -66,7 +66,8 @@ class Response { void set_code(int code) { m_returnCode = code; } void set_kind(Kind k); - void set_server_id(const std::string& id) { m_etag.set_body(id); } + Kind get_kind() const { return m_kind; } + void set_etag_body(const std::string& id) { m_etag.set_body(id); } void add_header(const std::string& name, const std::string& value) { m_customHeaders[name] = value; } int getReturnCode() const { return m_returnCode; } diff --git a/test/server.cpp b/test/server.cpp index 904c2361d..99fd3716a 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -1136,17 +1136,28 @@ TEST_F(ServerTest, ETagIsTheSameAcrossHeadAndGet) } } -TEST_F(ServerTest, DifferentServerInstancesProduceDifferentETags) +TEST_F(ServerTest, DifferentServerInstancesProduceDifferentETagsForDynamicContent) { ZimFileServer zfs2(SERVER_PORT + 1, ZimFileServer::DEFAULT_OPTIONS, ZIMFILES); for ( const Resource& res : all200Resources() ) { - if ( !res.etag_expected() ) continue; + if ( res.kind != DYNAMIC_CONTENT ) continue; const auto h1 = zfs1_->HEAD(res.url); const auto h2 = zfs2.HEAD(res.url); EXPECT_NE(h1->get_header_value("ETag"), h2->get_header_value("ETag")); } } +TEST_F(ServerTest, DifferentServerInstancesProduceIdenticalETagsForZimContent) +{ + ZimFileServer zfs2(SERVER_PORT + 1, ZimFileServer::DEFAULT_OPTIONS, ZIMFILES); + for ( const Resource& res : all200Resources() ) { + if ( res.kind != ZIM_CONTENT ) continue; + const auto h1 = zfs1_->HEAD(res.url); + const auto h2 = zfs2.HEAD(res.url); + EXPECT_EQ(h1->get_header_value("ETag"), h2->get_header_value("ETag")); + } +} + TEST_F(ServerTest, CompressionInfluencesETag) { for ( const Resource& res : resources200Compressible ) {