From 3f41a33982aab69fbe9ea9cb96e68870ee897169 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 16 Sep 2025 16:08:04 -0400 Subject: [PATCH 1/6] fix(streaming): Reallocate image before processing chunks to prevent out-of-bounds error When loading remote DICOM files via manifest.json, chunks arrive incrementally due to parallel asynchronous processing in importDataSources. This creates a race condition where: 1. Initial chunks (e.g., 1-50) arrive and image is allocated for 50 slices 2. These chunks start loading their data 3. Additional chunks (51-100) arrive with some already having loaded data 4. Without fix: processNewChunks() attempts to write data at positions 51-100 into an array only allocated for 50 slices, causing "offset is out of bounds" 5. With fix: reallocateImage() runs first, expanding the array to accommodate all chunks before any data is written The fix ensures proper ordering: - Move reallocateImage() call before processNewChunks() - Fix index lookup in processNewChunks() to use actual chunk position instead of filtered array index This prevents RangeError when chunks arrive in multiple batches with pre-loaded data from parallel fetching. --- src/core/streaming/dicomChunkImage.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/core/streaming/dicomChunkImage.ts b/src/core/streaming/dicomChunkImage.ts index 326955b07..e9545a246 100644 --- a/src/core/streaming/dicomChunkImage.ts +++ b/src/core/streaming/dicomChunkImage.ts @@ -206,12 +206,12 @@ export default class DicomChunkImage }); this.onChunksUpdated(); - this.registerChunkListeners(); - this.processNewChunks(newChunks); - if (this.getModality() !== 'SEG') { this.reallocateImage(); } + + this.registerChunkListeners(); + this.processNewChunks(newChunks); } getThumbnail(strategy: ThumbnailStrategy): Promise { @@ -237,8 +237,11 @@ export default class DicomChunkImage private processNewChunks(chunks: Chunk[]) { chunks .filter((chunk) => chunk.state === ChunkState.Loaded) - .forEach((_, idx) => { - this.onChunkHasData(idx); + .forEach((chunk) => { + const idx = this.chunks.indexOf(chunk); + if (idx !== -1) { + this.onChunkHasData(idx); + } }); } From 8afe2566796c5bd3168bea06f7cf1f5857e74293 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 18 Sep 2025 16:36:36 -0400 Subject: [PATCH 2/6] fix(dicom): allocate correct slice count for multi-file series with NumberOfFrames=1 Fixed buffer overflow when loading DICOM series where each file has NumberOfFrames=1. The issue occurred when the code incorrectly used NumberOfFrames (1) instead of the actual chunk count for memory allocation, causing an offset out of bounds error when loading subsequent slices. Now correctly uses sortedChunks.length when multiple chunks are present, regardless of individual NumberOfFrames values. Also fixed z-spacing calculation to use the allocated slice count. --- src/utils/allocateImageFromChunks.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/utils/allocateImageFromChunks.ts b/src/utils/allocateImageFromChunks.ts index 75461c486..029fde278 100644 --- a/src/utils/allocateImageFromChunks.ts +++ b/src/utils/allocateImageFromChunks.ts @@ -99,7 +99,9 @@ export function allocateImageFromChunks(sortedChunks: Chunk[]) { ); } - const slices = numberOfFrames === null ? sortedChunks.length : numberOfFrames; + // Some CT modality series still have NumberOfFrames === 1, so use the number of chunks if more than 1 + const slices = + sortedChunks.length > 1 ? sortedChunks.length : numberOfFrames ?? 1; const TypedArrayCtor = getTypedArrayConstructor( bitsStored, pixelRepresentation, @@ -126,7 +128,7 @@ export function allocateImageFromChunks(sortedChunks: Chunk[]) { const zVec = vec3.create(); const firstIPP = imagePositionPatient; vec3.sub(zVec, lastIPP as vec3, firstIPP as vec3); - const zSpacing = vec3.len(zVec) / (sortedChunks.length - 1) || 1; + const zSpacing = vec3.len(zVec) / (slices - 1) || 1; const spacing = [...pixelSpacing, zSpacing]; image.setSpacing(spacing); } From 7661a07b06de270ab76d5c10e6379d23c6bb400e Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 19 Sep 2025 10:32:38 -0400 Subject: [PATCH 3/6] chore(streaming): throw error for unsupported multiframe DICOM volumes --- src/core/streaming/dicomChunkImage.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/core/streaming/dicomChunkImage.ts b/src/core/streaming/dicomChunkImage.ts index e9545a246..d2de3daa8 100644 --- a/src/core/streaming/dicomChunkImage.ts +++ b/src/core/streaming/dicomChunkImage.ts @@ -340,6 +340,14 @@ export default class DicomChunkImage if (!result.image.data) throw new Error('No data read from chunk'); + if (result.image.size[2] > 1 && this.chunks.length > 1) { + // we're trying to load multiple chunks where individual chunks have multiple frames + throw new Error( + `Loading a single volume from multiple DICOM files where individual files contain multiple frames is not supported. ` + + `File ${chunkIndex} contains ${result.image.size[2]} frames.` + ); + } + const scalars = this.vtkImageData.value.getPointData().getScalars(); const pixelData = scalars.getData() as TypedArray; From 87d6ad6926eb06dc13ecc1aec5b1993d26b86a4e Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 19 Sep 2025 11:10:16 -0400 Subject: [PATCH 4/6] chore(vite.config): use default dev server port 5173 not 8080 --- tests/specs/state-manifest.e2e.ts | 2 +- vite.config.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/specs/state-manifest.e2e.ts b/tests/specs/state-manifest.e2e.ts index 14eeee96c..fe8324a0e 100644 --- a/tests/specs/state-manifest.e2e.ts +++ b/tests/specs/state-manifest.e2e.ts @@ -43,7 +43,7 @@ describe.skip('State file manifest.json code', () => { }); // Dev test - // http://localhost:8080/?&urls=[http://localhost:9999/session.volview-2-1-0-labelmap-tools.zip] + // http://localhost:5173/?&urls=[http://localhost:9999/session.volview-2-1-0-labelmap-tools.zip] it('has no errors loading version 2.1.0 manifest.json file ', async () => { const FILE_NAME = 'session.volview-2-1-0-labelmap-tools.zip'; diff --git a/vite.config.ts b/vite.config.ts index 9dff41dfe..b136377e4 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -213,7 +213,6 @@ export default defineConfig({ configureSentryPlugin(), ], server: { - port: 8080, // so `npm run test:e2e:dev` can access the webdriver static server temp directory proxy: { '/tmp': config.baseUrl!, From 1a3221cba4e9301a048a66e359d3515882cee80c Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 19 Sep 2025 12:00:34 -0400 Subject: [PATCH 5/6] fix(streaming): use Content-Type header for known MIME types Trust server-provided Content-Type for JSON, DICOM, ZIP, and GZIP files instead of always using magic byte detection. Fixes manifest loading from URLs without .json extension. --- src/core/streaming/types.ts | 1 + src/io/import/processors/updateUriType.ts | 32 ++++++++++++++++++++++- src/io/io.ts | 24 +++++++++++++---- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/core/streaming/types.ts b/src/core/streaming/types.ts index 9fe61c5b8..32740cd2c 100644 --- a/src/core/streaming/types.ts +++ b/src/core/streaming/types.ts @@ -44,5 +44,6 @@ export interface Fetcher { cachedChunks: Uint8Array[]; connected: boolean; size: number; + contentType?: string; abortSignal?: AbortSignal; } diff --git a/src/io/import/processors/updateUriType.ts b/src/io/import/processors/updateUriType.ts index 3407f1f93..ef78ec248 100644 --- a/src/io/import/processors/updateUriType.ts +++ b/src/io/import/processors/updateUriType.ts @@ -2,10 +2,14 @@ import { Skip } from '@/src/utils/evaluateChain'; import StreamingByteReader from '@/src/core/streaming/streamingByteReader'; import { ImportHandler, asIntermediateResult } from '@/src/io/import/common'; import { getFileMimeFromMagicStream } from '@/src/io/magic'; +import { getMimeTypeFromFilename } from '@/src/io/io'; import { asCoroutine } from '@/src/utils'; const DoneSignal = Symbol('DoneSignal'); +// MIME types that don't need magic byte detection +const TRUSTED_MIME_TYPES = new Set(['application/json', 'application/dicom']); + function detectStreamType(stream: ReadableStream) { return new Promise((resolve, reject) => { const reader = new StreamingByteReader(); @@ -44,11 +48,37 @@ const updateUriType: ImportHandler = async (dataSource) => { return Skip; } + // First, try to determine MIME type from filename extension + const mimeFromFilename = getMimeTypeFromFilename(dataSource.name); + if (mimeFromFilename) { + // Prioritize extension-based MIME type over server-provided type + return asIntermediateResult([ + { + ...dataSource, + mime: mimeFromFilename, + }, + ]); + } + const { fetcher } = dataSource; await fetcher.connect(); + + // First try to use the Content-Type header from the HTTP response + let mime = fetcher.contentType || ''; + + // Extract just the MIME type without charset or other parameters + if (mime.includes(';')) { + mime = mime.split(';')[0].trim(); + } + + // Always get the stream to ensure it's properly teed for later use const stream = fetcher.getStream(); - const mime = await detectStreamType(stream); + + // Use magic detection unless we trust the MIME type + if (!TRUSTED_MIME_TYPES.has(mime)) { + mime = await detectStreamType(stream); + } const streamDataSource = { ...dataSource, diff --git a/src/io/io.ts b/src/io/io.ts index 0a8afbc43..d6ff81fd9 100644 --- a/src/io/io.ts +++ b/src/io/io.ts @@ -2,6 +2,22 @@ import { FILE_EXTENSIONS, FILE_EXT_TO_MIME, MIME_TYPES } from './mimeTypes'; import { Maybe } from '../types'; import { getFileMimeFromMagic } from './magic'; +/** + * Determines MIME type from a filename based on its extension. + * + * @param filename The filename to check + * @returns The MIME type if a known extension is found, null otherwise + */ +export function getMimeTypeFromFilename(filename: string): Maybe { + const supportedExt = [...FILE_EXTENSIONS].find((ext) => + filename.toLowerCase().endsWith(`.${ext}`) + ); + if (supportedExt) { + return FILE_EXT_TO_MIME[supportedExt]; + } + return null; +} + /** * Determines the file's mime type. * @@ -17,11 +33,9 @@ export async function getFileMimeType(file: File): Promise> { return FILE_EXT_TO_MIME[fileType]; } - const supportedExt = [...FILE_EXTENSIONS].find((ext) => - file.name.toLowerCase().endsWith(`.${ext}`) - ); - if (supportedExt) { - return FILE_EXT_TO_MIME[supportedExt]; + const mimeFromFilename = getMimeTypeFromFilename(file.name); + if (mimeFromFilename) { + return mimeFromFilename; } const mimeFromMagic = await getFileMimeFromMagic(file); From 0367026895b98e5fe4311b707b0674125c58db58 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 19 Sep 2025 13:48:49 -0400 Subject: [PATCH 6/6] fix(dicom): surface chunk loading errors to user notifications --- src/core/streaming/dicomChunkImage.ts | 50 +++++++++++++++++--------- src/store/dicom-web/dicom-web-store.ts | 1 - src/store/image-cache.ts | 4 +++ src/store/messages.ts | 2 ++ src/store/tools/paintProcess.ts | 1 - src/utils/allocateImageFromChunks.ts | 3 +- 6 files changed, 41 insertions(+), 20 deletions(-) diff --git a/src/core/streaming/dicomChunkImage.ts b/src/core/streaming/dicomChunkImage.ts index d2de3daa8..05c7199f0 100644 --- a/src/core/streaming/dicomChunkImage.ts +++ b/src/core/streaming/dicomChunkImage.ts @@ -212,6 +212,11 @@ export default class DicomChunkImage this.registerChunkListeners(); this.processNewChunks(newChunks); + + // Update data range with already loaded chunks after reallocating image + if (this.getModality() !== 'SEG') { + this.updateDataRangeFromChunks(); + } } getThumbnail(strategy: ThumbnailStrategy): Promise { @@ -235,21 +240,22 @@ export default class DicomChunkImage } private processNewChunks(chunks: Chunk[]) { - chunks - .filter((chunk) => chunk.state === ChunkState.Loaded) - .forEach((chunk) => { - const idx = this.chunks.indexOf(chunk); - if (idx !== -1) { - this.onChunkHasData(idx); - } + chunks.forEach((chunk, idx) => { + if (chunk.state !== ChunkState.Loaded) return; + + this.onChunkHasData(idx).catch((err) => { + this.onChunkErrored(idx, err); }); + }); } private registerChunkListeners() { this.chunkListeners = [ ...this.chunks.map((chunk, index) => { const stopDoneData = chunk.addEventListener('doneData', () => { - this.onChunkHasData(index); + this.onChunkHasData(index).catch((err) => { + this.onChunkErrored(index, err); + }); }); const stopError = chunk.addEventListener('error', (err) => { @@ -273,13 +279,17 @@ export default class DicomChunkImage private reallocateImage() { this.vtkImageData.value.delete(); this.vtkImageData.value = allocateImageFromChunks(this.chunks); + } - // recalculate image data's data range, since allocateImageFromChunks doesn't know anything about it + private updateDataRangeFromChunks() { const scalars = this.vtkImageData.value.getPointData().getScalars(); - this.dataRangeFromChunks().forEach(([min, max], compIdx) => { - scalars.setRange({ min, max }, compIdx); - }); - scalars.modified(); // so image-stats will trigger update of range + const ranges = this.dataRangeFromChunks(); + if (ranges.length > 0) { + ranges.forEach(([min, max], compIdx) => { + scalars.setRange({ min, max }, compIdx); + }); + scalars.modified(); // so image-stats will trigger update of range + } } private dataRangeFromChunks() { @@ -312,7 +322,9 @@ export default class DicomChunkImage private async onSegChunkHasData(chunkIndex: number) { if (this.chunks.length !== 1 || chunkIndex !== 0) - throw new Error('cannot handle multiple seg files'); + throw new Error( + `Cannot handle multiple SEG files. Expected 1 chunk at index 0, got ${this.chunks.length} chunks with current index ${chunkIndex}` + ); const [chunk] = this.chunks; const results = await buildSegmentGroups( @@ -330,7 +342,10 @@ export default class DicomChunkImage private async onRegularChunkHasData(chunkIndex: number) { const chunk = this.chunks[chunkIndex]; - if (!chunk.dataBlob) throw new Error('Chunk does not have data'); + if (!chunk.dataBlob) + throw new Error(`Chunk ${chunkIndex} does not have data`); + + const chunkId = chunk.metadata ? getChunkId(chunk) : `index-${chunkIndex}`; const result = await readImage( new File([chunk.dataBlob], `file-${chunkIndex}.dcm`), { @@ -338,13 +353,14 @@ export default class DicomChunkImage } ); - if (!result.image.data) throw new Error('No data read from chunk'); + if (!result.image.data) + throw new Error(`No data read from chunk ${chunkId}`); if (result.image.size[2] > 1 && this.chunks.length > 1) { // we're trying to load multiple chunks where individual chunks have multiple frames throw new Error( `Loading a single volume from multiple DICOM files where individual files contain multiple frames is not supported. ` + - `File ${chunkIndex} contains ${result.image.size[2]} frames.` + `File ${chunkId} (chunk ${chunkIndex}) contains ${result.image.size[2]} frames.` ); } diff --git a/src/store/dicom-web/dicom-web-store.ts b/src/store/dicom-web/dicom-web-store.ts index acbd0c56d..254b648b6 100644 --- a/src/store/dicom-web/dicom-web-store.ts +++ b/src/store/dicom-web/dicom-web-store.ts @@ -202,7 +202,6 @@ export const useDicomWebStore = defineStore('dicom-web', () => { state: 'Done', }; } catch (error) { - console.error(error); const messageStore = useMessageStore(); messageStore.addError('Failed to load DICOM', error as Error); volumes.value[volumeKey] = { diff --git a/src/store/image-cache.ts b/src/store/image-cache.ts index b07d782f4..01e99e9e8 100644 --- a/src/store/image-cache.ts +++ b/src/store/image-cache.ts @@ -4,6 +4,7 @@ import { ProgressiveImageStatus, } from '@/src/core/progressiveImage'; import { useIdStore } from '@/src/store/id'; +import { useMessageStore } from '@/src/store/messages'; import { Maybe } from '@/src/types'; import { ImageMetadata } from '@/src/types/image'; import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; @@ -44,6 +45,9 @@ export const useImageCacheStore = defineStore('image-cache', () => { const onError = (error: Error) => { imageErrors[id] ??= []; imageErrors[id].push(error); + + const messageStore = useMessageStore(); + messageStore.addError('Error loading DICOM data', error); }; imageListenerCleanup[id] = () => { diff --git a/src/store/messages.ts b/src/store/messages.ts index a868e9817..12e166c1a 100644 --- a/src/store/messages.ts +++ b/src/store/messages.ts @@ -50,6 +50,8 @@ export const useMessageStore = defineStore('message', { * @param opts an Error, a string containing details, or a MessageOptions */ addError(title: string, opts?: Error | string | MessageOptions) { + console.error(title, opts); + if (opts instanceof Error) { return this._addMessage( { diff --git a/src/store/tools/paintProcess.ts b/src/store/tools/paintProcess.ts index e5cae1435..99d6ea9c3 100644 --- a/src/store/tools/paintProcess.ts +++ b/src/store/tools/paintProcess.ts @@ -142,7 +142,6 @@ export const usePaintProcessStore = defineStore('paintProcess', () => { showingOriginal: false, }; } catch (error) { - console.error(`${activeProcessType.value} Operation Failed:`, error); messageStore.addError( `${activeProcessType.value} Operation Failed`, error as Error diff --git a/src/utils/allocateImageFromChunks.ts b/src/utils/allocateImageFromChunks.ts index 029fde278..def96f3bb 100644 --- a/src/utils/allocateImageFromChunks.ts +++ b/src/utils/allocateImageFromChunks.ts @@ -99,7 +99,8 @@ export function allocateImageFromChunks(sortedChunks: Chunk[]) { ); } - // Some CT modality series still have NumberOfFrames === 1, so use the number of chunks if more than 1 + // We don't support volumes with multiple chunks/files with multi-frame data at the moment. + // Some CT modality series have NumberOfFrames === 1, so use the number of chunks if more than 1 chunk. const slices = sortedChunks.length > 1 ? sortedChunks.length : numberOfFrames ?? 1; const TypedArrayCtor = getTypedArrayConstructor(