diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 8286609..36d75a3 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -5,7 +5,7 @@ name: PR checks # runs concurrently, any failure fails the workflow. # # CodSpeed's first-class GHA integration replaces the manual codspeed-bench -# job from CircleCI: `CodSpeedHQ/action@v3` installs valgrind, sets up +# job from CircleCI: `CodSpeedHQ/action` installs valgrind, sets up # instrumentation, and uploads results in one step. on: @@ -187,7 +187,13 @@ jobs: codspeed-bench: needs: [detect-changes, build] if: needs.detect-changes.outputs.any == 'true' - runs-on: ubuntu-latest + # Pin the OS (not ubuntu-latest) so PR runs are measured in the same + # runtime environment as the baseline seeded on main. A drifting + # ubuntu-latest image changes the valgrind/toolchain fingerprint between + # baseline and PR, which makes CodSpeed report "Different runtime + # environments detected" and refuse to trust the comparison. The baseline + # on main must be re-seeded after changing this so both sides match. + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 @@ -236,7 +242,10 @@ jobs: # See BENCHMARKING.md at the repo root for the full measurement # model, how to read the cold/warm bench split, and what the # CodSpeed dashboard warnings mean. - uses: CodSpeedHQ/action@v4 + # Pinned to a full version (not the floating @v4) so the + # instrumentation environment stays identical between the main + # baseline and PR runs. + uses: CodSpeedHQ/action@v4.18.2 with: mode: simulation run: yarn run bench diff --git a/package.json b/package.json index 9b21500..4649988 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ }, "scripts": { "build:all": "lerna run build:ci --parallel --stream", + "test": "vitest run", "test:all": "lerna run test:ci --parallel --stream", "bench": "lerna run bench --parallel --stream", "bench:ci": "lerna run bench --parallel --stream", diff --git a/packages/dicom-codec/src/codecs/codecFactory.js b/packages/dicom-codec/src/codecs/codecFactory.js index 25fc2d5..c9060ba 100644 --- a/packages/dicom-codec/src/codecs/codecFactory.js +++ b/packages/dicom-codec/src/codecs/codecFactory.js @@ -203,38 +203,42 @@ function getImageFrame(typedArray) { function encode(context, codecConfig, imageFrame, imageInfo, options = {}) { const { iterations = 1 } = options; const encoderInstance = new codecConfig.Encoder(); - const decodedTypedArray = encoderInstance.getDecodedBuffer(imageInfo); - decodedTypedArray.set(imageFrame); + try { + const decodedTypedArray = encoderInstance.getDecodedBuffer(imageInfo); + decodedTypedArray.set(imageFrame); - const { beforeEncode = () => {} } = options; + const { beforeEncode = () => {} } = options; - beforeEncode(encoderInstance, codecConfig); + beforeEncode(encoderInstance, codecConfig); - context.timer.init("To encode length: " + imageFrame.length); - for (let i = 0; i < iterations; i++) { - encoderInstance.encode(); - } + context.timer.init("To encode length: " + imageFrame.length); + for (let i = 0; i < iterations; i++) { + encoderInstance.encode(); + } - context.timer.end(); + context.timer.end(); - const encodedTypedArray = encoderInstance.getEncodedBuffer(); - context.logger.log("Encoded length:" + encodedTypedArray.length); - context.logger.log( - "Encoded is a Typed array of: " + encodedTypedArray.constructor.name - ); + const encodedTypedArray = encoderInstance.getEncodedBuffer(); + context.logger.log("Encoded length:" + encodedTypedArray.length); + context.logger.log( + "Encoded is a Typed array of: " + encodedTypedArray.constructor.name + ); - // cleanup allocated memory - encoderInstance.delete(); + const imageFrameOut = getImageFrame(encodedTypedArray); - const processInfo = { - duration: context.timer.getDuration(), - }; + const processInfo = { + duration: context.timer.getDuration(), + }; - return { - imageFrame: getImageFrame(encodedTypedArray), - imageInfo: getTargetImageInfo(imageInfo, imageInfo), - processInfo, - }; + return { + imageFrame: imageFrameOut, + imageInfo: getTargetImageInfo(imageInfo, imageInfo), + processInfo, + }; + } finally { + // cleanup allocated memory + encoderInstance.delete(); + } } /** @@ -255,40 +259,44 @@ function decode(context, codecConfig, imageFrame, imageInfo) { } const decoderInstance = new codecConfig.Decoder(); - const { length } = imageFrame; - // get pointer to the source/encoded bit stream buffer in WASM memory - // that can hold the encoded bitstream - const encodedTypedArray = decoderInstance.getEncodedBuffer(length); - - // copy the encoded bitstream into WASM memory buffer - encodedTypedArray.set(imageFrame); - context.timer.init("To decode length: " + length); - // decode it - decoderInstance.decode(); - context.timer.end(); + try { + const { length } = imageFrame; + // get pointer to the source/encoded bit stream buffer in WASM memory + // that can hold the encoded bitstream + const encodedTypedArray = decoderInstance.getEncodedBuffer(length); + + // copy the encoded bitstream into WASM memory buffer + encodedTypedArray.set(imageFrame); + context.timer.init("To decode length: " + length); + // decode it + decoderInstance.decode(); + context.timer.end(); + + const decodedTypedArray = decoderInstance.getDecodedBuffer(); + + context.logger.log("Decoded length:" + decodedTypedArray.length); + context.logger.log( + "Decoded is a Typed array of: " + decodedTypedArray.constructor.name + ); - const decodedTypedArray = decoderInstance.getDecodedBuffer(); + // get information about the decoded image + const decodedImageInfo = decoderInstance.getFrameInfo(); - context.logger.log("Decoded length:" + decodedTypedArray.length); - context.logger.log( - "Decoded is a Typed array of: " + decodedTypedArray.constructor.name - ); + const imageFrameOut = getImageFrame(decodedTypedArray); - // get information about the decoded image - const decodedImageInfo = decoderInstance.getFrameInfo(); + const processInfo = { + duration: context.timer.getDuration(), + }; - // cleanup allocated memory - decoderInstance.delete(); - - const processInfo = { - duration: context.timer.getDuration(), - }; - - return { - imageFrame: getImageFrame(decodedTypedArray), - imageInfo: getTargetImageInfo(imageInfo, decodedImageInfo), - processInfo, - }; + return { + imageFrame: imageFrameOut, + imageInfo: getTargetImageInfo(imageInfo, decodedImageInfo), + processInfo, + }; + } finally { + // cleanup allocated memory + decoderInstance.delete(); + } } exports.runProcess = runProcess; diff --git a/packages/dicom-codec/src/codecs/libjpegTurbo12bit.js b/packages/dicom-codec/src/codecs/libjpegTurbo12bit.js index ef26caa..e99b96d 100644 --- a/packages/dicom-codec/src/codecs/libjpegTurbo12bit.js +++ b/packages/dicom-codec/src/codecs/libjpegTurbo12bit.js @@ -1,21 +1,41 @@ +const codecModule = require("@cornerstonejs/codec-libjpeg-turbo-12bit"); +const codecWasmModule = require("@cornerstonejs/codec-libjpeg-turbo-12bit/wasmjs"); +const codecFactory = require("./codecFactory"); + /** * @type {CodecWrapper} */ const codecWrapper = { - // assign it and prevent initialization codec: undefined, Decoder: undefined, Encoder: undefined, - decoderName: "codec libjpeg turbo 12bit", - encoderName: "codec libjpeg turbo 12bit", + encoderName: "JPEGEncoder", + decoderName: "JPEGDecoder", }; +/** + * Decode imageFrame using libjpegTurbo 12bit decoder. + * + * @param {TypedArray} imageFrame to decode. + * @param {ExtendedImageInfo} imageInfo image info options. + * @returns Object containing decoded image frame and imageInfo (current) data. + */ async function decode(imageFrame, imageInfo) { - throw Error("Decoder not found for codec:" + codecWrapper.encoderName); + return codecFactory.runProcess( + codecWrapper, + codecModule, + codecWasmModule, + codecWrapper.decoderName, + (context) => { + return codecFactory.decode(context, codecWrapper, imageFrame, imageInfo); + } + ); } /** - * <> Encode imageFrame to libjpegTurbo 12bits format. + * <> The libjpeg-turbo 12bit build does not expose an + * encoder (see src/jslib.cpp — the JPEGEncoder bindings are disabled), so + * encoding is not supported for this codec. * * @param {TypedArray} imageFrame to encode. * @param {ExtendedImageInfo} imageInfo image info options. @@ -23,13 +43,11 @@ async function decode(imageFrame, imageInfo) { * @returns Object containing encoded image frame and imageInfo (current) data */ async function encode(imageFrame, imageInfo, options = {}) { - throw Error("Encoder not found for codec:" + codecWrapper.encoderName); + throw Error("Encoder not supported for codec: libjpeg-turbo 12bit"); } function getPixelData(imageFrame, imageInfo) { - throw Error( - "GetPixel not found or not applied for codec:" + codecWrapper.encoderName - ); + return codecFactory.getPixelData(imageFrame, imageInfo); } exports.decode = decode; diff --git a/packages/dicom-codec/test/dispatch.test.js b/packages/dicom-codec/test/dispatch.test.js index 8f58634..03cd8bb 100644 --- a/packages/dicom-codec/test/dispatch.test.js +++ b/packages/dicom-codec/test/dispatch.test.js @@ -37,6 +37,47 @@ const SUPPORTED_UIDS = [ "1.2.840.10008.1.2.5", ] +describe("codecFactory instance cleanup", () => { + it("frees the decoder instance even when decode() throws", () => { + const codecFactory = require("../src/codecs/codecFactory") + + let deleted = false + + class FakeDecoder { + getEncodedBuffer() { + return { set: () => {} } + } + + decode() { + throw new Error("boom") + } + + delete() { + deleted = true + } + } + + const codecConfig = { Decoder: FakeDecoder } + const context = { + timer: { + init: () => {}, + end: () => {}, + getDuration: () => 0, + }, + logger: { + log: () => {}, + }, + } + const imageFrame = new Uint8Array([1, 2, 3]) + const imageInfo = {} + + expect(() => + codecFactory.decode(context, codecConfig, imageFrame, imageInfo) + ).toThrow("boom") + expect(deleted).toBe(true) + }) +}) + describe.skipIf(!ALL_BUILT)("dicom-codec dispatcher", () => { let dicomCodec diff --git a/packages/dicom-codec/test/integration.test.js b/packages/dicom-codec/test/integration.test.js index b931898..ac12660 100644 --- a/packages/dicom-codec/test/integration.test.js +++ b/packages/dicom-codec/test/integration.test.js @@ -9,6 +9,9 @@ const packagesRoot = resolve(__dirname, "../..") const LIBJPEG_8BIT_BUILT = existsSync( resolve(packagesRoot, "libjpeg-turbo-8bit/dist/libjpegturbojs.js") ) +const LIBJPEG_12BIT_BUILT = existsSync( + resolve(packagesRoot, "libjpeg-turbo-12bit/dist/libjpegturbo12js.js") +) const CHARLS_BUILT = existsSync( resolve(packagesRoot, "charls/dist/charlsjs.js") ) @@ -20,7 +23,11 @@ const OPENJPH_BUILT = existsSync( ) const ALL_BUILT = - LIBJPEG_8BIT_BUILT && CHARLS_BUILT && OPENJPEG_BUILT && OPENJPH_BUILT + LIBJPEG_8BIT_BUILT && + LIBJPEG_12BIT_BUILT && + CHARLS_BUILT && + OPENJPEG_BUILT && + OPENJPH_BUILT describe.skipIf(!ALL_BUILT)("dicom-codec integration", () => { let dicomCodec @@ -61,6 +68,37 @@ describe.skipIf(!ALL_BUILT)("dicom-codec integration", () => { }) }) + describe("JPEG Baseline 12-bit (1.2.840.10008.1.2.4.51)", () => { + const jpeg12BitBytes = readFileSync( + resolve( + packagesRoot, + "libjpeg-turbo-12bit/test/fixtures/jpeg/CT-512x512-12bit.jpg" + ) + ) + + it("decodes through the dispatcher", async () => { + const imageInfo = { + rows: 512, + columns: 512, + bitsAllocated: 16, + samplesPerPixel: 1, + pixelRepresentation: 0, + signed: false, + } + + const result = await dicomCodec.decode( + jpeg12BitBytes, + imageInfo, + "1.2.840.10008.1.2.4.51" + ) + + expect(result.imageFrame.byteLength).toBe(512 * 512 * 2) + expect(result.imageInfo.width).toBe(512) + expect(result.imageInfo.height).toBe(512) + expect(typeof result.processInfo.duration).toBe("number") + }) + }) + describe("JPEG-LS Lossless (1.2.840.10008.1.2.4.80)", () => { const jlsBytes = readFileSync( resolve(packagesRoot, "charls/test/fixtures/CT1.JLS") diff --git a/packages/libjpeg-turbo-12bit/package.json b/packages/libjpeg-turbo-12bit/package.json index 5096848..942d24e 100644 --- a/packages/libjpeg-turbo-12bit/package.json +++ b/packages/libjpeg-turbo-12bit/package.json @@ -2,7 +2,11 @@ "name": "@cornerstonejs/codec-libjpeg-turbo-12bit", "version": "0.4.1", "description": "JS/WASM Build of [libjpeg-turbo](https://github.com/libjpeg-turbo) WITH12BIT=ON", - "main": "dist/libjpeg-turbojs.js", + "main": "dist/libjpegturbo12js.js", + "exports": { + ".": "./dist/libjpegturbo12js.js", + "./wasmjs": "./dist/libjpegturbo12wasm.js" + }, "publishConfig": { "access": "public" }, @@ -20,8 +24,9 @@ "scripts": { "build": "bash build.sh", "build:ci": "yarn run build", - "test": "echo 'libjpeg-turbo-12bit tests skipped (.51 transfer syntax disabled)'", + "test": "vitest run", "test:ci": "yarn run test", + "test:watch": "vitest", "prepublishOnly": "yarn run build" }, "author": "", diff --git a/packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp b/packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp index 7a15c92..4f9caea 100644 --- a/packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp +++ b/packages/libjpeg-turbo-12bit/src/JPEGDecoder.hpp @@ -3,7 +3,9 @@ #pragma once +#include #include +#include #include // #include "config.h" #include "jpeglib.h" @@ -14,6 +16,7 @@ using namespace std; #include thread_local const emscripten::val Uint8ClampedArray = emscripten::val::global("Uint8ClampedArray"); +thread_local const emscripten::val Uint16Array = emscripten::val::global("Uint16Array"); #endif @@ -54,13 +57,16 @@ class JPEGDecoder { /// holds the decoded pixel data /// emscripten::val getDecodedBuffer() { - // Create a JavaScript-friendly result from the memory view - // instead of relying on the consumer to detach it from WASM memory - // See https://web.dev/webassembly-memory-debugging/ - emscripten::val js_result = Uint8ClampedArray.new_(emscripten::typed_memory_view( + // decoded_ holds one 12-bit grayscale sample per pixel in an int16_t + // (values 0..4095). Copy it into a JS-owned Uint16Array so the result is + // detached from WASM memory (see https://web.dev/webassembly-memory-debugging/). + // NOTE: must be a 16-bit typed array — wrapping in Uint8ClampedArray would + // run every sample through ToUint8Clamp and flatten anything above 255, + // destroying the 12-bit output. + emscripten::val js_result = Uint16Array.new_(emscripten::typed_memory_view( decoded_.size(), decoded_.data() )); - + return js_result; } #else @@ -119,28 +125,55 @@ class JPEGDecoder { jpeg_mem_src(&cinfo, encoded_.data(), encoded_.size()); // Read file header, set default decompression parameters jpeg_read_header(&cinfo, TRUE); - // Force RGBA decoding, even for grayscale images - cinfo.out_color_space = JCS_EXT_RGBA; + // Decode as single-component grayscale. This is a 12-bit-per-sample + // codec: each output value is a 16-bit-wide JSAMPLE (holding 0..4095), + // not an 8-bit RGBA quad. Previously this forced a 4-samples-per-pixel + // RGBA colorspace while the output buffer below was sized for 1 + // sample/pixel, causing libjpeg to write ~2x past the end of the + // allocated buffer (heap overflow). + cinfo.out_color_space = JCS_GRAYSCALE; jpeg_start_decompress(&cinfo); frameInfo_.width = cinfo.output_width; frameInfo_.height = cinfo.output_height; - frameInfo_.bitsPerSample = 8; - frameInfo_.componentCount = 1; //inColorspace == 2 ? 1 : 3; - - // Prepare output buffer - // int pixelFormat = (frameInfo_.componentCount == 1) ? TJPF_GRAY : TJPF_RGB; + frameInfo_.bitsPerSample = 12; + frameInfo_.componentCount = 1; + + // Prepare output buffer. One JSAMPLE (short, holding 0..4095) per pixel + // since output is single-component grayscale. + const int pixelFormat = 1; + + // Compute the output size (in samples) using a checked 64-bit multiply + // capped at 512 MiB so a malformed/adversarial header cannot overflow + // the size computation or force an unbounded allocation. + constexpr uint64_t kMaxOutputSamples = 512ull * 1024ull * 1024ull; // 512 MiB worth of samples + const uint64_t width64 = static_cast(cinfo.output_width); + const uint64_t height64 = static_cast(cinfo.output_height); + const uint64_t pixelFormat64 = static_cast(pixelFormat); + + if (width64 == 0 || height64 == 0) { + jpeg_destroy_decompress(&cinfo); + throw std::runtime_error("Invalid JPEG dimensions (zero width or height)"); + } - // const size_t destinationSize = frameInfo_.width * frameInfo_.height * tjPixelSize[pixelFormat]; - int pixelFormat = 1; - size_t output_size = cinfo.output_width * cinfo.output_height * pixelFormat; + uint64_t output_size64 = width64 * height64; + if (output_size64 / width64 != height64) { + // width * height overflowed + jpeg_destroy_decompress(&cinfo); + throw std::runtime_error("Overflow computing decoded buffer size"); + } + output_size64 *= pixelFormat64; + if (output_size64 == 0 || output_size64 > kMaxOutputSamples) { + jpeg_destroy_decompress(&cinfo); + throw std::runtime_error("Decoded buffer size exceeds allowed maximum or is invalid"); + } - // std::vector output_buffer(output_size); + const size_t output_size = static_cast(output_size64); decoded_.resize(output_size); - auto stride = cinfo.output_width * pixelFormat; + const size_t stride = static_cast(cinfo.output_width) * static_cast(pixelFormat); // Process data while (cinfo.output_scanline < cinfo.output_height) { @@ -149,10 +182,6 @@ class JPEGDecoder { } jpeg_finish_decompress(&cinfo); - // Step 7: release JPEG compression object - - // auto data = Uint8ClampedArray.new_(typed_memory_view(output_size, &output_buffer[0])); - // This is an important step since it will release a good deal of memory. jpeg_destroy_decompress(&cinfo); } diff --git a/packages/libjpeg-turbo-12bit/test/decode.test.js b/packages/libjpeg-turbo-12bit/test/decode.test.js new file mode 100644 index 0000000..589b465 --- /dev/null +++ b/packages/libjpeg-turbo-12bit/test/decode.test.js @@ -0,0 +1,89 @@ +import { beforeAll, describe, expect, it } from "vitest" +import { existsSync, readFileSync } from "node:fs" +import { fileURLToPath } from "node:url" +import { dirname, resolve } from "node:path" + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const fixturesDir = resolve(__dirname, "fixtures") + +// Genuine 12-bit baseline JPEG fixture (SOF marker 0xC1, precision=12, +// 512x512, 1 component — verified by inspecting the JPEG SOF segment). +const ct12bit = readFileSync(resolve(fixturesDir, "jpeg/CT-512x512-12bit.jpg")) + +async function loadModule(modulePath) { + const mod = await import(modulePath) + const factory = mod.default ?? mod + return await factory() +} + +const buildVariants = [ + { name: "asm.js (libjpegturbo12js)", path: "../dist/libjpegturbo12js.js" }, + { name: "wasm (libjpegturbo12wasm)", path: "../dist/libjpegturbo12wasm.js" }, +] + +describe.each(buildVariants)("libjpeg-turbo-12bit decode — $name", ({ path }) => { + const isBuilt = existsSync(resolve(__dirname, path)) + let codec + + beforeAll(async () => { + if (isBuilt) { + codec = await loadModule(path) + } + }) + + it.skipIf(!isBuilt)( + "decodes the CT-512x512 12-bit fixture and reports correct dimensions/format", + () => { + const decoder = new codec.JPEGDecoder() + const encodedBuffer = decoder.getEncodedBuffer(ct12bit.length) + encodedBuffer.set(ct12bit) + + decoder.decode() + + const frameInfo = decoder.getFrameInfo() + expect(frameInfo.width).toBe(512) + expect(frameInfo.height).toBe(512) + expect(frameInfo.bitsPerSample).toBe(12) + expect(frameInfo.componentCount).toBe(1) + + const decoded = decoder.getDecodedBuffer() + // One 16-bit-wide sample per pixel (grayscale, 1 component/pixel). + expect(decoded.length).toBe(512 * 512) + + decoder.delete() + } + ) + + // TODO: needs a real 12-bit fixture RAW reference (a decoded pixel buffer + // generated and verified by a trusted independent decoder) to perform a + // pixel-accurate comparison. fixtures/jpeg/CT-512x512-12bit.jpg is a + // genuine 12-bit baseline JPEG (verified via its SOF segment: marker + // 0xC1, precision=12, 512x512, 1 component), but no corresponding decoded + // RAW reference exists in the repo yet, so we do not fabricate expected + // pixel data here. Once fixtures/raw/CT-512x512-12bit.raw is added, + // replace this skip with a Buffer.equals-style comparison like the 8-bit + // decode test. + it.skip("decodes the CT-512x512 12-bit fixture and matches the RAW reference", () => {}) + + it.skipIf(!isBuilt)("handles truncated input without crashing", () => { + // libjpeg treats a premature end-of-file as a recoverable warning (it + // fills the missing scanlines rather than aborting), so decode() may + // return normally instead of throwing. The meaningful guarantee here is + // that truncated input is handled gracefully — it either throws or + // returns, but never corrupts the process. + const truncated = ct12bit.subarray(0, Math.floor(ct12bit.length / 2)) + const decoder = new codec.JPEGDecoder() + const encodedBuffer = decoder.getEncodedBuffer(truncated.length) + encodedBuffer.set(truncated) + + expect(() => { + try { + decoder.decode() + } catch (e) { + // throwing is an acceptable outcome for malformed input + } + }).not.toThrow() + + decoder.delete() + }) +}) diff --git a/packages/libjpeg-turbo-12bit/vitest.config.mjs b/packages/libjpeg-turbo-12bit/vitest.config.mjs new file mode 100644 index 0000000..ee9aa0b --- /dev/null +++ b/packages/libjpeg-turbo-12bit/vitest.config.mjs @@ -0,0 +1,14 @@ +import { defineConfig } from "vitest/config" +import codspeedPlugin from "@codspeed/vitest-plugin" + +export default defineConfig({ + plugins: [codspeedPlugin()], + test: { + name: "libjpeg-turbo-12bit", + include: ["test/**/*.test.js"], + benchmark: { + include: ["bench/**/*.bench.{js,mjs}"], + }, + testTimeout: 30000, + }, +}) diff --git a/packages/libjpeg-turbo-8bit/src/JPEGDecoder.hpp b/packages/libjpeg-turbo-8bit/src/JPEGDecoder.hpp index b3a92af..47ba282 100644 --- a/packages/libjpeg-turbo-8bit/src/JPEGDecoder.hpp +++ b/packages/libjpeg-turbo-8bit/src/JPEGDecoder.hpp @@ -3,7 +3,9 @@ #pragma once +#include #include +#include #include #include @@ -16,6 +18,22 @@ thread_local const emscripten::val Uint8ClampedArray = emscripten::val::global(" #include "FrameInfo.hpp" +/// +/// Computes width * height * components * bytesPerPixel while guarding +/// against 32-bit size_t overflow on the wasm32 target and rejecting +/// unreasonably large decoded buffer sizes. +/// +static inline size_t checkedDecodedSize(uint64_t width, uint64_t height, uint64_t components, uint64_t bytesPerPixel) { + const uint64_t kMaxBytes = 512ull * 1024ull * 1024ull; // 512 MiB + uint64_t total = width * height; + total *= components; + total *= bytesPerPixel; + if (total == 0 || total > kMaxBytes) { + throw std::runtime_error("decoded frame size out of range"); + } + return static_cast(total); +} + /// /// JavaScript API for decoding JPEG bistreams with libjpeg-turbo /// @@ -110,7 +128,7 @@ class JPEGDecoder { int pixelFormat = (frameInfo_.componentCount == 1) ? TJPF_GRAY : TJPF_RGB; - const size_t destinationSize = frameInfo_.width * frameInfo_.height * tjPixelSize[pixelFormat]; + const size_t destinationSize = checkedDecodedSize(frameInfo_.width, frameInfo_.height, 1, tjPixelSize[pixelFormat]); decoded_.resize(destinationSize); if (tjDecompress2(tjInstance, encoded_.data(), encoded_.size(), decoded_.data(), diff --git a/packages/libjpeg-turbo-8bit/test/decode.test.js b/packages/libjpeg-turbo-8bit/test/decode.test.js index 5212d90..ffe4087 100644 --- a/packages/libjpeg-turbo-8bit/test/decode.test.js +++ b/packages/libjpeg-turbo-8bit/test/decode.test.js @@ -1,9 +1,10 @@ import { beforeAll, describe, expect, it } from "vitest" -import { readFileSync } from "node:fs" +import { existsSync, readFileSync } from "node:fs" import { fileURLToPath } from "node:url" import { dirname, resolve } from "node:path" const __dirname = dirname(fileURLToPath(import.meta.url)) +const distDir = resolve(__dirname, "../dist") const fixturesDir = resolve(__dirname, "fixtures") const jpeg400 = readFileSync(resolve(fixturesDir, "jpeg/jpeg400jfif.jpg")) @@ -16,18 +17,19 @@ async function loadModule(modulePath) { } const buildVariants = [ - { name: "asm.js (libjpegturbojs)", path: "../dist/libjpegturbojs.js" }, - { name: "wasm (libjpegturbowasm)", path: "../dist/libjpegturbowasm.js" }, + { name: "asm.js (libjpegturbojs)", path: "../dist/libjpegturbojs.js", dist: "libjpegturbojs.js" }, + { name: "wasm (libjpegturbowasm)", path: "../dist/libjpegturbowasm.js", dist: "libjpegturbowasm.js" }, ] -describe.each(buildVariants)("libjpeg-turbo-8bit decode — $name", ({ path }) => { +describe.each(buildVariants)("libjpeg-turbo-8bit decode — $name", ({ path, dist }) => { + const isBuilt = existsSync(resolve(distDir, dist)) let codec beforeAll(async () => { - codec = await loadModule(path) + if (isBuilt) codec = await loadModule(path) }) - it("decodes the jpeg400 grayscale fixture", () => { + it.skipIf(!isBuilt)("decodes the jpeg400 grayscale fixture", () => { const decoder = new codec.JPEGDecoder() const encodedBuffer = decoder.getEncodedBuffer(jpeg400.length) encodedBuffer.set(jpeg400) @@ -47,7 +49,7 @@ describe.each(buildVariants)("libjpeg-turbo-8bit decode — $name", ({ path }) = decoder.delete() }) - it("throws or marks error on truncated input", () => { + it.skipIf(!isBuilt)("throws or marks error on truncated input", () => { const truncated = jpeg400.subarray(0, Math.floor(jpeg400.length / 2)) const decoder = new codec.JPEGDecoder() const encodedBuffer = decoder.getEncodedBuffer(truncated.length) @@ -61,14 +63,15 @@ describe.each(buildVariants)("libjpeg-turbo-8bit decode — $name", ({ path }) = describe.each(buildVariants)( "libjpeg-turbo-8bit encode + round-trip — $name", - ({ path }) => { + ({ path, dist }) => { + const isBuilt = existsSync(resolve(distDir, dist)) let codec beforeAll(async () => { - codec = await loadModule(path) + if (isBuilt) codec = await loadModule(path) }) - it("encodes raw → JPEG and decodes back to the same dimensions", () => { + it.skipIf(!isBuilt)("encodes raw → JPEG and decodes back to the same dimensions", () => { const frameInfo = { width: 600, height: 800, diff --git a/packages/openjpeg/src/BufferStream.hpp b/packages/openjpeg/src/BufferStream.hpp index cda2e71..59116e4 100644 --- a/packages/openjpeg/src/BufferStream.hpp +++ b/packages/openjpeg/src/BufferStream.hpp @@ -33,15 +33,17 @@ static OPJ_SIZE_T opj_write_to_buffer (void* p_buffer, OPJ_SIZE_T p_nb_bytes, opj_buffer_info_t* p_source_buffer) { - OPJ_BYTE* pbuf = p_source_buffer->buf; - OPJ_BYTE* pcur = p_source_buffer->cur; + OPJ_SIZE_T remaining = p_source_buffer->buf + p_source_buffer->len - p_source_buffer->cur; - OPJ_SIZE_T len = p_source_buffer->len; + if (remaining == 0) + return (OPJ_SIZE_T)-1; - memcpy (p_source_buffer->cur, p_buffer, p_nb_bytes); - p_source_buffer->cur += p_nb_bytes; + OPJ_SIZE_T n = p_nb_bytes > remaining ? remaining : p_nb_bytes; - return p_nb_bytes; + memcpy (p_source_buffer->cur, p_buffer, n); + p_source_buffer->cur += n; + + return n; } static OPJ_SIZE_T @@ -53,7 +55,7 @@ opj_skip_from_buffer (OPJ_SIZE_T len, opj_buffer_info_t* psrc) if (n > len) n = len; - psrc->cur += len; + psrc->cur += n; } else n = (OPJ_SIZE_T)-1; @@ -64,12 +66,15 @@ opj_skip_from_buffer (OPJ_SIZE_T len, opj_buffer_info_t* psrc) static OPJ_BOOL opj_seek_from_buffer (OPJ_OFF_T len, opj_buffer_info_t* psrc) { - OPJ_SIZE_T n = psrc->len; + if (len < 0) + return OPJ_FALSE; + + OPJ_SIZE_T off = (OPJ_SIZE_T)len; - if (n > len) - n = len; + if (off > psrc->len) + off = psrc->len; - psrc->cur = psrc->buf + n; + psrc->cur = psrc->buf + off; return OPJ_TRUE; } diff --git a/packages/openjpeg/src/J2KDecoder.hpp b/packages/openjpeg/src/J2KDecoder.hpp index a226d0f..70fee26 100644 --- a/packages/openjpeg/src/J2KDecoder.hpp +++ b/packages/openjpeg/src/J2KDecoder.hpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include "openjpeg.h" #include "format_defs.h" @@ -15,6 +17,21 @@ #define EMSCRIPTEN_API __attribute__((used)) #define J2K_MAGIC_NUMBER 0x51FF4FFF +/// +/// Computes width * height * components * bytesPerPixel with overflow +/// checking, throwing if the result is zero or exceeds a sane upper bound. +/// +static inline size_t checkedDecodedSize(uint64_t width, uint64_t height, uint64_t components, uint64_t bytesPerPixel) { + const uint64_t kMaxBytes = 512ull * 1024ull * 1024ull; // 512 MiB + uint64_t total = width * height; + total *= components; + total *= bytesPerPixel; + if (total == 0 || total > kMaxBytes) { + throw std::runtime_error("decoded frame size out of range"); + } + return static_cast(total); +} + #ifdef __EMSCRIPTEN__ #include @@ -748,6 +765,9 @@ class J2KDecoder { // NOTE: DICOM only supports OPJ_CODEC_J2K, but not everyone follows this // and some DICOM images will have JP2 encoded bitstreams // http://dicom.nema.org/medical/dicom/2017e/output/chtml/part05/sect_A.4.4.html + if (encoded_.size() < 4) { + throw std::runtime_error("encoded J2K buffer too small"); + } if( ((OPJ_INT32*)encoded_.data())[0] == J2K_MAGIC_NUMBER ){ l_codec = opj_create_decompress(OPJ_CODEC_J2K); }else{ @@ -815,6 +835,12 @@ class J2KDecoder { frameInfo_.width = image->x1; frameInfo_.height = image->y1; frameInfo_.componentCount = image->numcomps; + if (frameInfo_.componentCount != 1 && frameInfo_.componentCount != 3) { + opj_destroy_codec(l_codec); + opj_stream_destroy(l_stream); + opj_image_destroy(image); + throw std::runtime_error("unsupported J2K component count"); + } frameInfo_.isSigned = image->comps[0].sgnd; frameInfo_.bitsPerSample = image->comps[0].prec; @@ -839,7 +865,7 @@ class J2KDecoder { // allocate destination buffer Size sizeAtDecompositionLevel = calculateSizeAtDecompositionLevel(decompositionLevel); const size_t bytesPerPixel = (frameInfo_.bitsPerSample + 8 - 1) / 8; - const size_t destinationSize = sizeAtDecompositionLevel.width * sizeAtDecompositionLevel.height * frameInfo_.componentCount * bytesPerPixel; + const size_t destinationSize = checkedDecodedSize(sizeAtDecompositionLevel.width, sizeAtDecompositionLevel.height, frameInfo_.componentCount, bytesPerPixel); decoded_.resize(destinationSize); // Convert from int32 to native size diff --git a/packages/openjpeg/src/J2KEncoder.hpp b/packages/openjpeg/src/J2KEncoder.hpp index edd3fe7..16a1111 100644 --- a/packages/openjpeg/src/J2KEncoder.hpp +++ b/packages/openjpeg/src/J2KEncoder.hpp @@ -296,9 +296,12 @@ class J2KEncoder { return; // TODO: implement error handling } - // HACK: For now - make encoded buffer the same size as decoded so we can - // avoid messing with BufferStream malloc/free stuff - encoded_.resize(decoded_.size()); + // HACK: For now - make encoded buffer roughly the same size as decoded + // (plus headroom for worst-case expansion) so we can avoid messing with + // BufferStream malloc/free stuff. opj_write_to_buffer clamps writes to + // the buffer's remaining space, which is the hard safety net if this + // estimate is ever too small. + encoded_.resize(decoded_.size() + (decoded_.size() / 2) + 1024); /* open a byte stream for writing and allocate memory for all tiles */ opj_buffer_info_t buffer_info; diff --git a/packages/openjpeg/test/decode.test.js b/packages/openjpeg/test/decode.test.js index 220ff0d..915d030 100644 --- a/packages/openjpeg/test/decode.test.js +++ b/packages/openjpeg/test/decode.test.js @@ -88,6 +88,45 @@ describe.each(buildVariants)("openjpeg J2K decode — $name", ({ path, dist }) = ) }) +describe.each(buildVariants)("openjpeg J2K decode robustness — $name", ({ path, dist }) => { + const isBuilt = existsSync(resolve(distDir, dist)) + let codec + + beforeAll(async () => { + if (isBuilt) codec = await loadModule(path) + }) + + it.skipIf(!isBuilt)("throws when the encoded buffer is smaller than 4 bytes", () => { + const decoder = new codec.J2KDecoder() + const tooShort = new Uint8Array([0x00, 0x01, 0x02]) + decoder.getEncodedBuffer(tooShort.length).set(tooShort) + + expect(() => decoder.decode()).toThrow() + + decoder.delete() + }) + + it.skipIf(!isBuilt)("does not crash the process on a malformed/garbage buffer", () => { + const decoder = new codec.J2KDecoder() + const garbage = new Uint8Array(64) + for (let i = 0; i < garbage.length; i++) garbage[i] = (i * 37 + 11) % 256 + decoder.getEncodedBuffer(garbage.length).set(garbage) + + // Malformed input must either throw or return without corrupting the + // process (e.g. via an out-of-bounds heap write). Reaching this + // expectation at all is the meaningful assertion here. + expect(() => { + try { + decoder.decode() + } catch (e) { + // throwing is an acceptable, expected outcome for malformed input + } + }).not.toThrow() + + decoder.delete() + }) +}) + const encoderVariants = buildVariants.filter((v) => !v.name.includes("decode-only")) describe.each(encoderVariants)( diff --git a/packages/openjphjs/src/HTJ2KDecoder.hpp b/packages/openjphjs/src/HTJ2KDecoder.hpp index dcce11a..5230d2e 100644 --- a/packages/openjphjs/src/HTJ2KDecoder.hpp +++ b/packages/openjphjs/src/HTJ2KDecoder.hpp @@ -3,8 +3,10 @@ #pragma once +#include #include #include +#include #include #include @@ -22,6 +24,22 @@ #include "Point.hpp" #include "Size.hpp" +/// +/// Computes width * height * components * bytesPerPixel while guarding +/// against 32-bit size_t overflow on the wasm32 target and rejecting +/// unreasonably large decoded buffer sizes. +/// +static inline size_t checkedDecodedSize(uint64_t width, uint64_t height, uint64_t components, uint64_t bytesPerPixel) { + const uint64_t kMaxBytes = 512ull * 1024ull * 1024ull; // 512 MiB + uint64_t total = width * height; + total *= components; + total *= bytesPerPixel; + if (total == 0 || total > kMaxBytes) { + throw std::runtime_error("decoded frame size out of range"); + } + return static_cast(total); +} + /// /// JavaScript API for decoding HTJ2K bistreams with OpenJPH /// @@ -314,7 +332,7 @@ class HTJ2KDecoder Size sizeAtDecompositionLevel = calculateSizeAtDecompositionLevel(decompositionLevel); int resolutionLevel = numDecompositions_ - decompositionLevel; const size_t bytesPerPixel = (frameInfo_.bitsPerSample + 8 - 1) / 8; - const size_t destinationSize = sizeAtDecompositionLevel.width * sizeAtDecompositionLevel.height * frameInfo.componentCount * bytesPerPixel; + const size_t destinationSize = checkedDecodedSize(sizeAtDecompositionLevel.width, sizeAtDecompositionLevel.height, frameInfo.componentCount, bytesPerPixel); pDecoded_->resize(destinationSize); // set the level to read to and reconstruction level to the specified decompositionLevel