From 7eaad981e103033d79b2ced2474bcbfdb32387de Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 20 May 2026 17:51:34 -0400 Subject: [PATCH] Unify profile decoding paths. --- src/actions/receive-profile.ts | 13 +- src/profile-query/loader.ts | 2 +- .../__snapshots__/profile-fetch.test.ts.snap | 36 ----- src/test/unit/profile-fetch.test.ts | 130 +++++++----------- src/utils/profile-fetch.ts | 64 ++------- 5 files changed, 63 insertions(+), 182 deletions(-) diff --git a/src/actions/receive-profile.ts b/src/actions/receive-profile.ts index 7eb4182cdd..ae89921c21 100644 --- a/src/actions/receive-profile.ts +++ b/src/actions/receive-profile.ts @@ -1015,11 +1015,10 @@ export function retrieveProfileOrZipFromUrl( }); switch (response.responseType) { - case 'PROFILE': { - const serializedProfile = response.profile; + case 'BYTES': { const profileUpgradeInfo = {}; const profile = await unserializeProfileOfArbitraryFormat( - serializedProfile, + response.bytes, profileUrl, profileUpgradeInfo ); @@ -1197,13 +1196,15 @@ export function retrieveProfilesToCompare( dispatch(temporaryError(e)); }, }); - if (response.responseType !== 'PROFILE') { - throw new Error('Expected to receive a profile from fetchProfile'); + if (response.responseType !== 'BYTES') { + throw new Error( + 'Expected to receive a single profile (not a zip) from fetchProfile' + ); } const upgradeInfo: ProfileUpgradeInfo = {}; const profile = await unserializeProfileOfArbitraryFormat( - response.profile, + response.bytes, profileUrl, upgradeInfo ); diff --git a/src/profile-query/loader.ts b/src/profile-query/loader.ts index e1f4228273..5d05271f50 100644 --- a/src/profile-query/loader.ts +++ b/src/profile-query/loader.ts @@ -163,7 +163,7 @@ async function fetchAndParseProfile( const upgradeInfo: ProfileUpgradeInfo = {}; const profile = await unserializeProfileOfArbitraryFormat( - response.profile, + response.bytes, fetchUrl, upgradeInfo ); diff --git a/src/test/unit/__snapshots__/profile-fetch.test.ts.snap b/src/test/unit/__snapshots__/profile-fetch.test.ts.snap index 1de4f42d93..752ebee1a8 100644 --- a/src/test/unit/__snapshots__/profile-fetch.test.ts.snap +++ b/src/test/unit/__snapshots__/profile-fetch.test.ts.snap @@ -1,41 +1,5 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing -exports[`fetchProfile fails if a bad profile JSON is passed in 1`] = `[Error: The profile’s JSON could not be decoded. The full error information has been printed out to the DevTool’s console.]`; - -exports[`fetchProfile fails if a bad profile JSON is passed in 2`] = ` -Array [ - Array [ - "The profile’s JSON could not be decoded.", - ], - Array [ - "JSON parsing error:", - [SyntaxError: Unexpected token 'i', "invalid" is not valid JSON], - ], - Array [ - "Fetch response:", - Response {}, - ], -] -`; - -exports[`fetchProfile fails if a bad profile JSON is passed in, with no content type 1`] = `[Error: The profile’s JSON could not be decoded. The full error information has been printed out to the DevTool’s console.]`; - -exports[`fetchProfile fails if a bad profile JSON is passed in, with no content type 2`] = ` -Array [ - Array [ - "The profile’s JSON could not be decoded.", - ], - Array [ - "JSON parsing error:", - [SyntaxError: Unexpected token 'i', "invalid" is not valid JSON], - ], - Array [ - "Fetch response:", - Response {}, - ], -] -`; - exports[`fetchProfile fails if a bad zip file is passed in 1`] = `[Error: Unable to open the archive file. The full error information has been printed out to the DevTool’s console.]`; exports[`fetchProfile fails if a bad zip file is passed in 2`] = ` diff --git a/src/test/unit/profile-fetch.test.ts b/src/test/unit/profile-fetch.test.ts index 8484c9aee1..58596c0751 100644 --- a/src/test/unit/profile-fetch.test.ts +++ b/src/test/unit/profile-fetch.test.ts @@ -2,11 +2,11 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import JSZip from 'jszip'; +import zlib from 'zlib'; import { fetchProfile } from '../../utils/profile-fetch'; import { serializeProfileToJsonString } from '../../profile-logic/process-profile'; import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; -import { extractArrayBuffer } from '../fixtures/utils'; import type { Profile } from 'firefox-profiler/types'; @@ -22,15 +22,12 @@ function _getSimpleProfile(): Profile { } /** - * fetchProfile has a decent amount of complexity around different issues with loading - * in different support URL formats. It's mainly testing what happens when JSON - * and zip file is sent, and what happens when things fail. + * fetchProfile is responsible for downloading bytes and distinguishing ZIPs + * (via content-type or URL extension) from everything else. Format detection + * and gzip decompression happen downstream in unserializeProfileOfArbitraryFormat, + * so the non-ZIP path here just returns raw bytes. */ describe('fetchProfile', function () { - /** - * This helper function encapsulates various configurations for the type of content - * as well and response headers. - */ async function configureFetch(obj: { url: string; contentType?: string; @@ -38,26 +35,25 @@ describe('fetchProfile', function () { }) { const { url, contentType, content } = obj; const stringProfile = serializeProfileToJsonString(_getSimpleProfile()); - const profile = JSON.parse(stringProfile); - let arrayBuffer; + let bytes: Uint8Array; switch (content) { case 'generated-zip': { const zip = new JSZip(); zip.file('profile.json', stringProfile); - arrayBuffer = await zip.generateAsync({ type: 'uint8array' }); + bytes = await zip.generateAsync({ type: 'uint8array' }); break; } case 'generated-json': - arrayBuffer = encode(stringProfile); + bytes = encode(stringProfile); break; default: - arrayBuffer = content; + bytes = content; break; } window.fetchMock.catch(403).get(url, { - body: arrayBuffer, + body: bytes, headers: { 'content-type': contentType, }, @@ -70,19 +66,27 @@ describe('fetchProfile', function () { reportError, }; - // Return fetch's args, based on the inputs. - return { profile, args, reportError }; + return { bytes, args, reportError }; } - it('fetches a normal profile with the correct content-type headers', async function () { - const { profile, args } = await configureFetch({ + function expectBytesEqual(received: unknown, expected: Uint8Array) { + // Uint8Arrays from workers / other realms aren't instanceof Uint8Array here. + expect(Object.prototype.toString.call(received)).toBe( + '[object Uint8Array]' + ); + expect(Array.from(received as Uint8Array)).toEqual(Array.from(expected)); + } + + it('returns the raw bytes when the content-type is application/json', async function () { + const { bytes, args } = await configureFetch({ url: 'https://example.com/profile.json', contentType: 'application/json', content: 'generated-json', }); const profileOrZip = await fetchProfile(args); - expect(profileOrZip).toEqual({ responseType: 'PROFILE', profile }); + expect(profileOrZip.responseType).toBe('BYTES'); + expectBytesEqual((profileOrZip as any).bytes, bytes); }); it('fetches a zipped profile with correct content-type headers', async function () { @@ -108,70 +112,51 @@ describe('fetchProfile', function () { expect(reportError.mock.calls.length).toBe(0); }); - it('fetches a profile with incorrect content-type headers, but .json extension', async function () { - const { profile, args, reportError } = await configureFetch({ + it('returns the raw bytes when the URL has a .json extension but no content type', async function () { + const { bytes, args, reportError } = await configureFetch({ url: 'https://example.com/profile.json', content: 'generated-json', }); const profileOrZip = await fetchProfile(args); - expect(profileOrZip).toEqual({ responseType: 'PROFILE', profile }); + expect(profileOrZip.responseType).toBe('BYTES'); + expectBytesEqual((profileOrZip as any).bytes, bytes); expect(reportError.mock.calls.length).toBe(0); }); - it('fetches a profile with incorrect content-type headers, no known extension, and attempts to JSON parse it', async function () { - const { profile, args, reportError } = await configureFetch({ + it('returns the raw bytes when the URL extension and content type are both unknown', async function () { + const { bytes, args, reportError } = await configureFetch({ url: 'https://example.com/profile.file', content: 'generated-json', }); const profileOrZip = await fetchProfile(args); - expect(profileOrZip).toEqual({ responseType: 'PROFILE', profile }); + expect(profileOrZip.responseType).toBe('BYTES'); + expectBytesEqual((profileOrZip as any).bytes, bytes); expect(reportError.mock.calls.length).toBe(0); }); - it('fails if a bad zip file is passed in', async function () { - const { args, reportError } = await configureFetch({ - url: 'https://example.com/profile.file', - contentType: 'application/zip', - content: new Uint8Array([0, 1, 2, 3]), - }); - - let userFacingError; - try { - await fetchProfile(args); - } catch (error) { - userFacingError = error; - } - expect(userFacingError).toMatchSnapshot(); - expect(reportError.mock.calls.length).toBeGreaterThan(0); - expect(reportError.mock.calls).toMatchSnapshot(); - }); - - it('fails if a bad profile JSON is passed in', async function () { - const invalidJSON = 'invalid'; - const { args, reportError } = await configureFetch({ - url: 'https://example.com/profile.json', - contentType: 'application/json', - content: encode(invalidJSON), + it('passes gzipped bytes through unchanged (decompression is downstream)', async function () { + // Previously fetchProfile decompressed in-place, which detached the + // original ArrayBuffer and broke the fallback path for binary formats + // like JSLB. Now the bytes pass through as-is. + const payload = new Uint8Array([0x89, 0x4a, 0x53, 0x4c, 0x42, 0, 1, 2, 3]); + const gzipped = new Uint8Array(zlib.gzipSync(payload)); + const { args } = await configureFetch({ + url: 'https://example.com/profile.bin', + content: gzipped, }); - let userFacingError; - try { - await fetchProfile(args); - } catch (error) { - userFacingError = error; - } - expect(userFacingError).toMatchSnapshot(); - expect(reportError.mock.calls.length).toBeGreaterThan(0); - expect(reportError.mock.calls).toMatchSnapshot(); + const profileOrZip = await fetchProfile(args); + expect(profileOrZip.responseType).toBe('BYTES'); + expectBytesEqual((profileOrZip as any).bytes, gzipped); }); - it('fails if a bad profile JSON is passed in, with no content type', async function () { - const invalidJSON = 'invalid'; + it('fails if a bad zip file is passed in', async function () { const { args, reportError } = await configureFetch({ - url: 'https://example.com/profile.json', - content: encode(invalidJSON), + url: 'https://example.com/profile.file', + contentType: 'application/zip', + content: new Uint8Array([0, 1, 2, 3]), }); let userFacingError; @@ -184,25 +169,4 @@ describe('fetchProfile', function () { expect(reportError.mock.calls.length).toBeGreaterThan(0); expect(reportError.mock.calls).toMatchSnapshot(); }); - - it('fallback behavior if a completely unknown file is passed in', async function () { - const invalidJSON = 'invalid'; - const profile = encode(invalidJSON); - const { args } = await configureFetch({ - url: 'https://example.com/profile.unknown', - content: profile, - }); - - let userFacingError = null; - try { - const profileOrZip = await fetchProfile(args); - expect(profileOrZip).toEqual({ - responseType: 'PROFILE', - profile: extractArrayBuffer(profile), - }); - } catch (error) { - userFacingError = error; - } - expect(userFacingError).toBeNull(); - }); }); diff --git a/src/utils/profile-fetch.ts b/src/utils/profile-fetch.ts index 3a7f1638ba..8e35829e34 100644 --- a/src/utils/profile-fetch.ts +++ b/src/utils/profile-fetch.ts @@ -154,48 +154,6 @@ export async function extractJsonFromArrayBuffer( return JSON.parse(textDecoder.decode(profileBytes)); } -/** - * Don't trust third party responses, try and handle a variety of responses gracefully. - */ -async function _extractJsonFromResponse( - response: Response, - reportError: (...data: Array) => void, - fileType: 'application/json' | null -): Promise { - let arrayBuffer: ArrayBuffer | null = null; - try { - // await before returning so that we can catch JSON parse errors. - arrayBuffer = await response.arrayBuffer(); - return await extractJsonFromArrayBuffer(arrayBuffer); - } catch (error) { - // Change the error message depending on the circumstance: - let message; - if (error && typeof error === 'object' && error.name === 'AbortError') { - message = 'The network request to load the profile was aborted.'; - } else if (fileType === 'application/json') { - message = 'The profile’s JSON could not be decoded.'; - } else if (fileType === null && arrayBuffer !== null) { - // If the content type is not specified, use a raw array buffer - // to fallback to other supported profile formats. - return arrayBuffer; - } else { - message = oneLine` - The profile could not be downloaded and decoded. This does not look like a supported file - type. - `; - } - - // Provide helpful debugging information to the console. - reportError(message); - reportError('JSON parsing error:', error); - reportError('Fetch response:', response); - - throw new Error( - `${message} The full error information has been printed out to the DevTool’s console.` - ); - } -} - /** * Attempt to load a zip file from a third party. This process can fail, so make sure * to handle and report the error if it does. @@ -228,12 +186,13 @@ async function _extractZipFromResponse( } export type ProfileOrZip = - | { responseType: 'PROFILE'; profile: unknown } + | { responseType: 'BYTES'; bytes: Uint8Array } | { responseType: 'ZIP'; zip: JSZip }; /** - * This function guesses the correct content-type (even if one isn't sent) and then - * attempts to use the proper method to extract the response. + * Use the content-type (or URL extension) to pick between zip and "bytes". + * Bytes are returned as-is; gzip decompression and format detection happen + * downstream in `unserializeProfileOfArbitraryFormat`. */ async function _extractProfileOrZipFromResponse( url: string, @@ -251,17 +210,10 @@ async function _extractProfileOrZipFromResponse( zip: await _extractZipFromResponse(response, reportError), }; case 'application/json': - case null: - // The content type is null if it is unknown, or an unsupported type. Go ahead - // and try to process it as a profile. - return { - responseType: 'PROFILE', - profile: await _extractJsonFromResponse( - response, - reportError, - contentType - ), - }; + case null: { + const buffer = await response.arrayBuffer(); + return { responseType: 'BYTES', bytes: new Uint8Array(buffer) }; + } default: throw assertExhaustiveCheck(contentType); }