Commit 949141f8 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Exclude the nonce from ScopedOriginCrashKey value.

Before the current CL, the nonce in the crash key is unique in each
crash report, making it more difficult to aggregate the data.

Additionally, after r783795 landed, we started seeing new values of
request_initiator_site_lock crash key associated with
https://crbug.com/1056949, but unfortunately the interesting part of the
crash key was truncated, making it impossible to distinguish between
mhtml.subframe.invalid VS browser.initiated.invalid VS
error.page.invalid.  For example, before the current CL, the crash key
values might look like the following (truncated to 64 characters):

    null [internally: (646094991C8C225E35D3FEE319396B15) derived fro

After the current CL, the crash key value omits the nonce:

    null [internally: derived from https://error.page.invalid]

Bug: 1056949
Change-Id: Ib676e4484f1814a494b5359db13cfc9160233b63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2284044
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786422}
parent fe5c7a11
......@@ -244,7 +244,7 @@ Origin Origin::DeriveNewOpaqueOrigin() const {
return Origin(Nonce(), tuple_);
}
std::string Origin::GetDebugString() const {
std::string Origin::GetDebugString(bool include_nonce) const {
// Handle non-opaque origins first, as they are simpler.
if (!opaque()) {
std::string out = Serialize();
......@@ -256,11 +256,15 @@ std::string Origin::GetDebugString() const {
// For opaque origins, log the nonce and precursor as well. Without this,
// EXPECT_EQ failures between opaque origins are nearly impossible to
// understand.
std::string nonce = nonce_->raw_token().is_empty()
? std::string("nonce TBD")
: nonce_->raw_token().ToString();
std::string out = base::StrCat({Serialize(), " [internally: (", nonce, ")"});
std::string out = base::StrCat({Serialize(), " [internally:"});
if (include_nonce) {
out += " (";
if (nonce_->raw_token().is_empty())
out += "nonce TBD";
else
out += nonce_->raw_token().ToString();
out += ")";
}
if (!tuple_.IsValid())
base::StrAppend(&out, {" anonymous]"});
else
......@@ -438,7 +442,8 @@ ScopedOriginCrashKey::ScopedOriginCrashKey(
const url::Origin* value)
: base::debug::ScopedCrashKeyString(
crash_key,
value ? value->GetDebugString() : "nullptr") {}
value ? value->GetDebugString(false /* include_nonce */)
: "nullptr") {}
ScopedOriginCrashKey::~ScopedOriginCrashKey() = default;
......
......@@ -288,7 +288,7 @@ class COMPONENT_EXPORT(URL) Origin {
// Creates a string representation of the object that can be used for logging
// and debugging. It serializes the internal state, such as the nonce value
// and precursor information.
std::string GetDebugString() const;
std::string GetDebugString(bool include_nonce = true) const;
#if defined(OS_ANDROID)
base::android::ScopedJavaLocalRef<jobject> CreateJavaObject() const;
......
......@@ -846,6 +846,10 @@ TEST_F(OriginTest, GetDebugString) {
http_opaque_origin.GetDebugString().c_str(),
::testing::MatchesRegex(
"null \\[internally: \\(\\w*\\) derived from http://192.168.9.1\\]"));
EXPECT_THAT(
http_opaque_origin.GetDebugString(false /* include_nonce */).c_str(),
::testing::MatchesRegex(
"null \\[internally: derived from http://192.168.9.1\\]"));
Origin data_origin = Origin::Create(GURL("data:"));
EXPECT_STREQ(data_origin.GetDebugString().c_str(),
......@@ -857,6 +861,9 @@ TEST_F(OriginTest, GetDebugString) {
EXPECT_THAT(
data_derived_origin.GetDebugString().c_str(),
::testing::MatchesRegex("null \\[internally: \\(\\w*\\) anonymous\\]"));
EXPECT_THAT(
data_derived_origin.GetDebugString(false /* include_nonce */).c_str(),
::testing::MatchesRegex("null \\[internally: anonymous\\]"));
Origin file_origin = Origin::Create(GURL("file:///etc/passwd"));
EXPECT_STREQ(file_origin.GetDebugString().c_str(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment