Skip to content

Commit

Permalink
Clear reports in WPT collector when queried.
Browse files Browse the repository at this point in the history
This fixes a bug in the WPT report collector where reports are not
removed from the stash when queried, as they were intended to be.

There was one test in the Network Error Logging suite which relied on
this bug, and tested reports in two passes, which could now fail if all
reports are received before the first query is performed, so this CL
also fixes that test by adding an optional 'retain' query parameter to
the report collector.

Bug: 1177757
Change-Id: I945325f15a2ce633cfa32bcebeedb5be5afaa860
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2689369
Reviewed-by: Stephen McGruer <[email protected]>
Commit-Queue: Ian Clelland <[email protected]>
Cr-Commit-Position: refs/heads/master@{#853528}
  • Loading branch information
clelland authored and chromium-wpt-export-bot committed Feb 12, 2021
1 parent f3a7512 commit be055c4
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 13 deletions.
2 changes: 1 addition & 1 deletion network-error-logging/sends-report-on-redirect.https.html
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
metadata: {
content_type: "application/reports+json",
},
}), "receive report about redirected resource");
}, true /* retain */), "receive report about redirected resource");
assert_true(await reportExists({
// This happens to be where we're redirected to.
url: getURLForResourceWithNoPolicy(),
Expand Down
8 changes: 6 additions & 2 deletions network-error-logging/support/nel.sub.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,14 @@ function _isSubsetOf(obj1, obj2) {
* expected.
*/

async function reportExists(expected) {
async function reportExists(expected, retain_reports) {
var timeout =
document.querySelector("meta[name=timeout][content=long]") ? 50 : 1;
var reportLocation =
"/reporting/resources/report.py?op=retrieve_report&timeout=" +
timeout + "&reportID=" + reportID;
if (retain_reports)
reportLocation += "&retain=1";
const response = await fetch(reportLocation);
const json = await response.json();
for (const report of json) {
Expand All @@ -268,11 +270,13 @@ async function reportExists(expected) {
* expected.
*/

async function reportsExist(expected_reports) {
async function reportsExist(expected_reports, retain_reports) {
const timeout = 10;
let reportLocation =
"/reporting/resources/report.py?op=retrieve_report&timeout=" +
timeout + "&reportID=" + reportID;
if (retain_reports)
reportLocation += "&retain";
// There must be the report of pass.png, so adding 1.
const min_count = expected_reports.length + 1;
reportLocation += "&min_count=" + min_count;
Expand Down
2 changes: 2 additions & 0 deletions reporting/resources/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ Supported GET parameters:
`min_count`: The minimum number of reports to return with the `retrieve_report`
operation. If there have been fewer than this many reports received, then an
empty report list will be returned instead.
`retain`: If present, reports will remain in the stash after being retrieved.
By default, reports are cleared once retrieved.

Operations:
`retrieve_report`: Returns all reports received so far for this reportID, as a
Expand Down
24 changes: 14 additions & 10 deletions reporting/resources/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from wptserve.utils import isomorphic_decode

def retrieve_from_stash(request, key, timeout, default_value, min_count=None):
def retrieve_from_stash(request, key, timeout, default_value, min_count=None, retain=False):
"""Retrieve the set of reports for a given report ID.
This will extract either the set of reports, credentials, or request count
Expand All @@ -22,14 +22,17 @@ def retrieve_from_stash(request, key, timeout, default_value, min_count=None):
time.sleep(0.5)
with request.server.stash.lock:
value = request.server.stash.take(key=key)
if value is not None and (min_count is None or len(value) >= min_count):
request.server.stash.put(key=key, value=value)
# If the last report received looks like a CSP report-uri report, then
# extract it from the list and return it alone. (This is until the CSP
# tests are modified to expect a list of reports returned in all cases.)
if isinstance(value,list) and 'csp-report' in value[-1]:
value = value[-1]
return json.dumps(value)
if value is not None:
have_sufficient_reports = (min_count is None or len(value) >= min_count)
if retain or not have_sufficient_reports:
request.server.stash.put(key=key, value=value)
if have_sufficient_reports:
# If the last report received looks like a CSP report-uri report, then
# extract it from the list and return it alone. (This is until the CSP
# tests are modified to expect a list of reports returned in all cases.)
if isinstance(value,list) and 'csp-report' in value[-1]:
value = value[-1]
return json.dumps(value)

return default_value

Expand Down Expand Up @@ -67,10 +70,11 @@ def main(request, response):
min_count = int(request.GET.first(b"min_count"))
except:
min_count = 1
retain = (b"retain" in request.GET)

op = request.GET.first(b"op", b"")
if op in (b"retrieve_report", b""):
return [(b"Content-Type", b"application/json")], retrieve_from_stash(request, key, timeout, u'[]', min_count)
return [(b"Content-Type", b"application/json")], retrieve_from_stash(request, key, timeout, u'[]', min_count, retain)

if op == b"retrieve_cookies":
return [(b"Content-Type", b"application/json")], u"{ \"reportCookies\" : " + str(retrieve_from_stash(request, cookie_key, timeout, u"\"None\"")) + u"}"
Expand Down

0 comments on commit be055c4

Please sign in to comment.