fix(http): evaluate If-None-Match lists and ignore If-Modified-Since with it

Makefile

@@ -414,6 +414,13 @@ unittest: prepare libhv
 ifeq ($(WITH_EVPP), yes)
 	$(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Issl -Ievent -Icpputil -Ievpp -o bin/tcpclient_dns_test unittest/tcpclient_dns_test.cpp -Llib -lhv -pthread
 endif
+ifeq ($(WITH_EVPP), yes)
+ifeq ($(WITH_HTTP_SERVER), yes)
+ifeq ($(WITH_HTTP_CLIENT), yes)
+	$(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Issl -Ievent -Icpputil -Ievpp -Ihttp -Ihttp/server -Ihttp/client -o bin/http_static_not_modified_test unittest/http_static_not_modified_test.cpp -Llib -lhv -pthread
+endif
+endif
+endif
 ifeq ($(WITH_HTTP), yes)
 	$(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Ihttp -o bin/websocket_parser_test unittest/websocket_parser_test.cpp -Llib -lhv -pthread
 endif

http/server/HttpHandler.cpp

@@ -24,6 +24,19 @@ using namespace hv;
 #define HTTP_200_CONNECT_RESPONSE       "HTTP/1.1 200 Connection established\r\n\r\n"
 #define HTTP_200_CONNECT_RESPONSE_LEN   39
 
+// Whether an If-None-Match value ("*" or a list of entity tags) matches etag,
+// using the weak comparison (RFC 9110 section 13.1.2).
+static bool if_none_match(const std::string& value, const char* etag) {
+    if (strncmp(etag, "W/", 2) == 0) etag += 2;
+    for (const auto& item : hv::split(value, ',')) {
+        std::string tag = hv::trim(item);
+        if (tag == "*") return true;
+        if (tag.compare(0, 2, "W/") == 0) tag.erase(0, 2);
+        if (tag == etag) return true;
+    }
+    return false;
+}
+
 HttpHandler::HttpHandler(hio_t* io) :
     protocol(HttpHandler::UNKNOWN),
     state(WANT_RECV),
@@ -688,19 +701,21 @@ int HttpHandler::defaultStaticHandler() {
         }
     }
     else {
-        // Not Modified
+        // Not Modified: If-Modified-Since is only evaluated when the request
+        // has no If-None-Match (RFC 9110 section 13.2.2)
         auto iter = req->headers.find("if-none-match");
-        if (iter != req->headers.end() &&
-            strcmp(iter->second.c_str(), fc->etag) == 0) {
-            fc = NULL;
-            return HTTP_STATUS_NOT_MODIFIED;
-        }
-
-        iter = req->headers.find("if-modified-since");
-        if (iter != req->headers.end() &&
-            strcmp(iter->second.c_str(), fc->last_modified) == 0) {
-            fc = NULL;
-            return HTTP_STATUS_NOT_MODIFIED;
+        if (iter != req->headers.end()) {
+            if (if_none_match(iter->second, fc->etag)) {
+                fc = NULL;
+                return HTTP_STATUS_NOT_MODIFIED;
+            }
+        } else {
+            iter = req->headers.find("if-modified-since");
+            if (iter != req->headers.end() &&
+                strcmp(iter->second.c_str(), fc->last_modified) == 0) {
+                fc = NULL;
+                return HTTP_STATUS_NOT_MODIFIED;
+            }
         }
     }
     return status_code;

scripts/unittest.sh

@@ -32,6 +32,9 @@ bin/socketpair_test
 # bin/objectpool_test
 bin/sizeof_test
 bin/http_router_test
+if [ -x bin/http_static_not_modified_test ]; then
+    bin/http_static_not_modified_test || exit $?
+fi
 if [ -x bin/websocket_parser_test ]; then
     bin/websocket_parser_test || exit $?
 fi

unittest/CMakeLists.txt

@@ -90,6 +90,13 @@ target_include_directories(sendmail PRIVATE .. ../base ../protocol ../util)
 add_executable(http_router_test http_router_test.cpp)
 target_include_directories(http_router_test PRIVATE ../http/server)
 
+if(WITH_EVPP AND WITH_HTTP AND WITH_HTTP_SERVER AND WITH_HTTP_CLIENT)
+add_executable(http_static_not_modified_test http_static_not_modified_test.cpp)
+target_include_directories(http_static_not_modified_test PRIVATE .. ../base ../ssl ../event ../cpputil ../evpp ../http ../http/server ../http/client)
+target_link_libraries(http_static_not_modified_test ${HV_LIBRARIES})
+set(HTTP_STATIC_UNITTEST_TARGETS http_static_not_modified_test)
+endif()
+
 if(WITH_HTTP)
 add_executable(websocket_parser_test websocket_parser_test.cpp)
 target_include_directories(websocket_parser_test PRIVATE .. ../base ../http)
@@ -237,6 +244,7 @@ add_custom_target(unittest DEPENDS
     sendmail
     http_router_test
     ${HTTP_UNITTEST_TARGETS}
+    ${HTTP_STATIC_UNITTEST_TARGETS}
     ${HTTP_LUA_UNITTEST_TARGETS}
     ${HTTP_JS_UNITTEST_TARGETS}
     ${EVENT_UNITTEST_TARGETS}

unittest/http_static_not_modified_test.cpp

@@ -0,0 +1,63 @@
+/*
+ * http_static_not_modified_test: If-None-Match and If-Modified-Since handled
+ * by the default static file handler (RFC 9110 section 13).
+ */
+
+#include <assert.h>
+#include <stdio.h>
+
+#include "hbase.h"
+#include "hfile.h"
+#include "htime.h"
+#include "HttpServer.h"
+#include "HttpService.h"
+#include "requests.h"
+
+static const char* url = "http://127.0.0.1:18083/a.txt";
+
+static int get(const char* name, const std::string& value, const char* name2 = NULL, const std::string& value2 = "") {
+    auto req = std::make_shared<HttpRequest>();
+    req->url = url;
+    req->headers[name] = value;
+    if (name2) req->headers[name2] = value2;
+    auto resp = requests::request(req);
+    assert(resp != NULL);
+    printf("%s: %s %s%s%s => %d\n", name, value.c_str(), name2 ? name2 : "", name2 ? ": " : "", value2.c_str(), (int)resp->status_code);
+    return resp->status_code;
+}
+
+int main() {
+    hv_mkdir_p("tmp/http_static_not_modified_test");
+    HFile file;
+    int ret = file.open("tmp/http_static_not_modified_test/a.txt", "wb");
+    assert(ret == 0);
+    file.write("0123456789", 10);
+    file.close();
+
+    HttpService service;
+    service.document_root = "tmp/http_static_not_modified_test";
+
+    hv::HttpServer server(&service);
+    server.setPort(18083);
+    server.start();
+    hv_msleep(200);
+
+    auto resp = requests::get(url);
+    assert(resp != NULL && resp->status_code == 200);
+    std::string etag = resp->GetHeader("Etag");
+    std::string last_modified = resp->GetHeader("Last-Modified");
+    assert(!etag.empty() && !last_modified.empty());
+
+    assert(get("If-None-Match", etag) == 304);
+    assert(get("If-None-Match", "\"other\", " + etag) == 304);
+    assert(get("If-None-Match", "*") == 304);
+    assert(get("If-None-Match", "W/" + etag) == 304);
+    assert(get("If-None-Match", "\"other\"") == 200);
+    assert(get("If-Modified-Since", last_modified) == 304);
+    // If-Modified-Since is ignored when If-None-Match is present
+    assert(get("If-None-Match", "\"other\"", "If-Modified-Since", last_modified) == 200);
+
+    server.stop();
+    printf("http_static_not_modified_test passed\n");
+    return 0;
+}