Summary: | [Resource Timing] Implement cross-origin restrictions | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Simonsen <simonjam> | ||||||||
Component: | Platform | Assignee: | James Simonsen <simonjam> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | levin+threading, pan.deng, sullivan, tonyg, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
URL: | http://dvcs.w3.org/hg/webperf/raw-file/tip/specs/ResourceTiming/Overview.html | ||||||||||
Bug Depends on: | 84883 | ||||||||||
Bug Blocks: | 61138 | ||||||||||
Attachments: |
|
Description
James Simonsen
2012-04-25 12:41:09 PDT
Created attachment 161925 [details]
Patch
Created attachment 170155 [details]
Patch
Comment on attachment 170155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170155&action=review > Source/WebCore/page/Performance.cpp:189 > +static bool passesTimingAllowCheck(const ResourceResponse& response, SecurityOrigin* securityOrigin) I think this should happen in the PerformanceResourceTiming constructor. It's really only applicable to that class. > Source/WebCore/page/Performance.cpp:201 > + if (timingAllowOriginString != securityOrigin->toString()) The string may be a space separated list. We should check each item in it. > Source/WebCore/page/Performance.cpp:207 > +void Performance::addResourceTiming(const ResourceRequest& request, const ResourceResponse& response, double finishTime, Document* requestingDocument, SecurityOrigin* initiatorOrigin) We don't need to pass in the origin. You can get it from requestingDocument->securityOrigin(). > Source/WebCore/page/Performance.cpp:213 > + bool allowTiming = WebCore::SecurityOrigin::create(response.url())->equal(initiatorOrigin) || passesTimingAllowCheck(response, initiatorOrigin); allowTiming -> shouldReportDetails > Source/WebCore/page/PerformanceResourceTiming.h:52 > + static PassRefPtr<PerformanceResourceTiming> create(const ResourceRequest& request, const ResourceResponse& response, double finishTime, Document* requestingDocument, bool isAllowTiming) isAllowTiming -> shouldReportDetails > Source/WebCore/page/PerformanceResourceTiming.h:74 > + PerformanceResourceTiming(const ResourceRequest&, const ResourceResponse&, double finishTime, Document*, bool); This needs a name. You can only omit the name if it's clear what it is. > LayoutTests/http/tests/w3c/webperf/submission/Intel/resource-timing/test_resource_timing_timing_allow_cross_origin_resource_request.html:51 > + requestUrl += '?origin=http://' + pageOrigin; We should have a test where it's allowed by this parameter. (In reply to comment #3) > > LayoutTests/http/tests/w3c/webperf/submission/Intel/resource-timing/test_resource_timing_timing_allow_cross_origin_resource_request.html:51 > > + requestUrl += '?origin=http://' + pageOrigin; > > We should have a test where it's allowed by this parameter. Sorry, I misread the code. I saw the 0's but didn't notice the greater_than. I think we're good here. Created attachment 177042 [details]
Patch
In the interest of getting this landed soon, I made the suggestions. Someone else (Tony?) will have to review it still. Comment on attachment 177042 [details] Patch Clearing flags on attachment: 177042 Committed r136329: <http://trac.webkit.org/changeset/136329> All reviewed patches have been landed. Closing bug. |