[libsoup] cache: Do not revalidate resources that haven't expired even if must-revalidate is present

[Carlos Garcia Campos <carlosgc src gnome org>]

commit 8dcc74e0dd278c6cf0be119aa7f6f969f8418649
Author: Carlos Garcia Campos <cgarcia igalia com>
Date:   Fri Feb 13 13:39:50 2015 +0100

    cache: Do not revalidate resources that haven't expired even if must-revalidate is present
    
    RFC2616 14.9.4
    
    "When the must-revalidate directive is present in a response received by
    a cache, that cache MUST NOT use the entry after it becomes stale to
    respond to a subsequent request without first revalidating it with the origin server.
    (I.e., the cache MUST do an end-to-end revalidation every time, if,
    based solely on the origin server's Expires or max-age value, the cached
    response is stale.)"
    
    Update also the unit tests, since they were checking the wrong
    behaviour.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=744468

 libsoup/soup-cache.c |   13 +++++++++----
 tests/cache-test.c   |   10 +++++++++-
 2 files changed, 18 insertions(+), 5 deletions(-)
---
diff --git a/libsoup/soup-cache.c b/libsoup/soup-cache.c
index e953816..d118c4c 100644
--- a/libsoup/soup-cache.c
+++ b/libsoup/soup-cache.c
@@ -1186,12 +1186,17 @@ soup_cache_has_response (SoupCache *cache, SoupMessage *msg)
        /* 6. The stored response is either: fresh, allowed to be
         * served stale or succesfully validated
         */
-       /* TODO consider also proxy-revalidate & s-maxage */
-       if (entry->must_revalidate)
-               return SOUP_CACHE_RESPONSE_NEEDS_VALIDATION;
-
        if (!soup_cache_entry_is_fresh_enough (entry, min_fresh)) {
                /* Not fresh, can it be served stale? */
+
+               /* When the must-revalidate directive is present in a
+                * response received by a cache, that cache MUST NOT
+                * use the entry after it becomes stale
+                */
+               /* TODO consider also proxy-revalidate & s-maxage */
+               if (entry->must_revalidate)
+                       return SOUP_CACHE_RESPONSE_NEEDS_VALIDATION;
+
                if (max_stale != -1) {
                        /* G_MAXINT32 means we accept any staleness */
                        if (max_stale == G_MAXINT32)
diff --git a/tests/cache-test.c b/tests/cache-test.c
index 543ef02..f332a3f 100644
--- a/tests/cache-test.c
+++ b/tests/cache-test.c
@@ -288,9 +288,11 @@ do_basics_test (gconstpointer data)
                            NULL);
        body2 = do_request (session, base_uri, "GET", "/2", NULL,
                            "Test-Set-Last-Modified", "Fri, 01 Jan 2010 00:00:00 GMT",
+                           "Test-Set-Cache-Control", "must-revalidate",
                            NULL);
        body3 = do_request (session, base_uri, "GET", "/3", NULL,
                            "Test-Set-Last-Modified", "Fri, 01 Jan 2010 00:00:00 GMT",
+                           "Test-Set-Expires", "Sat, 02 Jan 2011 00:00:00 GMT",
                            "Test-Set-Cache-Control", "must-revalidate",
                            NULL);
        body4 = do_request (session, base_uri, "GET", "/4", NULL,
@@ -316,6 +318,9 @@ do_basics_test (gconstpointer data)
        debug_printf (1, "  Heuristically-fresh cached resource\n");
        cmp = do_request (session, base_uri, "GET", "/2", NULL,
                          NULL);
+       /* Not validated even if it has must-revalidate, because it hasn't expired */
+       soup_test_assert (!last_request_validated,
+                         "Request for /2 was validated");
        soup_test_assert (!last_request_hit_network,
                          "Request for /2 not filled from cache");
        g_assert_cmpstr (body2, ==, cmp);
@@ -338,10 +343,11 @@ do_basics_test (gconstpointer data)
        g_free (cmp);
 
 
-       /* Last-Modified + must-revalidate causes a conditional request */
+       /* Expired + must-revalidate causes a conditional request */
        debug_printf (1, "  Unchanged must-revalidate resource w/ Last-Modified\n");
        cmp = do_request (session, base_uri, "GET", "/3", NULL,
                          "Test-Set-Last-Modified", "Fri, 01 Jan 2010 00:00:00 GMT",
+                         "Test-Set-Expires", "Sat, 02 Jan 2011 00:00:00 GMT",
                          "Test-Set-Cache-Control", "must-revalidate",
                          NULL);
        soup_test_assert (last_request_validated,
@@ -356,6 +362,7 @@ do_basics_test (gconstpointer data)
        debug_printf (1, "  Changed must-revalidate resource w/ Last-Modified\n");
        cmp = do_request (session, base_uri, "GET", "/3", NULL,
                          "Test-Set-Last-Modified", "Sat, 02 Jan 2010 00:00:00 GMT",
+                         "Test-Set-Expires", "Sat, 02 Jan 2011 00:00:00 GMT",
                          "Test-Set-Cache-Control", "must-revalidate",
                          NULL);
        soup_test_assert (last_request_validated,
@@ -455,6 +462,7 @@ do_cancel_test (gconstpointer data)
                            NULL);
        body2 = do_request (session, base_uri, "GET", "/2", NULL,
                            "Test-Set-Last-Modified", "Fri, 01 Jan 2010 00:00:00 GMT",
+                           "Test-Set-Expires", "Fri, 01 Jan 2011 00:00:00 GMT",
                            "Test-Set-Cache-Control", "must-revalidate",
                            NULL);