Make sure that AppCache tests subresource URLrequests against NETWORK whitelist

webkit/appcache/appcache_request_handler.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/appcache/appcache_request_handler.h"
 
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_job.h"
 #include "webkit/appcache/appcache.h"
 #include "webkit/appcache/appcache_policy.h"
 #include "webkit/appcache/appcache_url_request_job.h"
 
 namespace appcache {
 
 AppCacheRequestHandler::AppCacheRequestHandler(
     AppCacheHost* host, ResourceType::Type resource_type)
     : host_(host), resource_type_(resource_type),
       is_waiting_for_cache_selection_(false), found_group_id_(0),
       found_cache_id_(0), found_network_namespace_(false),
-      cache_entry_not_found_(false) {
+      request_response_found_(false), cache_entry_not_found_(false) {
   DCHECK(host_);
   host_->AddObserver(this);
 }
 
 AppCacheRequestHandler::~AppCacheRequestHandler() {
   if (host_) {
     storage()->CancelDelegateCallbacks(this);
     host_->RemoveObserver(this);
   }
 }
@@ skipping 33 matching lines @@
     return NULL;
   }
 
   // Clear out our 'found' fields since we're starting a request for a
   // new resource, any values in those fields are no longer valid.
   found_entry_ = AppCacheEntry();
   found_fallback_entry_ = AppCacheEntry();
   found_cache_id_ = kNoCacheId;
   found_manifest_url_ = GURL();
   found_network_namespace_ = false;
+  request_response_found_ = false;
 
   if (is_main_resource())
     MaybeLoadMainResource(request);
   else
     MaybeLoadSubResource(request);
 
   // If its been setup to deliver a network response, we can just delete
   // it now and return NULL instead to achieve that since it couldn't
   // have been started yet.
   if (job_ && job_->is_delivering_network_response()) {
     DCHECK(!job_->has_been_started());
     job_ = NULL;
   }
 
   return job_;
 }
 
 AppCacheURLRequestJob* AppCacheRequestHandler::MaybeLoadFallbackForRedirect(
     net::URLRequest* request, const GURL& location) {
   if (!host_ || !IsSchemeAndMethodSupported(request) || cache_entry_not_found_)
     return NULL;
   if (is_main_resource())
     return NULL;
   if (request->url().GetOrigin() == location.GetOrigin())
     return NULL;
 
   DCHECK(!job_);  // our jobs never generate redirects
 

[michaeln, 2012/08/17 00:50:43]

I think there's an additional condition that has to be taken care of...

if (host_->is_selection_pending()) {
  // Stuff is being read out of the db on a background thread,
  // FindResponseForSubRequest() can't be used until it's loaded up.
  // See MaybeLoadSubResource() and OnCacheSelectionComplete() for
  // how that's handled when this class gets a chance to intercept
  // the request from the Start(). We provide a 'job' in that case,
  // but the 'job' just sits there and waits for OnCacheSelectionComplete()
  // prior to continuing.
}

[vabr (Chromium), 2012/08/17 16:22:35]

This should now be addressed by the call to MaybeLoadResources.

+  if (!request_response_found_) {
+    storage()->FindResponseForSubRequest(
+        host_->associated_cache(), request->url(),
+        &found_entry_, &found_fallback_entry_, &found_network_namespace_);

[michaeln, 2012/08/17 00:50:43]

What should we do if found_entry_.has_response_id() at this point? The
originally requested url is in the appcache, and we've been redirected via
WebRequest (presumably) to go elsewhere. There's no clause for that particular
case below. As coded, the appcache will synthesize an error and the page won't
work (despite there being a cached resource ready to go).

[vabr (Chromium), 2012/08/17 16:22:35]

Here I realised that I should use original_url(), not url().
Ideally, we should let the user know what is happening, but this is beyond this
CL and bug 141114 (it is within the scope of 121325). As a workaround, I chose
to let AppCache use the cached page. This would happen, if AppCache got its
hands on the request before it was hijacked via NetworkDelegate.

+    request_response_found_ = true;
+  }
   if (found_fallback_entry_.has_response_id()) {

[michaeln, 2012/08/17 00:50:43]

As coded we'll will load an appcached fallback resource and disregard the
redirect in this case where we've found a fallback resource. From the appcache
consumers point of view, the loading of a fallback resource is a network error
handling behavior. Not really sure that behavior is appropriate in this case.

[vabr (Chromium), 2012/08/17 16:22:35]

Through the use of MaybeLoadResource within MaybeLoadFallbackForRedirect we end
up serving fallback resource if the original_url is not cached, and also not
network-whitelisted (if I understood correctly that
found_fallback_entry_.has_response_id() and found_network_namespace_ cannot both
be true). Are there still situations here where serving the fallback would be
inappropriate?

     // 6.9.6, step 4: If this results in a redirect to another origin,
     // get the resource of the fallback entry.
     job_ = new AppCacheURLRequestJob(request, storage());
     DeliverAppCachedResponse(
         found_fallback_entry_, found_cache_id_, found_group_id_,
         found_manifest_url_,  true, found_namespace_entry_url_);
   } else if (!found_network_namespace_) {
     // 6.9.6, step 6: Fail the resource load.
     job_ = new AppCacheURLRequestJob(request, storage());
     DeliverErrorResponse();
@@ skipping 190 matching lines @@
   // equivalent ... then fetch the resource normally.
   DCHECK(job_);
   DCHECK(host_->associated_cache() &&
          host_->associated_cache()->is_complete());
 
   const GURL& url = job_->request()->url();
   AppCache* cache = host_->associated_cache();
   storage()->FindResponseForSubRequest(
       host_->associated_cache(), url,
       &found_entry_, &found_fallback_entry_, &found_network_namespace_);
+  request_response_found_ = true;
 
   if (found_entry_.has_response_id()) {
     // Step 2: If there's an entry, get it instead.
     DCHECK(!found_network_namespace_ &&
            !found_fallback_entry_.has_response_id());
     found_cache_id_ = cache->cache_id();
     found_group_id_ = cache->owning_group()->group_id();
     found_manifest_url_ = cache->owning_group()->manifest_url();
     DeliverAppCachedResponse(
         found_entry_, found_cache_id_, found_group_id_, found_manifest_url_,
@@ skipping 36 matching lines @@
   if (!host_->associated_cache() ||
       !host_->associated_cache()->is_complete()) {
     DeliverNetworkResponse();
     return;
   }
 
   ContinueMaybeLoadSubResource();
 }
 
 }  // namespace appcache