Added support for cache control in libprocess when dealing with static files.
When serving a static file, libprocess returns the header `Last-Modified` which is used by browsers to control Cache. When a http request arrives containing the header `If-Modified-Since`, a response `304 Not Modified` is returned if the date in the request and the modification time (as returned by doing `stat` in the file) coincide. Unit tests added. Review: https://reviews.apache.org/r/30032 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d0300e1a Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d0300e1a Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d0300e1a Branch: refs/heads/master Commit: d0300e1a47d1ba5d6714957fc258ab125fd53ed1 Parents: 2dc9f5e Author: Alexander Rojas <[email protected]> Authored: Thu Jul 9 17:30:02 2015 +0200 Committer: Till Toenshoff <[email protected]> Committed: Thu Jul 9 17:35:18 2015 +0200 ---------------------------------------------------------------------- 3rdparty/libprocess/include/process/http.hpp | 16 ++++ 3rdparty/libprocess/src/process.cpp | 62 ++++++++++++- 3rdparty/libprocess/src/tests/process_tests.cpp | 96 ++++++++++++++++++++ 3 files changed, 173 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/d0300e1a/3rdparty/libprocess/include/process/http.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/include/process/http.hpp b/3rdparty/libprocess/include/process/http.hpp index 8d9adc5..32a2d84 100644 --- a/3rdparty/libprocess/include/process/http.hpp +++ b/3rdparty/libprocess/include/process/http.hpp @@ -18,6 +18,7 @@ #include <process/future.hpp> #include <process/owned.hpp> #include <process/pid.hpp> +#include <process/time.hpp> #include <stout/error.hpp> #include <stout/hashmap.hpp> @@ -367,6 +368,21 @@ struct Accepted : Response }; +struct NotModified : Response +{ + explicit NotModified(const Time& lastModified) + { + status = "304 Not Modified"; + + std::string mtime = stringify(RFC1123(lastModified)); + + if (!mtime.empty()) { + headers["Last-Modified"] = mtime; + } + } +}; + + struct TemporaryRedirect : Response { explicit TemporaryRedirect(const std::string& url) http://git-wip-us.apache.org/repos/asf/mesos/blob/d0300e1a/3rdparty/libprocess/src/process.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp index c3e1763..9037b98 100644 --- a/3rdparty/libprocess/src/process.cpp +++ b/3rdparty/libprocess/src/process.cpp @@ -8,6 +8,7 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <time.h> #include <unistd.h> #include <arpa/inet.h> @@ -80,6 +81,8 @@ #include <stout/thread.hpp> #include <stout/unreachable.hpp> +#include <stout/os/stat.hpp> + #include "config.hpp" #include "decoder.hpp" #include "encoder.hpp" @@ -93,6 +96,7 @@ using namespace process::firewall; using namespace process::metrics::internal; +using process::RFC1123; using process::wait; // Necessary on some OS's to disambiguate. using process::http::Accepted; @@ -100,6 +104,7 @@ using process::http::BadRequest; using process::http::Forbidden; using process::http::InternalServerError; using process::http::NotFound; +using process::http::NotModified; using process::http::OK; using process::http::Request; using process::http::Response; @@ -3009,6 +3014,53 @@ void ProcessBase::visit(const HttpEvent& event) } } + // Get the modification time of the file. + // TODO(arojas): Use Path.mtime() once it is available. + const Try<long> systemMtime = os::stat::mtime(response.path); + const Try<Time> mtime = systemMtime.isSome() + ? Time::create(systemMtime.get()) + : Error(systemMtime.error()); + + bool modified = true; + + // In order to use the HTTP cache the following is needed: + // 1. Request must include the header 'If-Modified-Since' and it + // must be a valid date. + // 2. Modification time from the resource must be determined. + // 3. Both times must be equal. + // However, if any of the conditions are not met, there's no need + // to propagate the error. If a condition is not satisfied, the + // whole file is returned and the appropriate reason is logged. + if (mtime.isSome() && + event.request->headers.contains("If-Modified-Since")) { + tm clientMtime = {}; + + if (strptime( + event.request->headers["If-Modified-Since"].c_str(), + "%a, %d %b %Y %T %Z", + &clientMtime) != NULL) { + const time_t client = std::mktime(&clientMtime); + const time_t server = static_cast<time_t>(mtime.get().secs()); + + if (client != -1 && client == server) { + modified = false; + } else if (client == -1 || server == -1) { + VLOG(1) << "Failed to create 'time_t' through mktime"; + } + } else { + VLOG(1) << "Failed to parse time."; + } + } + + // Provide the Last-Modified header to the client so it can set up + // the cache. + if (mtime.isSome()) { + response.headers["Last-Modified"] = stringify(RFC1123(mtime.get())); + } else { + VLOG(1) << "Failed to get mtime for " << response.path << ": " + << mtime.error(); + } + // TODO(benh): Use "text/plain" for assets that don't have an // extension or we don't have a mapping for? It might be better to // just let the browser guess (or do it's own default). @@ -3018,7 +3070,15 @@ void ProcessBase::visit(const HttpEvent& event) // Enqueue the response with the HttpProxy so that it respects the // order of requests to account for HTTP/1.1 pipelining. - dispatch(proxy, &HttpProxy::enqueue, response, *event.request); + if (modified) { + dispatch(proxy, &HttpProxy::enqueue, response, *event.request); + } else { + dispatch( + proxy, + &HttpProxy::enqueue, + NotModified(mtime.get()), + *event.request); + } } else { VLOG(1) << "Returning '404 Not Found' for '" << event.request->path << "'"; http://git-wip-us.apache.org/repos/asf/mesos/blob/d0300e1a/3rdparty/libprocess/src/tests/process_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/tests/process_tests.cpp b/3rdparty/libprocess/src/tests/process_tests.cpp index f123ed4..4fa834d 100644 --- a/3rdparty/libprocess/src/tests/process_tests.cpp +++ b/3rdparty/libprocess/src/tests/process_tests.cpp @@ -1,3 +1,5 @@ +#include <time.h> + #include <arpa/inet.h> #include <gmock/gmock.h> @@ -1679,6 +1681,100 @@ TEST(ProcessTest, Provide) } +// Requests a static resource via HTTP and validates if proper answers +// are returned for both cache hits and misses. +TEST(ProcessTest, Cache) +{ + const Try<string>& mkdtemp = os::mkdtemp(); + ASSERT_SOME(mkdtemp); + + const string LOREM_IPSUM = + "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do " + "eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad " + "minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip " + "ex ea commodo consequat. Duis aute irure dolor in reprehenderit in " + "voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur " + "sint occaecat cupidatat non proident, sunt in culpa qui officia " + "deserunt mollit anim id est laborum."; + + const string path = path::join(mkdtemp.get(), "lorem.txt"); + ASSERT_SOME(os::write(path, LOREM_IPSUM)); + + FileServer server(path); + PID<FileServer> pid = spawn(server); + + // First request. + Future<http::Response> response = http::get(pid); + + AWAIT_READY(response); + + // First request had no information about the served file, so a 200 + // code is expected as well as the full body. + EXPECT_EQ("200 OK", response.get().status); + ASSERT_TRUE(response.get().headers.contains("Last-Modified")); + + // Request includes proper cache headers. + const std::string mtime = response.get().headers.get("Last-Modified").get(); + + hashmap<std::string, std::string> headers; + headers.put("Cache-Control", "max-age=0"); + headers.put("If-Modified-Since", mtime); + + response = http::get(pid, None(), None(), headers); + + AWAIT_READY(response); + ASSERT_EQ("304 Not Modified", response.get().status); + ASSERT_TRUE(response.get().headers.contains("Last-Modified")); + EXPECT_SOME_EQ(mtime, response.get().headers.get("Last-Modified")); + + static const char HTTP_DATE[] = "%a, %d %b %Y %T %Z"; + + // Use a different modification time for the request. The whole file + // should be served. + tm tmTime = {}; + strptime(mtime.c_str(), HTTP_DATE, &tmTime); + time_t earlierTime = mktime(&tmTime); + ASSERT_NE(-1, earlierTime); + + // Just an arbitrary earlier time than the mtime. In this case, 100 + // seconds. + earlierTime -= 100; + + // Transform the time back to a string. + ASSERT_TRUE(NULL != gmtime_r(&earlierTime, &tmTime)); + static const size_t BUFFER_SIZE = 200; + char buffer[BUFFER_SIZE] = {0}; + ASSERT_NE(0, strftime(buffer, BUFFER_SIZE, HTTP_DATE, &tmTime)); + std::string strTime = buffer; + headers.put("If-Modified-Since", strTime); + + response = http::get(pid, None(), None(), headers); + + AWAIT_READY(response); + + // Request 'If-Modified-Since' header doesn't match the server's + // mtime. Full file expected. + EXPECT_EQ("200 OK", response.get().status); + EXPECT_TRUE(response.get().headers.contains("Last-Modified")); + EXPECT_EQ(LOREM_IPSUM, response.get().body); + + // Finally, test with an unparseable date. + headers.put("If-Modified-Since", "XXX-YYY-ZZZ PPP"); + response = http::get(pid, None(), None(), headers); + + // Request 'If-Modified-Since' header doesn't match the server's + // mtime. Full file expected. + EXPECT_EQ("200 OK", response.get().status); + EXPECT_TRUE(response.get().headers.contains("Last-Modified")); + EXPECT_EQ(LOREM_IPSUM, response.get().body); + + terminate(server); + wait(server); + + ASSERT_SOME(os::rmdir(path)); +} + + int baz(string s) { return 42; } Future<int> bam(string s) { return 42; }
