From 89e952bbedfc0e624b564a935b8aeb851aad19db Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 11:36:54 +0200 Subject: [PATCH 01/10] Add sanitized-string format --- src/profile-logic/marker-data.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/profile-logic/marker-data.js b/src/profile-logic/marker-data.js index af8d4c20f8..339bca8124 100644 --- a/src/profile-logic/marker-data.js +++ b/src/profile-logic/marker-data.js @@ -1464,6 +1464,11 @@ export function sanitizeFromMarkerSchema( ...markerPayload, [key]: removeFilePath(markerPayload[key]), }: any); + } else if (format === 'sanitized-string') { + markerPayload = ({ + ...markerPayload, + [key]: "", + }: any); } } } From f2ca5cf17bdb8ec2757a4ede02170b9c48cf5d8b Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 11:46:58 +0200 Subject: [PATCH 02/10] fix lint --- src/profile-logic/marker-data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/profile-logic/marker-data.js b/src/profile-logic/marker-data.js index 339bca8124..0962fddcf3 100644 --- a/src/profile-logic/marker-data.js +++ b/src/profile-logic/marker-data.js @@ -1467,7 +1467,7 @@ export function sanitizeFromMarkerSchema( } else if (format === 'sanitized-string') { markerPayload = ({ ...markerPayload, - [key]: "", + [key]: '', }: any); } } From 0172c7f6a6dc6ae8f844280332ca123905fb6ea3 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 15:53:12 +0200 Subject: [PATCH 03/10] Bump profile version to 30 --- docs-developer/CHANGELOG-formats.md | 4 ++++ src/app-logic/constants.js | 2 +- src/profile-logic/gecko-profile-versioning.js | 7 +++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs-developer/CHANGELOG-formats.md b/docs-developer/CHANGELOG-formats.md index d7e1e8c1e1..fb35a2bf14 100644 --- a/docs-developer/CHANGELOG-formats.md +++ b/docs-developer/CHANGELOG-formats.md @@ -70,6 +70,10 @@ Older versions are not documented in this changelog but can be found in [process ## Gecko profile format +### Version 30 + +A new `sanitized-string` marker schema format type has been added, allowing markers to carry arbitrary strings containing PII that will be sanitized along with URLs and FilePaths. + ### Version 29 Removed the 'sample_groups' object from the GeckoCounter structure. diff --git a/src/app-logic/constants.js b/src/app-logic/constants.js index 4c7234cf57..99f275d7bf 100644 --- a/src/app-logic/constants.js +++ b/src/app-logic/constants.js @@ -9,7 +9,7 @@ import type { MarkerPhase } from 'firefox-profiler/types'; // The current version of the Gecko profile format. // Please don't forget to update the gecko profile format changelog in // `docs-developer/CHANGELOG-formats.md`. -export const GECKO_PROFILE_VERSION = 29; +export const GECKO_PROFILE_VERSION = 30; // The current version of the "processed" profile format. // Please don't forget to update the processed profile format changelog in diff --git a/src/profile-logic/gecko-profile-versioning.js b/src/profile-logic/gecko-profile-versioning.js index 5b34e5f53d..ea5b8bc7ee 100644 --- a/src/profile-logic/gecko-profile-versioning.js +++ b/src/profile-logic/gecko-profile-versioning.js @@ -1473,6 +1473,13 @@ const _upgraders = { } convertToVersion29Recursive(profile); }, + [30]: (_) => { + // This version bump added a new marker schema format type, named "sanitized-string", + // which older frontends will not be able to display. + // No upgrade is needed, as older versions of firefox would not generate + // marker data with unique-string typed data, and no modification is needed in the + // frontend to display older formats. + }, // If you add a new upgrader here, please document the change in // `docs-developer/CHANGELOG-formats.md`. From a166cd7626086f59e576f5bafa6be3ce1177a8c6 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 15:57:34 +0200 Subject: [PATCH 04/10] Bump processed profile version to 49 --- docs-developer/CHANGELOG-formats.md | 4 ++++ src/app-logic/constants.js | 2 +- src/profile-logic/gecko-profile-versioning.js | 2 +- src/profile-logic/processed-profile-versioning.js | 3 +++ 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/docs-developer/CHANGELOG-formats.md b/docs-developer/CHANGELOG-formats.md index fb35a2bf14..c2e11feaef 100644 --- a/docs-developer/CHANGELOG-formats.md +++ b/docs-developer/CHANGELOG-formats.md @@ -6,6 +6,10 @@ Note that this is not an exhaustive list. Processed profile format upgraders can ## Processed profile format +### Version 49 + +A new `sanitized-string` marker schema format type has been added, allowing markers to carry arbitrary strings containing PII that will be sanitized along with URLs and FilePaths. + ### Version 48 Removed the 'sampleGroups' object from the Counter structure. diff --git a/src/app-logic/constants.js b/src/app-logic/constants.js index 99f275d7bf..ec055f0300 100644 --- a/src/app-logic/constants.js +++ b/src/app-logic/constants.js @@ -14,7 +14,7 @@ export const GECKO_PROFILE_VERSION = 30; // The current version of the "processed" profile format. // Please don't forget to update the processed profile format changelog in // `docs-developer/CHANGELOG-formats.md`. -export const PROCESSED_PROFILE_VERSION = 48; +export const PROCESSED_PROFILE_VERSION = 49; // The following are the margin sizes for the left and right of the timeline. Independent // components need to share these values. diff --git a/src/profile-logic/gecko-profile-versioning.js b/src/profile-logic/gecko-profile-versioning.js index ea5b8bc7ee..a0df20f626 100644 --- a/src/profile-logic/gecko-profile-versioning.js +++ b/src/profile-logic/gecko-profile-versioning.js @@ -1477,7 +1477,7 @@ const _upgraders = { // This version bump added a new marker schema format type, named "sanitized-string", // which older frontends will not be able to display. // No upgrade is needed, as older versions of firefox would not generate - // marker data with unique-string typed data, and no modification is needed in the + // marker data with sanitized-string typed data, and no modification is needed in the // frontend to display older formats. }, diff --git a/src/profile-logic/processed-profile-versioning.js b/src/profile-logic/processed-profile-versioning.js index 004418d8da..c46872d1cd 100644 --- a/src/profile-logic/processed-profile-versioning.js +++ b/src/profile-logic/processed-profile-versioning.js @@ -2264,6 +2264,9 @@ const _upgraders = { } } }, + [49]: (_) => { + // The 'sanitized-string' marker schema format type has been added. + }, // If you add a new upgrader here, please document the change in // `docs-developer/CHANGELOG-formats.md`. }; From 4aff239392dc7e1bdbffef29566d4693f91c479b Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 16:06:17 +0200 Subject: [PATCH 05/10] Fix tests --- .../__snapshots__/profile-view.test.js.snap | 4 +- .../profile-conversion.test.js.snap | 48 +++++++++---------- .../profile-upgrading.test.js.snap | 14 +++--- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index e72d94552a..4f1c9f1f3f 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -423,7 +423,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "sourceURL": "", @@ -431,7 +431,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 29, + "version": 30, }, "pages": Array [ Object { diff --git a/src/test/unit/__snapshots__/profile-conversion.test.js.snap b/src/test/unit/__snapshots__/profile-conversion.test.js.snap index faad3cf38e..ed54a5b40b 100644 --- a/src/test/unit/__snapshots__/profile-conversion.test.js.snap +++ b/src/test/unit/__snapshots__/profile-conversion.test.js.snap @@ -469,7 +469,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "ART Trace (Android)", "sampleUnits": undefined, @@ -479,7 +479,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 29, + "version": 30, "visualMetrics": undefined, }, "pages": Array [], @@ -100220,7 +100220,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "ART Trace (Android)", "sampleUnits": undefined, @@ -100230,7 +100230,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 29, + "version": 30, "visualMetrics": undefined, }, "pages": Array [], @@ -382162,7 +382162,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Chrome Trace", "sourceURL": "", @@ -382170,7 +382170,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 29, + "version": 30, }, "pages": Array [], "threads": Array [ @@ -430866,7 +430866,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Chrome Trace", "sourceURL": "", @@ -430874,7 +430874,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 29, + "version": 30, }, "pages": Array [], "threads": Array [ @@ -479570,7 +479570,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 66155012.423, @@ -479580,7 +479580,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 29, + "version": 30, }, "pages": Array [], "threads": Array [ @@ -482612,7 +482612,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Chrome Trace", "sourceURL": "", @@ -482620,7 +482620,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 29, + "version": 30, }, "pages": Array [], "threads": Array [ @@ -485730,7 +485730,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 66155012.423, @@ -485740,7 +485740,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 29, + "version": 30, }, "pages": Array [], "threads": Array [ @@ -486930,7 +486930,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -486940,7 +486940,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 29, + "version": 30, "visualMetrics": undefined, }, "pages": Array [], @@ -494455,7 +494455,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -494465,7 +494465,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 29, + "version": 30, "visualMetrics": undefined, }, "pages": Array [], @@ -513868,7 +513868,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -513878,7 +513878,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 29, + "version": 30, "visualMetrics": undefined, }, "pages": Array [], @@ -521386,7 +521386,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -521396,7 +521396,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 29, + "version": 30, "visualMetrics": undefined, }, "pages": Array [], @@ -544986,7 +544986,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "target/debug/examples/work_log (dhat)", "sourceURL": "", @@ -544994,7 +544994,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 29, + "version": 30, }, "pages": Array [], "threads": Array [ diff --git a/src/test/unit/__snapshots__/profile-upgrading.test.js.snap b/src/test/unit/__snapshots__/profile-upgrading.test.js.snap index 7d65ccb730..e2e66e3805 100644 --- a/src/test/unit/__snapshots__/profile-upgrading.test.js.snap +++ b/src/test/unit/__snapshots__/profile-upgrading.test.js.snap @@ -40,7 +40,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -50,7 +50,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 29, + "version": 30, "visualMetrics": undefined, }, "pages": Array [], @@ -7975,7 +7975,7 @@ Object { "stackwalk": 1, "startTime": 1460221352723.438, "toolkit": "cocoa", - "version": 29, + "version": 30, }, "pausedRanges": Array [], "processes": Array [ @@ -9431,7 +9431,7 @@ Object { "stackwalk": 1, "startTime": 1460221352723.438, "toolkit": "cocoa", - "version": 29, + "version": 30, }, "pages": Array [ Object { @@ -10971,7 +10971,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -12625,7 +12625,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -14422,7 +14422,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 48, + "preprocessedProfileVersion": 49, "processType": 0, "product": "Firefox", "stackwalk": 1, From 9b09352fa6191870ff64cfef6dd21cef49d6acb5 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 19:20:00 +0200 Subject: [PATCH 06/10] Add sanitized-string logic to formatFromMarkerSchema and test --- src/profile-logic/marker-schema.js | 1 + src/test/unit/marker-schema.test.js | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/profile-logic/marker-schema.js b/src/profile-logic/marker-schema.js index f3a428eb8c..8655014620 100644 --- a/src/profile-logic/marker-schema.js +++ b/src/profile-logic/marker-schema.js @@ -472,6 +472,7 @@ export function formatFromMarkerSchema( switch (format) { case 'url': case 'file-path': + case 'sanitized-string': case 'string': // Make sure a non-empty string is returned here. return String(value) || '(empty)'; diff --git a/src/test/unit/marker-schema.test.js b/src/test/unit/marker-schema.test.js index 569c41e7f6..c958d9b86a 100644 --- a/src/test/unit/marker-schema.test.js +++ b/src/test/unit/marker-schema.test.js @@ -210,6 +210,7 @@ describe('marker schema formatting', function () { ['file-path', '/Users/me/gecko'], ['file-path', null], ['file-path', undefined], + ['sanitized-string', 'domain.name'], ['duration', 0], ['duration', 10], ['duration', 12.3456789], @@ -298,6 +299,7 @@ describe('marker schema formatting', function () { "file-path - /Users/me/gecko", "file-path - (empty)", "file-path - (empty)", + "sanitized-string - domain.name", "duration - 0s", "duration - 10ms", "duration - 12.346ms", From 1c63a325e99b2efe89f9aee12457096334a9b040 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 19:34:17 +0200 Subject: [PATCH 07/10] Add unit test for sanitized-string fields --- src/test/unit/sanitize.test.js | 57 ++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/test/unit/sanitize.test.js b/src/test/unit/sanitize.test.js index d0916365ac..21c0f2e83b 100644 --- a/src/test/unit/sanitize.test.js +++ b/src/test/unit/sanitize.test.js @@ -108,6 +108,27 @@ describe('sanitizePII', function () { }, ], }, + HostResolver: { + name: 'HostResolver', + tableLabel: '{marker.name} - {marker.data.host}', + display: ['marker-chart', 'marker-table'], + data: [ + { + key: 'host', + format: 'sanitized-string', + searchable: true, + }, + { + key: 'originSuffix', + format: 'sanitized-string', + searchable: true, + }, + { + key: 'flags', + format: 'string', + }, + ], + }, }; const sanitizedProfile = sanitizePII( @@ -792,6 +813,42 @@ describe('sanitizePII', function () { expect(marker.url).toBe('https://'); }); + it('should sanitize the sanitized-string markers', function () { + const { sanitizedProfile } = setup( + { + shouldRemoveUrls: true, + }, + getProfileWithMarkers([ + [ + 'nsHostResolver::ResolveHost', + 1, + 2, + { + type: 'HostResolver', + host: 'domain.name', + originSuffix: '^other.domain', + flags: '0xf00ba4', + }, + ], + ]) + ); + expect(sanitizedProfile.threads.length).toEqual(1); + const thread = sanitizedProfile.threads[0]; + expect(thread.markers.length).toEqual(1); + + const marker = thread.markers.data[0]; + + // The host fields should still be there + if (!marker || !marker.host) { + throw new Error('Failed to find host property in the payload'); + } + + // Now check the host fields and make sure they are sanitized. + expect(marker.host).toBe(''); + expect(marker.originSuffix).toBe(''); + expect(marker.flags).toBe('0xf00ba4'); + }); + it('should sanitize the eTLD+1 field if urls are supposed to be sanitized', function () { // Create a simple profile with eTLD+1 field in its thread. const { profile } = getProfileFromTextSamples('A'); From 68b7702fe63a15f40964ee07d8e867f84d121326 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 19:38:43 +0200 Subject: [PATCH 08/10] Add sanitized-string to MarkerFormatType --- src/types/markers.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/types/markers.js b/src/types/markers.js index 292a2c6956..f5794fe10c 100644 --- a/src/types/markers.js +++ b/src/types/markers.js @@ -24,6 +24,8 @@ export type MarkerFormatType = | 'url' // Show the file path, and handle PII sanitization. | 'file-path' + // Show regular string, and handle PII sanitization. + | 'sanitized-string' // 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. // e.g. "Label: Some String" From 03935e49058616172e45cd71635b8a2fca65048e Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 23:33:59 +0200 Subject: [PATCH 09/10] Fix test lint --- src/test/unit/sanitize.test.js | 2 +- src/types/markers.js | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/test/unit/sanitize.test.js b/src/test/unit/sanitize.test.js index 21c0f2e83b..e2b1f04451 100644 --- a/src/test/unit/sanitize.test.js +++ b/src/test/unit/sanitize.test.js @@ -821,8 +821,8 @@ describe('sanitizePII', function () { getProfileWithMarkers([ [ 'nsHostResolver::ResolveHost', + 0, 1, - 2, { type: 'HostResolver', host: 'domain.name', diff --git a/src/types/markers.js b/src/types/markers.js index f5794fe10c..b613572b3a 100644 --- a/src/types/markers.js +++ b/src/types/markers.js @@ -763,6 +763,13 @@ export type UrlMarkerPayload = {| url: string, |}; +export type HostResolverPayload = {| + type: 'HostResolver', + host: string, + originSuffix: string, + flags: string, +|} + /** * The union of all the different marker payloads that profiler.firefox.com knows about, * this is not guaranteed to be all the payloads that we actually get from the Gecko @@ -793,7 +800,8 @@ export type MarkerPayload = | JankPayload | BrowsertimeMarkerPayload | NoPayloadUserData - | UrlMarkerPayload; + | UrlMarkerPayload + | HostResolverPayload; export type MarkerPayload_Gecko = | GPUMarkerPayload From 4fe78baed6337537e70dcc91130e427e492e8e69 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 22 May 2024 23:35:57 +0200 Subject: [PATCH 10/10] missing ; --- src/types/markers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/markers.js b/src/types/markers.js index b613572b3a..dbba01fd08 100644 --- a/src/types/markers.js +++ b/src/types/markers.js @@ -768,7 +768,7 @@ export type HostResolverPayload = {| host: string, originSuffix: string, flags: string, -|} +|}; /** * The union of all the different marker payloads that profiler.firefox.com knows about,