New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 892148 link

Starred by 2 users
Status: WontFix
Owner:
█ dcreager@chromium.org
Last visit > 30 days ago
Closed: Oct 11
Cc:
█ lassey@chromium.org
rsesek@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Use data_decoder's JSON parser to parse Reporting and NEL policy headers

Project Member Reported by dcreager@chromium.org, Oct 4 
Reporting and Network Error Logging use JSON-encoded response headers to instruct Chrome when to collect reports about particular domains, and where to send those reports.  We're currently parsing those headers directly in the browser process, but want to switch to using data_decoder's SafeJsonParser instead.

Comment 1 by dcreager@chromium.org, Oct 4

Cc: rsesek@chromium.org
The ParseBatch method that rsesek added will be key, since that will mean we won't have to fire up a separate utility process for each network request!
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 8 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8f4b6863de9997033e4ba61266a97e83d6e2f25

commit c8f4b6863de9997033e4ba61266a97e83d6e2f25
Author: Douglas Creager <dcreager@chromium.org>
Date: Mon Oct 08 12:46:19 2018

Reporting: Add delegate for parsing JSON

This will allow us to add a new implementation that uses data_decoder to
safely parse the untrusted JSON headers in a utility process.  This
patch just adds the new delegate interface and a default implementation
that provides the existing behavior of parsing in-process.

Bug:  892148 ,  811485 
Change-Id: Ida8027ce44163c7f35535a3327343b7bf94ea854
Reviewed-on: https://chromium-review.googlesource.com/c/1262595
Commit-Queue: Douglas Creager <dcreager@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597540}
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/BUILD.gn
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/network_error_logging/network_error_logging_end_to_end_test.cc
[add] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/json_parser_delegate.cc
[add] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/json_parser_delegate.h
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/reporting_context.cc
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/reporting_context.h
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/reporting_delegate.cc
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/reporting_delegate.h
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/reporting_service.cc
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/reporting_service.h
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/reporting_test_util.cc
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/reporting/reporting_test_util.h
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/c8f4b6863de9997033e4ba61266a97e83d6e2f25/net/url_request/url_request_context_builder.h

Comment 3 by dcreager@chromium.org, Oct 8

Cc: lassey@chromium.org

Comment 4 by dcreager@chromium.org, Oct 11

Status: WontFix (was: Assigned)
Per https://chromium-review.googlesource.com/c/chromium/src/+/1268635#message-563e45b5b5835961ca7e5c7466e9fc77ec93f649 we don't need to use data_decoder for this.  The parsing will happen in the networking code, which is about to be in its own separate process anyway.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b81901f5f5e38784e849fffafe4036cbbbfe50c

commit 2b81901f5f5e38784e849fffafe4036cbbbfe50c
Author: Douglas Creager <dcreager@chromium.org>
Date: Thu Oct 11 21:29:36 2018

Reporting: Remove JSONParserDelegate

After further discussion, we won't ever need to worry about parsing JSON
headers in a separate process — the network stack itself will be moving
into a separate process in the near future, and that's good enough to
get the parsing logic out of the browser process.  This patch therefore
removes the delegate interface, since there will never be custom
implementations of it.

Bug:  892148 
Change-Id: I8c84c9811505ae751f3f4c9a383e69fedbe8606d
Reviewed-on: https://chromium-review.googlesource.com/c/1277545
Reviewed-by: Misha Efimov <mef@chromium.org>
Commit-Queue: Douglas Creager <dcreager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598952}
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/BUILD.gn
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/network_error_logging/network_error_logging_end_to_end_test.cc
[delete] https://crrev.com/e60b20c598a465fbb3fcb857dd85dcd212a01974/net/reporting/json_parser_delegate.cc
[delete] https://crrev.com/e60b20c598a465fbb3fcb857dd85dcd212a01974/net/reporting/json_parser_delegate.h
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/reporting/reporting_context.cc
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/reporting/reporting_context.h
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/reporting/reporting_service.cc
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/reporting/reporting_service.h
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/reporting/reporting_test_util.cc
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/2b81901f5f5e38784e849fffafe4036cbbbfe50c/net/url_request/url_request_context_builder.h
►

Sign in to add a comment