RESOLVED DUPLICATE of bug 171394 171231
Resource Timing doesn't require Timing-Allow-Origin header for cross origin requests
https://bugs.webkit.org/show_bug.cgi?id=171231
Summary Resource Timing doesn't require Timing-Allow-Origin header for cross origin r...
John Schulz
Reported 2017-04-24 10:26:18 PDT
According to https://w3c.github.io/resource-timing/#cross-origin-resources the following properties should be set to 0 for cross-origin requests: redirectStart, redirectEnd, domainLookupStart, domainLookupEnd, connectStart, connectEnd, requestStart, responseStart, secureConnectionStart, transferSize, encodedBodySize, decodedBodySize However, as shown in https://twitter.com/JFSIII/status/855505436971155456 & https://twitter.com/JFSIII/status/855531120800129024, this is not always the case.
Attachments
[IMAGE] Web Inspector - bbc.com Resource Timing Entries (1017.19 KB, image/png)
2017-04-27 14:55 PDT, Joseph Pecoraro
no flags
List of resources (302.02 KB, image/png)
2017-04-27 15:41 PDT, John Schulz
no flags
CSS file 1 - no header - non-zero timing (159.34 KB, image/png)
2017-04-27 15:43 PDT, John Schulz
no flags
CSS file 2 - no header - non-zero timing (158.07 KB, image/png)
2017-04-27 15:43 PDT, John Schulz
no flags
JS file 1 - no header - zeroed timing (169.86 KB, image/png)
2017-04-27 15:44 PDT, John Schulz
no flags
JS file 2 - no header - zeroed timing (165.07 KB, image/png)
2017-04-27 15:44 PDT, John Schulz
no flags
Radar WebKit Bug Importer
Comment 1 2017-04-24 16:43:35 PDT
Joseph Pecoraro
Comment 2 2017-04-27 14:32:51 PDT
I handled cache responses in bug 171394. I'll use this one to investigate why non-cached responses for cross-origin resources were somehow allowing these properties.
Joseph Pecoraro
Comment 3 2017-04-27 14:55:08 PDT
Created attachment 308453 [details] [IMAGE] Web Inspector - bbc.com Resource Timing Entries Hmm, I'm not able to reproduce this with trunk (and with my changes on bug 171394). See attached screenshot of: > console.table(performance.getEntriesByType("resource"), ["name", "fetchStart", "connectStart", "domainLookupStart"]) Performed on bbc.com. This was load without using the cache (Web Inspector shows all resources did not use the memory cache). In this case only a few resources had connect/dns information and each of those domains explicitly had Timing-Allow-Origin: "*". Each of the others are zero'd indicating they are cross-origin without TAO.
Joseph Pecoraro
Comment 4 2017-04-27 14:59:33 PDT
Ahh. So here is my theory. The same resource often shows up a bunch of times, so maybe when you ran: js> Array.from(performance.getEntriesByType("resource")).filter(r => r.connectEnd).map(r => r.name); You were seeing the cache responses in the resulting list (which will at the time had their connectStart/domainLookupStart falling back to fetchStart like a normal request). Those cache responses are being corrected, so now these false positives don't show up. Let me try to verify that.
Joseph Pecoraro
Comment 5 2017-04-27 15:08:57 PDT
Hmm, that doesn't seem to fully explain things. On STP 28 I was able to reproduce the issue with a single cross-origin resource that claimed to not be from a cache yet had non-zero'd data. However, I'm unable to reproduce that on my trunk builds, so it seems this will be fixed by the change on 171394.
John Schulz
Comment 6 2017-04-27 15:41:17 PDT
Created attachment 308462 [details] List of resources
John Schulz
Comment 7 2017-04-27 15:41:40 PDT
I ran the same test on cnn.com which I've never visited in STP 28. Many uncached resources had connectStart values and did not have a TAO header. I'm attaching some screenshots showing what I saw.
John Schulz
Comment 8 2017-04-27 15:43:40 PDT
Created attachment 308463 [details] CSS file 1 - no header - non-zero timing
John Schulz
Comment 9 2017-04-27 15:43:54 PDT
Created attachment 308464 [details] CSS file 2 - no header - non-zero timing
John Schulz
Comment 10 2017-04-27 15:44:16 PDT
Created attachment 308465 [details] JS file 1 - no header - zeroed timing
John Schulz
Comment 11 2017-04-27 15:44:31 PDT
Created attachment 308466 [details] JS file 2 - no header - zeroed timing
Joseph Pecoraro
Comment 12 2017-04-27 16:53:17 PDT
Yep, as I'm discovering in bug 171394 that code path might have been possible for ongoing requests. So the more I look into it, I think that should fix this issue.
Joseph Pecoraro
Comment 13 2017-05-01 15:11:30 PDT
Yes, I'm going to mark this as a duplicate of 171394. Sorry for unnecessarily creating an extra bug, I had just originally thought there were two distinct issues! I'm unable to reproduce after that change: <http://trac.webkit.org/changeset/215981> That change fixed two issues. (1) Loads from the cache for an earlier completed request. - The typical cache case. This will appear in Web Inspector as cached (from Memory or Disk cache) (2) Loads that would use the cache result for an "ongoing request". - This can happen internally within WebCore making multiple requests for a resource - This could eventually appear in Web Inspector as a Network request when the load actually completes These examples are of case (2). In cases where there was an ongoing request we were incorrectly creating a PerformanceResourceTiming entry before the request completed (and therefore before we knew if there was a TAO or not). So, the information for these requests, with and without TAO should be more accurate. -- Thanks for reporting this issue! We were reporting inaccurate / misleading in these cases. You should be able to verify in a WebKit Nightly, or a Safari Technology Preview in a few weeks. *** This bug has been marked as a duplicate of bug 171394 ***
John Schulz
Comment 14 2017-05-01 20:32:36 PDT
Checked out in WebKit Nightly and things look good to me.
Note You need to log in before you can comment on or make changes to this bug.