Verify the order of ResourceClient callbacks |
|||||||
Issue descriptionThe order of ResourceClient callbacks, especially RawResourceClient/DocumentThreadableLoader, is - important, because calling ResourceClient callbacks in a wrong order might cause security bugs, and - prone to error, especially when MemoryCache, revalidation, etc. are involved. This issue tracks efforts to verify the ResourceClient callbacks.
,
Aug 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/48136fcd4b053686ce2f1f98e64b00a064907ec4 commit 48136fcd4b053686ce2f1f98e64b00a064907ec4 Author: hiroshige <hiroshige@chromium.org> Date: Wed Aug 24 05:52:30 2016 Verify the order of RawResourceClient callbacks in DocumentThreadableLoader The errors in the order of RawResourceClient callbacks potentially leads to CORS bypassing, especially in DocumentThreadableLoader. This CL introduces RawResourceClientStateChecker to check the order of RawResourceClient callbacks, assuming that the RawResourceClient is added to at most one RawResource, and applies it to DocumentThreadableLoader. BUG= 633696 , 640291 Review-Url: https://codereview.chromium.org/2020313002 Cr-Commit-Position: refs/heads/master@{#413990} [modify] https://crrev.com/48136fcd4b053686ce2f1f98e64b00a064907ec4/third_party/WebKit/Source/core/fetch/RawResource.cpp [modify] https://crrev.com/48136fcd4b053686ce2f1f98e64b00a064907ec4/third_party/WebKit/Source/core/fetch/RawResource.h [modify] https://crrev.com/48136fcd4b053686ce2f1f98e64b00a064907ec4/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp [modify] https://crrev.com/48136fcd4b053686ce2f1f98e64b00a064907ec4/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h
,
Sep 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be7d5bee4161e753df168ead5fbbb44b4680f058 commit be7d5bee4161e753df168ead5fbbb44b4680f058 Author: hiroshige <hiroshige@chromium.org> Date: Thu Sep 01 05:10:57 2016 Supress CHECK() in RawResourceClientStateChecker::responseReceived() This CL temporarily turns the SECURITY_CHECK() in RawResourceClientStateChecker::responseReceived() into SECURITY_DCHECK(), so that the check doesn't crash release builds and is potentially catched by tests or fuzzers while we are investigating the issue. BUG=640960, 640291 Review-Url: https://codereview.chromium.org/2288193002 Cr-Commit-Position: refs/heads/master@{#415889} [modify] https://crrev.com/be7d5bee4161e753df168ead5fbbb44b4680f058/third_party/WebKit/Source/core/fetch/RawResource.cpp
,
Sep 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4f7bc92ee120a71f6fb9426be96a25b7e9a92f33 commit 4f7bc92ee120a71f6fb9426be96a25b7e9a92f33 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Mon Sep 05 06:17:24 2016 Supress CHECK() in RawResourceClientStateChecker::responseReceived() This CL temporarily turns the SECURITY_CHECK() in RawResourceClientStateChecker::responseReceived() into SECURITY_DCHECK(), so that the check doesn't crash release builds and is potentially catched by tests or fuzzers while we are investigating the issue. BUG=640960, 640291 Review-Url: https://codereview.chromium.org/2288193002 Cr-Commit-Position: refs/heads/master@{#415889} (cherry picked from commit be7d5bee4161e753df168ead5fbbb44b4680f058) Review URL: https://codereview.chromium.org/2309223002 . Cr-Commit-Position: refs/branch-heads/2840@{#151} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/4f7bc92ee120a71f6fb9426be96a25b7e9a92f33/third_party/WebKit/Source/core/fetch/RawResource.cpp
,
Sep 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f127916cd01e3e6fc2c00c3389bd7a96260cdd2 commit 0f127916cd01e3e6fc2c00c3389bd7a96260cdd2 Author: hiroshige <hiroshige@chromium.org> Date: Wed Sep 07 02:12:16 2016 Add NEVER_INLINE to RawResourceClientStateChecker to show it in stack traces BUG=640291 Review-Url: https://codereview.chromium.org/2317613002 Cr-Commit-Position: refs/heads/master@{#416821} [modify] https://crrev.com/0f127916cd01e3e6fc2c00c3389bd7a96260cdd2/third_party/WebKit/Source/core/fetch/RawResource.cpp
,
Sep 30 2016
,
Sep 30 2016
,
Oct 5 2016
,
Oct 5 2016
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4f7bc92ee120a71f6fb9426be96a25b7e9a92f33 commit 4f7bc92ee120a71f6fb9426be96a25b7e9a92f33 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Mon Sep 05 06:17:24 2016 Supress CHECK() in RawResourceClientStateChecker::responseReceived() This CL temporarily turns the SECURITY_CHECK() in RawResourceClientStateChecker::responseReceived() into SECURITY_DCHECK(), so that the check doesn't crash release builds and is potentially catched by tests or fuzzers while we are investigating the issue. BUG=640960, 640291 Review-Url: https://codereview.chromium.org/2288193002 Cr-Commit-Position: refs/heads/master@{#415889} (cherry picked from commit be7d5bee4161e753df168ead5fbbb44b4680f058) Review URL: https://codereview.chromium.org/2309223002 . Cr-Commit-Position: refs/branch-heads/2840@{#151} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/4f7bc92ee120a71f6fb9426be96a25b7e9a92f33/third_party/WebKit/Source/core/fetch/RawResource.cpp
,
Dec 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b18efa1312b262c6277c66fd01ef114e75d6ae0 commit 7b18efa1312b262c6277c66fd01ef114e75d6ae0 Author: hiroshige <hiroshige@chromium.org> Date: Tue Dec 27 22:08:51 2016 Introduce RevalidationStartForbiddenScope This CL makes revalidation-related CHECK()s in RawResource::didAddClient() cleaner and enables us to identify how revalidation is actually started as the crash stack trace of Resource::setRevalidatingRequest(). BUG=640291, 648945 Review-Url: https://codereview.chromium.org/2607623002 Cr-Commit-Position: refs/heads/master@{#440794} [modify] https://crrev.com/7b18efa1312b262c6277c66fd01ef114e75d6ae0/third_party/WebKit/Source/core/fetch/RawResource.cpp [modify] https://crrev.com/7b18efa1312b262c6277c66fd01ef114e75d6ae0/third_party/WebKit/Source/core/fetch/Resource.cpp [modify] https://crrev.com/7b18efa1312b262c6277c66fd01ef114e75d6ae0/third_party/WebKit/Source/core/fetch/Resource.h
,
Dec 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ae15c13530084329ac2509ccded45428d7d7d2c8 commit ae15c13530084329ac2509ccded45428d7d7d2c8 Author: Nate Chapin <japhet@chromium.org> Date: Sat Dec 02 02:03:07 2017 Make DocumentThreadableLoader a ResourceOwner It had a custom implementation to support use of RawResourceClientStateChecker. Separate these. Bug: 790778 , 640291 Change-Id: Id8a29a5ad916429969b47a0258675437de284b55 Reviewed-on: https://chromium-review.googlesource.com/802094 Commit-Queue: Nate Chapin <japhet@chromium.org> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Cr-Commit-Position: refs/heads/master@{#521177} [modify] https://crrev.com/ae15c13530084329ac2509ccded45428d7d7d2c8/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp [modify] https://crrev.com/ae15c13530084329ac2509ccded45428d7d7d2c8/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h [modify] https://crrev.com/ae15c13530084329ac2509ccded45428d7d7d2c8/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp [modify] https://crrev.com/ae15c13530084329ac2509ccded45428d7d7d2c8/third_party/WebKit/Source/platform/loader/fetch/RawResource.h
,
Dec 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6e8fd3eca1608700199771ecc24edd8f4255911a commit 6e8fd3eca1608700199771ecc24edd8f4255911a Author: Nate Chapin <japhet@chromium.org> Date: Mon Dec 04 21:02:02 2017 Revert "Make DocumentThreadableLoader a ResourceOwner" This reverts commit ae15c13530084329ac2509ccded45428d7d7d2c8. Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=791291 https://bugs.chromium.org/p/chromium/issues/detail?id=791316 https://bugs.chromium.org/p/chromium/issues/detail?id=791288 https://bugs.chromium.org/p/chromium/issues/detail?id=791348 https://bugs.chromium.org/p/chromium/issues/detail?id=791347 https://bugs.chromium.org/p/chromium/issues/detail?id=791450 https://bugs.chromium.org/p/chromium/issues/detail?id=791300 https://bugs.chromium.org/p/chromium/issues/detail?id=791522 Original change's description: > Make DocumentThreadableLoader a ResourceOwner > > It had a custom implementation to support use of RawResourceClientStateChecker. > Separate these. > > Bug: 790778 , 640291 > Change-Id: Id8a29a5ad916429969b47a0258675437de284b55 > Reviewed-on: https://chromium-review.googlesource.com/802094 > Commit-Queue: Nate Chapin <japhet@chromium.org> > Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> > Cr-Commit-Position: refs/heads/master@{#521177} TBR=hiroshige@chromium.org,japhet@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 790778 , 640291 Change-Id: I15938754b57b8142f8d7e9c94f8aff9bdd8329ed Reviewed-on: https://chromium-review.googlesource.com/806794 Reviewed-by: Nate Chapin <japhet@chromium.org> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Commit-Queue: Nate Chapin <japhet@chromium.org> Cr-Commit-Position: refs/heads/master@{#521448} [modify] https://crrev.com/6e8fd3eca1608700199771ecc24edd8f4255911a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp [modify] https://crrev.com/6e8fd3eca1608700199771ecc24edd8f4255911a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h [modify] https://crrev.com/6e8fd3eca1608700199771ecc24edd8f4255911a/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp [modify] https://crrev.com/6e8fd3eca1608700199771ecc24edd8f4255911a/third_party/WebKit/Source/platform/loader/fetch/RawResource.h
,
Dec 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f3b283cf948f10ad8c919c4c8ebec5219f058d28 commit f3b283cf948f10ad8c919c4c8ebec5219f058d28 Author: Nate Chapin <japhet@chromium.org> Date: Mon Dec 04 22:30:39 2017 Reland "Make DocumentThreadableLoader a ResourceOwner" It had a custom implementation to support use of RawResourceClientStateChecker. Separate these. The original version (https://chromium.googlesource.com/chromium/src/+/ae15c13530084329ac2509ccded45428d7d7d2c8) failed to update DocumentThreadableLoader::Trace(). It introduced an intermediate subclass, ResourceOwner<RawResource>, between DocumentThreadableLoader and RawResourceClient, but because Trace() wasn't updated, ResourceOwner::resource_ was left untraced. The GC clang plugin would normally catch a mistake like this, but the plugin ignores ResourceOwner due to https://crbug.com/652966, so it wasn't detected at compile time. Bug: 790778 , 640291 Change-Id: I114df7eaa97139fbb6aa70b22b2f41019988631f Reviewed-on: https://chromium-review.googlesource.com/806753 Commit-Queue: Nate Chapin <japhet@chromium.org> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Cr-Commit-Position: refs/heads/master@{#521490} [modify] https://crrev.com/f3b283cf948f10ad8c919c4c8ebec5219f058d28/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp [modify] https://crrev.com/f3b283cf948f10ad8c919c4c8ebec5219f058d28/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h [modify] https://crrev.com/f3b283cf948f10ad8c919c4c8ebec5219f058d28/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp [modify] https://crrev.com/f3b283cf948f10ad8c919c4c8ebec5219f058d28/third_party/WebKit/Source/platform/loader/fetch/RawResource.h
,
Aug 1
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by hirosh...@chromium.org
, Aug 23 2016