Skip to content

Commit dbe20fe

Browse files
committed
Fix overflow when decoding WASM histogram
1 parent cc23b3a commit dbe20fe

3 files changed

Lines changed: 31 additions & 32 deletions

File tree

assembly/Histogram.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,13 @@ export default class Histogram<T, U> extends AbstractHistogramBase<T, U> {
605605
}
606606

607607
setCountAtIndex(index: i32, value: u64): void {
608+
// @ts-ignore
609+
if ((<u64>value) as number > this.maxBucketSize) {
610+
const bitSize = <u8>(sizeof<U>() * 8);
611+
throw new Error(
612+
value.toString() + " would overflow " + bitSize.toString() + "bits integer count"
613+
);
614+
}
608615
// @ts-ignore
609616
unchecked((this.counts[index] = <U>value));
610617
}

assembly/encoding.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,9 @@ function fillCountsArrayFromSourceBuffer<T, U>(
111111
const endPosition = sourceBuffer.position + lengthInBytes;
112112
while (sourceBuffer.position < endPosition) {
113113
let zerosCount: i32 = 0;
114-
let count = <i32>ZigZagEncoding.decode(sourceBuffer);
114+
let count = <i64>ZigZagEncoding.decode(sourceBuffer);
115115
if (count < 0) {
116-
zerosCount = -count;
116+
zerosCount = <i32>-count;
117117
dstIndex += zerosCount; // No need to set zeros in array. Just skip them.
118118
} else {
119119
self.setCountAtIndex(dstIndex++, count);

src/Histogram.spec.ts

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -559,16 +559,13 @@ describe("Histogram clearing support", () => {
559559
});
560560

561561
describe("WASM Histogram bucket size overflow", () => {
562-
let wasmHistogram: Histogram;
563562
beforeAll(initWebAssembly);
564-
afterEach(() => wasmHistogram.destroy());
565-
566563
[8 as const, 16 as const].forEach(
567564
(bitBucketSize) => {
568565
const maxBucketSize = (2 ** bitBucketSize) - 1;
569566
it(`should fail when recording more than ${maxBucketSize} times the same value`, () => {
570567
//given
571-
wasmHistogram = build({ useWebAssembly: true, bitBucketSize });
568+
const wasmHistogram = build({ useWebAssembly: true, bitBucketSize });
572569

573570
//when //then
574571
try {
@@ -577,42 +574,37 @@ describe("WASM Histogram bucket size overflow", () => {
577574
wasmHistogram.recordValue(1);
578575
}
579576
fail(`should have failed due to ${bitBucketSize}bits integer overflow (bucket size : ${i})`);
580-
} catch (e) {
577+
} catch(e) {
581578
//ok
579+
} finally {
580+
wasmHistogram.destroy();
582581
}
583582
});
584583

585-
//Cannot use a destroyed histogram error
586-
it.skip(`should fail when adding two histograms when the same bucket count addition is greater than ${bitBucketSize}bits max integer value`, () => {
587-
//given
588-
const histogram1 = build({ useWebAssembly: true, bitBucketSize });
589-
histogram1.recordValueWithCount(1, maxBucketSize);
590-
const histogram2 = build({ useWebAssembly: true, bitBucketSize });
591-
histogram2.recordValueWithCount(1, maxBucketSize);
592-
593-
//when //then
594-
try {
595-
histogram2.add(histogram1);
596-
fail(`should have failed due to ${bitBucketSize}bits integer overflow`);
597-
} catch (e) {
598-
//ok
599-
}
600-
});
584+
it(`should fail when adding two histograms when the same bucket count addition is greater than ${bitBucketSize}bits max integer value`, () => {
585+
//given
586+
const histogram1 = build({ useWebAssembly: true, bitBucketSize });
587+
histogram1.recordValueWithCount(1, maxBucketSize);
588+
const histogram2 = build({ useWebAssembly: true, bitBucketSize });
589+
histogram2.recordValueWithCount(1, maxBucketSize);
590+
591+
//when //then
592+
expect(() => histogram2.add(histogram1)).toThrow();
593+
594+
histogram1.destroy();
595+
histogram2.destroy();
596+
});
601597
});
602-
//Cannot use a destroyed histogram error
603-
it.skip("should fail when decoding an Int32 histogram with one bucket count greater than 16bits", () => {
598+
599+
it("should fail when decoding an Int32 histogram with one bucket count greater than 16bits in a smaller histogram", () => {
604600
//given
605601
const int32Histogram = build({ useWebAssembly: true, bitBucketSize: 32 }) as WasmHistogram;
606602
int32Histogram.recordValueWithCount(1, 2**32 - 1);
603+
const encodedInt32Histogram = int32Histogram.encodeIntoCompressedBase64();
607604

608605
//when //then
609-
try {
610-
const encodedInt32Histogram = int32Histogram.encodeIntoCompressedBase64();
611-
decodeFromCompressedBase64(encodedInt32Histogram, 16, true);
612-
fail(`should have failed due to 16bits integer overflow`);
613-
} catch (e) {
614-
//ok
615-
}
606+
expect(() => decodeFromCompressedBase64(encodedInt32Histogram, 16, true)).toThrow();
607+
int32Histogram.destroy();
616608
});
617609
});
618610

0 commit comments

Comments
 (0)