Add bounds checking for sampleTypeIndex (#449)

Wow this was surprising. As reported in #448, the `simple.speedscope.json` file failed import. This was surprising to me because there's a test that covers that file to ensure it imports correctly.

The file provided in #448, however, is from a version of speedscope from 5 years ago before the file had been pretty printed. It turns out that when this *particular* file is not pretty-printed, it's a schematically valid pprof profile.

The fix is to do some bounds checks and return null. After the change, the file imports as you'd expect after realizing its not actually a valid pprof profile.

Fixes #448
This commit is contained in:
Jamie Wong 2023-12-07 18:43:47 -08:00 committed by GitHub
parent de17f128d0
commit ac4a015559
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 72 additions and 1 deletions

View File

@ -0,0 +1 @@
{"version":"0.0.1","$schema":"https://www.speedscope.app/file-format-schema.json","shared":{"frames":[{"name":"a"},{"name":"b"},{"name":"c"},{"name":"d"}]},"profiles":[{"type":"evented","name":"simple.txt","unit":"none","startValue":0,"endValue":14,"events":[{"type":"O","frame":0,"at":0},{"type":"O","frame":1,"at":0},{"type":"O","frame":2,"at":0},{"type":"C","frame":2,"at":2},{"type":"O","frame":3,"at":2},{"type":"C","frame":3,"at":6},{"type":"O","frame":2,"at":6},{"type":"C","frame":2,"at":9},{"type":"C","frame":1,"at":14},{"type":"C","frame":0,"at":14}]}]}

View File

@ -129,6 +129,11 @@ export function importAsPprofProfile(rawProfile: ArrayBuffer): Profile | null {
}))
const sampleTypeIndex = getSampleTypeIndex(protoProfile)
if (sampleTypeIndex < 0 || sampleTypeIndex >= sampleTypes.length) {
return null
}
const sampleType = sampleTypes[sampleTypeIndex]
const profileBuilder = new StackListProfileBuilder()
@ -149,7 +154,12 @@ export function importAsPprofProfile(rawProfile: ArrayBuffer): Profile | null {
for (let s of protoProfile.sample) {
const stack = s.locationId ? s.locationId.map(l => frameByLocationID.get(i32(l))) : []
stack.reverse()
const value = s.value![sampleTypeIndex]
if (s.value == null || s.value.length <= sampleTypeIndex) {
return null
}
const value = s.value[sampleTypeIndex]
profileBuilder.appendSampleWithWeight(stack.filter(f => f != null) as FrameInfo[], +value)
}

View File

@ -1,5 +1,59 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`importSpeedscopeProfiles 0.0.1 evented profile (no pretty print) 1`] = `
Object {
"frames": Array [
Frame {
"col": undefined,
"file": undefined,
"key": 0,
"line": undefined,
"name": "a",
"selfWeight": 0,
"totalWeight": 14,
},
Frame {
"col": undefined,
"file": undefined,
"key": 1,
"line": undefined,
"name": "b",
"selfWeight": 5,
"totalWeight": 14,
},
Frame {
"col": undefined,
"file": undefined,
"key": 2,
"line": undefined,
"name": "c",
"selfWeight": 5,
"totalWeight": 5,
},
Frame {
"col": undefined,
"file": undefined,
"key": 3,
"line": undefined,
"name": "d",
"selfWeight": 4,
"totalWeight": 4,
},
],
"name": "simple.txt",
"stacks": Array [
"a;b;c 2",
"a;b;d 4",
"a;b;c 3",
"a;b 5",
],
}
`;
exports[`importSpeedscopeProfiles 0.0.1 evented profile (no pretty print): indexToView 1`] = `0`;
exports[`importSpeedscopeProfiles 0.0.1 evented profile (no pretty print): profileGroup.name 1`] = `"simple.txt"`;
exports[`importSpeedscopeProfiles 0.0.1 evented profile 1`] = `
Object {
"frames": Array [

View File

@ -5,6 +5,12 @@ describe('importSpeedscopeProfiles', () => {
await checkProfileSnapshot('./sample/profiles/speedscope/0.0.1/simple.speedscope.json')
})
test('0.0.1 evented profile (no pretty print)', async () => {
await checkProfileSnapshot(
'./sample/profiles/speedscope/0.0.1/simple-no-pretty-print.speedscope.json',
)
})
test('0.1.2 sampled profile', async () => {
await checkProfileSnapshot('./sample/profiles/speedscope/0.1.2/simple-sampled.speedscope.json')
})