From 7ebb860b8142bf743c6c100de24b35cac940e606 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Tue, 4 Jun 2024 17:59:19 +0000 Subject: [PATCH] Bug 1898171 - Add SanitizedString profiler marker format r=acreskey,profiler-reviewers,aabh The sanitization function for URL and FilePath cannot currently sanitize an arbitrary string in the profiler data. The expectation is that the URL starts with a scheme like http:// and that a file path contains a /, so none of them are sanitized if the contents are a domain name. This commit introduces a new 'sanitized-string' format, that the profiler can make sure to completely blank out. Differential Revision: https://phabricator.services.mozilla.com/D211171 --- mozglue/baseprofiler/core/ProfilerMarkers.cpp | 2 ++ .../baseprofiler/public/BaseAndGeckoProfilerDetail.h | 2 +- .../public/BaseProfilerMarkersPrerequisites.h | 2 ++ tools/profiler/tests/gtest/GeckoProfiler.cpp | 11 ++++++++++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/mozglue/baseprofiler/core/ProfilerMarkers.cpp b/mozglue/baseprofiler/core/ProfilerMarkers.cpp index 2a6115c16691..c4c9cef51807 100644 --- a/mozglue/baseprofiler/core/ProfilerMarkers.cpp +++ b/mozglue/baseprofiler/core/ProfilerMarkers.cpp @@ -320,6 +320,8 @@ Span MarkerSchema::FormatToStringSpan( return mozilla::MakeStringSpan("url"); case Format::FilePath: return mozilla::MakeStringSpan("file-path"); + case Format::SanitizedString: + return mozilla::MakeStringSpan("sanitized-string"); case Format::String: return mozilla::MakeStringSpan("string"); case Format::UniqueString: diff --git a/mozglue/baseprofiler/public/BaseAndGeckoProfilerDetail.h b/mozglue/baseprofiler/public/BaseAndGeckoProfilerDetail.h index f3cb539f6439..fc360041b9b3 100644 --- a/mozglue/baseprofiler/public/BaseAndGeckoProfilerDetail.h +++ b/mozglue/baseprofiler/public/BaseAndGeckoProfilerDetail.h @@ -24,7 +24,7 @@ namespace mozilla { class ProfileBufferChunkManagerWithLocalLimit; // Centrally defines the version of the gecko profiler JSON format. -const int GECKO_PROFILER_FORMAT_VERSION = 29; +const int GECKO_PROFILER_FORMAT_VERSION = 30; namespace baseprofiler::detail { diff --git a/mozglue/baseprofiler/public/BaseProfilerMarkersPrerequisites.h b/mozglue/baseprofiler/public/BaseProfilerMarkersPrerequisites.h index c1de8955a7e5..3a57ebab3933 100644 --- a/mozglue/baseprofiler/public/BaseProfilerMarkersPrerequisites.h +++ b/mozglue/baseprofiler/public/BaseProfilerMarkersPrerequisites.h @@ -736,6 +736,8 @@ class MarkerSchema { Url, // Show the file path, and handle PII sanitization. FilePath, + // Show arbitrary string and handle PII sanitization + SanitizedString, // Important, do not put URL or file path information here, as it will not // be sanitized. Please be careful with including other types of PII here as // well. diff --git a/tools/profiler/tests/gtest/GeckoProfiler.cpp b/tools/profiler/tests/gtest/GeckoProfiler.cpp index 2e9790c8c99a..4e07f03339be 100644 --- a/tools/profiler/tests/gtest/GeckoProfiler.cpp +++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp @@ -2430,6 +2430,9 @@ TEST(GeckoProfiler, Markers) schema.AddKeyFormat("key with decimal", MS::Format::Decimal); schema.AddStaticLabelValue("static label", "static value"); schema.AddKeyFormat("key with unique string", MS::Format::UniqueString); + schema.AddKeyFormatSearchable("key with sanitized string", + MS::Format::SanitizedString, + MS::Searchable::Searchable); return schema; } }; @@ -3358,7 +3361,7 @@ TEST(GeckoProfiler, Markers) EXPECT_EQ_JSON(schema["tooltipLabel"], String, "tooltip label"); EXPECT_EQ_JSON(schema["tableLabel"], String, "table label"); - ASSERT_EQ(data.size(), 15u); + ASSERT_EQ(data.size(), 16u); ASSERT_TRUE(data[0u].isObject()); EXPECT_EQ_JSON(data[0u]["key"], String, "key with url"); @@ -3450,6 +3453,12 @@ TEST(GeckoProfiler, Markers) EXPECT_EQ_JSON(data[14u]["format"], String, "unique-string"); EXPECT_TRUE(data[14u]["searchable"].isNull()); + ASSERT_TRUE(data[15u].isObject()); + EXPECT_EQ_JSON(data[15u]["key"], String, + "key with sanitized string"); + EXPECT_TRUE(data[15u]["label"].isNull()); + EXPECT_EQ_JSON(data[15u]["format"], String, "sanitized-string"); + EXPECT_EQ_JSON(data[15u]["searchable"], Bool, true); } else if (nameString == "markers-gtest-special") { EXPECT_EQ(display.size(), 0u); ASSERT_EQ(data.size(), 0u);