Commit 126e3f89 authored by mkwst@chromium.org's avatar mkwst@chromium.org

Revert of Revert of Mixed Content: Move subframe checks into ResourceFetcher....

Revert of Revert of Mixed Content: Move subframe checks into ResourceFetcher. (patchset #1 id:1 of https://codereview.chromium.org/550083003/)

Reason for revert:
This wasn't the root cause of the non-V8 context call.

Original issue's description:
> Revert of Mixed Content: Move subframe checks into ResourceFetcher. (patchset #2 id:40001 of https://codereview.chromium.org/544573002/)
> 
> Reason for revert:
> calls V8 from a non-V8 context
> 
> Original issue's description:
> > Mixed Content: Move subframe checks into ResourceFetcher.
> > 
> > Currently we're checking for mixed content when loading the main
> > resource of subframes in two places: DocumentLoader and ResourceFetcher.
> > The former properly checks against the frame's parent frame, while the
> > latter bypasses the checks entirely by virtue of the request's
> > 'Resource::Type' being 'Main'.
> > 
> > This is both confusing and difficult to replicate when moving to the
> > brave new world of request contexts and frame types that Fetch now
> > defines.
> > 
> > This patch drops the DocumentLoader check, and moves the relevant
> > logic into ResourceFetcher::canRequest and
> > ResourceFetcher::checkInsecureContent.
> > 
> > BUG=400087
> > 
> > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181383
> 
> TBR=mkwst@chromium.org
> NOTREECHECKS=true
> NOTRY=true
> BUG=400087
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181543

TBR=jochen@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=400087

Review URL: https://codereview.chromium.org/559333002

git-svn-id: svn://svn.chromium.org/blink/trunk@181798 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent a1e2c4f6
...@@ -302,7 +302,7 @@ ResourcePtr<ImageResource> ResourceFetcher::fetchImage(FetchRequest& request) ...@@ -302,7 +302,7 @@ ResourcePtr<ImageResource> ResourceFetcher::fetchImage(FetchRequest& request)
if (LocalFrame* f = frame()) { if (LocalFrame* f = frame()) {
if (f->document()->pageDismissalEventBeingDispatched() != Document::NoDismissal) { if (f->document()->pageDismissalEventBeingDispatched() != Document::NoDismissal) {
KURL requestURL = request.resourceRequest().url(); KURL requestURL = request.resourceRequest().url();
if (requestURL.isValid() && canRequest(Resource::Image, requestURL, request.options(), request.forPreload(), request.originRestriction())) if (requestURL.isValid() && canRequest(Resource::Image, request.resourceRequest(), requestURL, request.options(), request.forPreload(), request.originRestriction()))
PingLoader::loadImage(f, requestURL); PingLoader::loadImage(f, requestURL);
return 0; return 0;
} }
...@@ -431,7 +431,7 @@ void ResourceFetcher::preCacheSubstituteDataForMainResource(const FetchRequest& ...@@ -431,7 +431,7 @@ void ResourceFetcher::preCacheSubstituteDataForMainResource(const FetchRequest&
memoryCache()->add(resource.get()); memoryCache()->add(resource.get());
} }
bool ResourceFetcher::checkInsecureContent(Resource::Type type, const KURL& url, MixedContentBlockingTreatment treatment) const bool ResourceFetcher::checkInsecureContent(Resource::Type type, const KURL& url, LocalFrame* frame, MixedContentBlockingTreatment treatment) const
{ {
if (treatment == TreatAsDefaultForType) { if (treatment == TreatAsDefaultForType) {
switch (type) { switch (type) {
...@@ -468,32 +468,33 @@ bool ResourceFetcher::checkInsecureContent(Resource::Type type, const KURL& url, ...@@ -468,32 +468,33 @@ bool ResourceFetcher::checkInsecureContent(Resource::Type type, const KURL& url,
break; break;
} }
} }
// No frame, no mixed content.
if (!frame)
return true;
if (treatment == TreatAsActiveContent) { if (treatment == TreatAsActiveContent) {
if (LocalFrame* f = frame()) { if (!frame->loader().mixedContentChecker()->canRunInsecureContent(frame->document()->securityOrigin(), url))
if (!f->loader().mixedContentChecker()->canRunInsecureContent(m_document->securityOrigin(), url)) return false;
return false;
}
} else if (treatment == TreatAsPassiveContent) { } else if (treatment == TreatAsPassiveContent) {
if (LocalFrame* f = frame()) { if (!frame->loader().mixedContentChecker()->canDisplayInsecureContent(frame->document()->securityOrigin(), url))
if (!f->loader().mixedContentChecker()->canDisplayInsecureContent(m_document->securityOrigin(), url)) return false;
return false; if (MixedContentChecker::isMixedContent(frame->document()->securityOrigin(), url) || MixedContentChecker::isMixedContent(toLocalFrame(frame->tree().top())->document()->securityOrigin(), url)) {
if (MixedContentChecker::isMixedContent(f->document()->securityOrigin(), url) || MixedContentChecker::isMixedContent(toLocalFrame(frame()->tree().top())->document()->securityOrigin(), url)) { switch (type) {
switch (type) { case Resource::Raw:
case Resource::Raw: UseCounter::count(frame->document(), UseCounter::MixedContentRaw);
UseCounter::count(f->document(), UseCounter::MixedContentRaw); break;
break;
case Resource::Image:
case Resource::Image: UseCounter::count(frame->document(), UseCounter::MixedContentImage);
UseCounter::count(f->document(), UseCounter::MixedContentImage); break;
break;
case Resource::Media:
case Resource::Media: UseCounter::count(frame->document(), UseCounter::MixedContentMedia);
UseCounter::count(f->document(), UseCounter::MixedContentMedia); break;
break;
default:
default: ASSERT_NOT_REACHED();
ASSERT_NOT_REACHED();
}
} }
} }
} else { } else {
...@@ -502,7 +503,7 @@ bool ResourceFetcher::checkInsecureContent(Resource::Type type, const KURL& url, ...@@ -502,7 +503,7 @@ bool ResourceFetcher::checkInsecureContent(Resource::Type type, const KURL& url,
return true; return true;
} }
bool ResourceFetcher::canRequest(Resource::Type type, const KURL& url, const ResourceLoaderOptions& options, bool forPreload, FetchRequest::OriginRestriction originRestriction) const bool ResourceFetcher::canRequest(Resource::Type type, const ResourceRequest& resourceRequest, const KURL& url, const ResourceLoaderOptions& options, bool forPreload, FetchRequest::OriginRestriction originRestriction) const
{ {
SecurityOrigin* securityOrigin = options.securityOrigin.get(); SecurityOrigin* securityOrigin = options.securityOrigin.get();
if (!securityOrigin && document()) if (!securityOrigin && document())
...@@ -623,8 +624,20 @@ bool ResourceFetcher::canRequest(Resource::Type type, const KURL& url, const Res ...@@ -623,8 +624,20 @@ bool ResourceFetcher::canRequest(Resource::Type type, const KURL& url, const Res
// folks block insecure content with a CSP policy, they don't get a warning. // folks block insecure content with a CSP policy, they don't get a warning.
// They'll still get a warning in the console about CSP blocking the load. // They'll still get a warning in the console about CSP blocking the load.
// If we're loading the main resource of a subframe, ensure that we treat the resource as active
// content for the purposes of mixed content checks, and that we check against the parent of the
// active frame, rather than the frame itself.
LocalFrame* effectiveFrame = frame();
MixedContentBlockingTreatment effectiveTreatment = options.mixedContentBlockingTreatment;
if (resourceRequest.frameType() == WebURLRequest::FrameTypeNested) {
effectiveTreatment = TreatAsActiveContent;
// FIXME: Deal with RemoteFrames.
if (frame()->tree().parent()->isLocalFrame())
effectiveFrame = toLocalFrame(frame()->tree().parent());
}
// FIXME: Should we consider forPreload here? // FIXME: Should we consider forPreload here?
if (!checkInsecureContent(type, url, options.mixedContentBlockingTreatment)) if (!checkInsecureContent(type, url, effectiveFrame, effectiveTreatment))
return false; return false;
return true; return true;
...@@ -633,7 +646,7 @@ bool ResourceFetcher::canRequest(Resource::Type type, const KURL& url, const Res ...@@ -633,7 +646,7 @@ bool ResourceFetcher::canRequest(Resource::Type type, const KURL& url, const Res
bool ResourceFetcher::canAccessResource(Resource* resource, SecurityOrigin* sourceOrigin, const KURL& url) const bool ResourceFetcher::canAccessResource(Resource* resource, SecurityOrigin* sourceOrigin, const KURL& url) const
{ {
// Redirects can change the response URL different from one of request. // Redirects can change the response URL different from one of request.
if (!canRequest(resource->type(), url, resource->options(), resource->isUnusedPreload(), FetchRequest::UseDefaultOriginRestrictionForType)) if (!canRequest(resource->type(), resource->resourceRequest(), url, resource->options(), resource->isUnusedPreload(), FetchRequest::UseDefaultOriginRestrictionForType))
return false; return false;
if (!sourceOrigin && document()) if (!sourceOrigin && document())
...@@ -713,7 +726,7 @@ ResourcePtr<Resource> ResourceFetcher::requestResource(Resource::Type type, Fetc ...@@ -713,7 +726,7 @@ ResourcePtr<Resource> ResourceFetcher::requestResource(Resource::Type type, Fetc
if (!url.isValid()) if (!url.isValid())
return 0; return 0;
if (!canRequest(type, url, request.options(), request.forPreload(), request.originRestriction())) if (!canRequest(type, request.resourceRequest(), url, request.options(), request.forPreload(), request.originRestriction()))
return 0; return 0;
if (LocalFrame* f = frame()) if (LocalFrame* f = frame())
...@@ -1336,7 +1349,7 @@ void ResourceFetcher::didReceiveResponse(const Resource* resource, const Resourc ...@@ -1336,7 +1349,7 @@ void ResourceFetcher::didReceiveResponse(const Resource* resource, const Resourc
{ {
// If the response is fetched via ServiceWorker, the original URL of the response could be different from the URL of the request. // If the response is fetched via ServiceWorker, the original URL of the response could be different from the URL of the request.
if (response.wasFetchedViaServiceWorker()) { if (response.wasFetchedViaServiceWorker()) {
if (!canRequest(resource->type(), response.url(), resource->options(), false, FetchRequest::UseDefaultOriginRestrictionForType)) { if (!canRequest(resource->type(), resource->resourceRequest(), response.url(), resource->options(), false, FetchRequest::UseDefaultOriginRestrictionForType)) {
resource->loader()->cancel(); resource->loader()->cancel();
context().dispatchDidFail(m_documentLoader, resource->identifier(), ResourceError(errorDomainBlinkInternal, 0, response.url().string(), "Unsafe attempt to load URL " + response.url().elidedString() + " fetched by a ServiceWorker.")); context().dispatchDidFail(m_documentLoader, resource->identifier(), ResourceError(errorDomainBlinkInternal, 0, response.url().string(), "Unsafe attempt to load URL " + response.url().elidedString() + " fetched by a ServiceWorker."));
return; return;
...@@ -1427,7 +1440,7 @@ bool ResourceFetcher::isLoadedBy(ResourceLoaderHost* possibleOwner) const ...@@ -1427,7 +1440,7 @@ bool ResourceFetcher::isLoadedBy(ResourceLoaderHost* possibleOwner) const
bool ResourceFetcher::canAccessRedirect(Resource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse, ResourceLoaderOptions& options) bool ResourceFetcher::canAccessRedirect(Resource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse, ResourceLoaderOptions& options)
{ {
if (!canRequest(resource->type(), request.url(), options, resource->isUnusedPreload(), FetchRequest::UseDefaultOriginRestrictionForType)) if (!canRequest(resource->type(), request, request.url(), options, resource->isUnusedPreload(), FetchRequest::UseDefaultOriginRestrictionForType))
return false; return false;
if (options.corsEnabled == IsCORSEnabled) { if (options.corsEnabled == IsCORSEnabled) {
SecurityOrigin* sourceOrigin = options.securityOrigin.get(); SecurityOrigin* sourceOrigin = options.securityOrigin.get();
......
...@@ -187,8 +187,8 @@ private: ...@@ -187,8 +187,8 @@ private:
ResourceRequestCachePolicy resourceRequestCachePolicy(const ResourceRequest&, Resource::Type); ResourceRequestCachePolicy resourceRequestCachePolicy(const ResourceRequest&, Resource::Type);
void addAdditionalRequestHeaders(ResourceRequest&, Resource::Type); void addAdditionalRequestHeaders(ResourceRequest&, Resource::Type);
bool canRequest(Resource::Type, const KURL&, const ResourceLoaderOptions&, bool forPreload, FetchRequest::OriginRestriction) const; bool canRequest(Resource::Type, const ResourceRequest&, const KURL&, const ResourceLoaderOptions&, bool forPreload, FetchRequest::OriginRestriction) const;
bool checkInsecureContent(Resource::Type, const KURL&, MixedContentBlockingTreatment) const; bool checkInsecureContent(Resource::Type, const KURL&, LocalFrame*, MixedContentBlockingTreatment) const;
static bool resourceNeedsLoad(Resource*, const FetchRequest&, RevalidationPolicy); static bool resourceNeedsLoad(Resource*, const FetchRequest&, RevalidationPolicy);
......
...@@ -376,14 +376,6 @@ void DocumentLoader::willSendRequest(ResourceRequest& newRequest, const Resource ...@@ -376,14 +376,6 @@ void DocumentLoader::willSendRequest(ResourceRequest& newRequest, const Resource
if (newRequest.cachePolicy() == UseProtocolCachePolicy && isRedirectAfterPost(newRequest, redirectResponse)) if (newRequest.cachePolicy() == UseProtocolCachePolicy && isRedirectAfterPost(newRequest, redirectResponse))
newRequest.setCachePolicy(ReloadBypassingCache); newRequest.setCachePolicy(ReloadBypassingCache);
// If this is a sub-frame, check for mixed content blocking against the parent frame.
if (Frame* parent = m_frame->tree().parent()) {
if (parent->isLocalFrame() && !toLocalFrame(parent)->loader().mixedContentChecker()->canFrameInsecureContent(toLocalFrame(parent)->document()->securityOrigin(), newRequest.url())) {
cancelMainResourceLoad(ResourceError::cancelledError(newRequest.url()));
return;
}
}
m_request = newRequest; m_request = newRequest;
if (redirectResponse.isNull()) if (redirectResponse.isNull())
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment