From f281e6ae8a4f1e6d20a269a984d92dad9f49dcf2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= <mail@kkaefer.com>
Date: Fri, 22 Sep 2017 12:05:22 +0200
Subject: [core] fix loading of stale must-revalidate resources

In https://github.com/mapbox/mapbox-gl-native/pull/9670, we implemented support for the Cache-Control: must-revalidate header. While we now respect this header and don't show resources that are stale and have this header set, the optional cache request never completes. This means we're also never going to try to actually get a fresh tile and just never show this tile anymore.

This commit fixes this by responding with a Not Found error when the resource is unusable (= stale and has must-revalidate set). Since we actually still have the data (but can't use it due to caching rules), we're responding with the data as well. To avoid a second cache hit, tile_loader_impl.hpp now passes on the data from the Optional to the Required request so that it can be reused when we get a 304 Not Modified response from the server.
---
 platform/default/default_file_source.cpp | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

(limited to 'platform/default/default_file_source.cpp')

diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp
index 1e855dd9f7..3fdb03e6b4 100644
--- a/platform/default/default_file_source.cpp
+++ b/platform/default/default_file_source.cpp
@@ -126,9 +126,8 @@ public:
             tasks[req] = localFileSource->request(resource, callback);
         } else {
             // Try the offline database
-            Resource revalidation = resource;
-
-            const bool hasPrior = resource.priorEtag || resource.priorModified || resource.priorExpires;
+            const bool hasPrior = resource.priorEtag || resource.priorModified ||
+                                  resource.priorExpires || resource.priorData;
             if (!hasPrior || resource.necessity == Resource::Optional) {
                 auto offlineResponse = offlineDatabase->get(resource);
 
@@ -142,27 +141,34 @@ public:
                 }
 
                 if (offlineResponse) {
-                    revalidation.priorModified = offlineResponse->modified;
-                    revalidation.priorExpires = offlineResponse->expires;
-                    revalidation.priorEtag = offlineResponse->etag;
+                    resource.priorModified = offlineResponse->modified;
+                    resource.priorExpires = offlineResponse->expires;
+                    resource.priorEtag = offlineResponse->etag;
 
                     // Don't return resources the server requested not to show when they're stale.
                     // Even if we can't directly use the response, we may still use it to send a
                     // conditional HTTP request.
                     if (offlineResponse->isUsable()) {
                         callback(*offlineResponse);
+                    } else if (resource.necessity == Resource::Optional) {
+                        // Instead of the data that we got, return a not found error so that
+                        // underlying implementations know about the fact that we couldn't find
+                        // usable cache data.
+                        offlineResponse->error = std::make_unique<Response::Error>(
+                            Response::Error::Reason::NotFound, "Cached resource is unusable");
+                        callback(*offlineResponse);
                     } else {
                         // Since we can't return the data immediately, we'll have to hold on so that
                         // we can return it later in case we get a 304 Not Modified response.
-                        revalidation.priorData = offlineResponse->data;
+                        resource.priorData = offlineResponse->data;
                     }
                 }
             }
 
             // Get from the online file source
             if (resource.necessity == Resource::Required) {
-                tasks[req] = onlineFileSource.request(revalidation, [=] (Response onlineResponse) mutable {
-                    this->offlineDatabase->put(revalidation, onlineResponse);
+                tasks[req] = onlineFileSource.request(resource, [=] (Response onlineResponse) mutable {
+                    this->offlineDatabase->put(resource, onlineResponse);
                     callback(onlineResponse);
                 });
             }
-- 
cgit v1.2.1