From b2b62dd5cf9a641be82da574db779b99f195242e Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Thu, 21 Nov 2024 15:39:01 -0800 Subject: [PATCH 1/2] fix(issues): Avoid destructuring null in event data fixes JAVASCRIPT-2WQ3 Possible for the array to contain null which errors when being destructured --- .../events/interfaces/request/index.spec.tsx | 35 +++++++++++++++++++ .../events/interfaces/request/index.tsx | 11 +++--- .../components/events/interfaces/utils.tsx | 20 ++++++----- static/app/types/event.tsx | 6 ++-- 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/static/app/components/events/interfaces/request/index.spec.tsx b/static/app/components/events/interfaces/request/index.spec.tsx index df27c7b78df44f..059c1ce42bef13 100644 --- a/static/app/components/events/interfaces/request/index.spec.tsx +++ b/static/app/components/events/interfaces/request/index.spec.tsx @@ -1,10 +1,12 @@ import {DataScrubbingRelayPiiConfigFixture} from 'sentry-fixture/dataScrubbingRelayPiiConfig'; import {EventFixture} from 'sentry-fixture/event'; +import {UserFixture} from 'sentry-fixture/user'; import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; import {textWithMarkupMatcher} from 'sentry-test/utils'; import {Request} from 'sentry/components/events/interfaces/request'; +import ConfigStore from 'sentry/stores/configStore'; import type {EntryRequest} from 'sentry/types/event'; import {EntryType} from 'sentry/types/event'; @@ -327,6 +329,39 @@ describe('Request entry', function () { ).not.toThrow(); }); + it('should remove any non-tuple values from array', function () { + const user = UserFixture(); + user.options.prefersIssueDetailsStreamlinedUI = true; + ConfigStore.set('user', user); + + const data: EntryRequest['data'] = { + apiTarget: null, + query: 'a%AFc', + data: '', + headers: [['foo', 'bar'], null], + cookies: [], + env: {}, + method: 'POST', + url: '/Home/PostIndex', + }; + const event = EventFixture({ + entries: [ + { + type: EntryType.REQUEST, + data, + }, + ], + }); + expect(() => + render(, { + organization: { + relayPiiConfig: JSON.stringify(DataScrubbingRelayPiiConfigFixture()), + }, + }) + ).not.toThrow(); + ConfigStore.set('user', user); + }); + it("should not cause an invariant violation if data.data isn't a string", function () { const data: EntryRequest['data'] = { apiTarget: null, diff --git a/static/app/components/events/interfaces/request/index.tsx b/static/app/components/events/interfaces/request/index.tsx index 16be046eaa16aa..c1417f34507a58 100644 --- a/static/app/components/events/interfaces/request/index.tsx +++ b/static/app/components/events/interfaces/request/index.tsx @@ -251,10 +251,13 @@ function RequestDataCard({ const contentItems: KeyValueDataContentProps[] = []; if (Array.isArray(data) && data.length > 0) { - data.forEach(([key, value], i: number) => { - const valueMeta = meta?.[i] ? meta[i]?.[1] : undefined; - contentItems.push({item: {key, subject: key, value}, meta: valueMeta}); - }); + data + // Remove any non-tuple values + .filter(x => Array.isArray(x)) + .forEach(([key, value], i: number) => { + const valueMeta = meta?.[i] ? meta[i]?.[1] : undefined; + contentItems.push({item: {key, subject: key, value}, meta: valueMeta}); + }); } else if (typeof data === 'object') { // Spread to flatten if it's a proxy Object.entries({...data}).forEach(([key, value]) => { diff --git a/static/app/components/events/interfaces/utils.tsx b/static/app/components/events/interfaces/utils.tsx index da8de23bbbd49f..5020068be07e2b 100644 --- a/static/app/components/events/interfaces/utils.tsx +++ b/static/app/components/events/interfaces/utils.tsx @@ -125,22 +125,22 @@ export function getCurlCommand(data: EntryRequest['data']) { result += ' \\\n -X ' + data.method; } - data.headers = data.headers?.filter(defined); + const headers = + data.headers + ?.filter(defined) + // sort headers + .sort(function (a, b) { + return a[0] === b[0] ? 0 : a[0] < b[0] ? -1 : 1; + }) ?? []; // TODO(benvinegar): just gzip? what about deflate? - const compressed = data.headers?.find( + const compressed = headers?.find( h => h[0] === 'Accept-Encoding' && h[1].includes('gzip') ); if (compressed) { result += ' \\\n --compressed'; } - // sort headers - const headers = - data.headers?.sort(function (a, b) { - return a[0] === b[0] ? 0 : a[0] < b[0] ? -1 : 1; - }) ?? []; - for (const header of headers) { result += ' \\\n -H "' + header[0] + ': ' + escapeBashString(header[1] + '') + '"'; } @@ -172,7 +172,9 @@ export function getCurlCommand(data: EntryRequest['data']) { return result; } -export function stringifyQueryList(query: string | [key: string, value: string][]) { +export function stringifyQueryList( + query: string | Array<[key: string, value: string] | null> +) { if (typeof query === 'string') { return query; } diff --git a/static/app/types/event.tsx b/static/app/types/event.tsx index e95a322cc3b93d..1ffb547747d7f6 100644 --- a/static/app/types/event.tsx +++ b/static/app/types/event.tsx @@ -336,17 +336,17 @@ export interface EntryRequestDataDefault { apiTarget: null; method: string; url: string; - cookies?: [key: string, value: string][]; + cookies?: Array<[key: string, value: string] | null>; data?: string | null | Record | [key: string, value: any][]; env?: Record; fragment?: string | null; - headers?: [key: string, value: string][]; + headers?: Array<[key: string, value: string] | null>; inferredContentType?: | null | 'application/json' | 'application/x-www-form-urlencoded' | 'multipart/form-data'; - query?: [key: string, value: string][] | string; + query?: Array<[key: string, value: string] | null> | string; } export interface EntryRequestDataGraphQl From e67d74967f8742afc773f9ba9056857ec9115b94 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Thu, 21 Nov 2024 15:46:45 -0800 Subject: [PATCH 2/2] fix test act --- .../app/components/events/interfaces/request/index.spec.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/static/app/components/events/interfaces/request/index.spec.tsx b/static/app/components/events/interfaces/request/index.spec.tsx index 059c1ce42bef13..2ee61131f256e3 100644 --- a/static/app/components/events/interfaces/request/index.spec.tsx +++ b/static/app/components/events/interfaces/request/index.spec.tsx @@ -13,6 +13,10 @@ import {EntryType} from 'sentry/types/event'; jest.unmock('prismjs'); describe('Request entry', function () { + beforeEach(() => { + ConfigStore.set('user', UserFixture()); + }); + it('display redacted data', async function () { const event = EventFixture({ entries: [ @@ -359,7 +363,6 @@ describe('Request entry', function () { }, }) ).not.toThrow(); - ConfigStore.set('user', user); }); it("should not cause an invariant violation if data.data isn't a string", function () {