fix(cornerstone): guard against non-finite slice counts crashing the viewer - #6187
Open
awatson1978 wants to merge 1 commit into
Open
fix(cornerstone): guard against non-finite slice counts crashing the viewer#6187awatson1978 wants to merge 1 commit into
awatson1978 wants to merge 1 commit into
Conversation
…viewer viewport.getNumberOfSlices() returns Infinity when the volume's spacing in the camera's normal direction degenerates to 0, which happens with some SEG-derived volumes in the segmentation mode's multi-viewport layouts. The slice progress scrollbar fed that count straight into new Uint8Array(size), throwing 'RangeError: Invalid typed array length: Infinity' and blanking the whole viewer behind the route error boundary. Clamp the count to a finite non-negative value in getViewportSliceCount, route the slice-event path through the same guard, and defensively sanitize the size in useByteArray so no caller can crash the allocation. Fixes OHIF#6174 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughViewport slice counts are sanitized for invalid values, navigation consumes the sanitized count, and SmartScrollbar byte-array sizes are normalized before allocation. ChangesSlice and byte-array size safety
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
awatson1978
marked this pull request as ready for review
July 27, 2026 20:28
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform/ui-next/src/components/SmartScrollbar/useByteArray.ts`:
- Around line 25-28: Update the shared getViewportSliceCount flow to clamp or
reject finite positive slice counts above the supported allocation limit before
useByteArray receives them. Preserve the existing handling for non-finite and
non-positive counts, and ensure useByteArray’s Uint8Array allocation only
receives bounded values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 34fb40c9-310f-474f-a454-bb07f83247b1
📒 Files selected for processing (3)
extensions/cornerstone/src/Viewport/Overlays/ViewportSliceProgressScrollbar/hooks.tsextensions/cornerstone/src/utils/viewportDataShape.tsplatform/ui-next/src/components/SmartScrollbar/useByteArray.ts
Comment on lines
+25
to
28
| // A non-finite or negative size (e.g. a degenerate volume reporting | ||
| // Infinity slices) must not crash the Uint8Array allocation. | ||
| size = Number.isFinite(size) && size > 0 ? Math.floor(size) : 0; | ||
| const bytesRef = useRef(new Uint8Array(size)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
node - <<'NODE'
for (const size of [2 ** 32, Number.MAX_SAFE_INTEGER]) {
try {
new Uint8Array(size);
console.log(size, 'accepted');
} catch (error) {
console.log(size, error.name);
}
}
NODERepository: OHIF/Viewers
Length of output: 198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate file and related symbols =="
fd -a 'useByteArray\.ts$|SmartScrollbar' . | sed 's#^\./##' | head -50
echo
echo "== useByteArray.ts outline =="
ast-grep outline platform/ui-next/src/components/SmartScrollbar/useByteArray.ts --view compact || true
echo
echo "== useByteArray.ts relevant contents =="
cat -n platform/ui-next/src/components/SmartScrollbar/useByteArray.ts
echo
echo "== Search getViewportSliceCount usages =="
rg -n "getViewportSliceCount|useByteArray|Uint8Array" platform/ui-next/src -S
echo
echo "== Locate getViewportSliceCount definition =="
rg -n "function getViewportSliceCount|const [A-Za-z0-9_]+ getViewportSliceCount|getViewportSliceCount =|getViewportSliceCount\\(" -S .Repository: OHIF/Viewers
Length of output: 10267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== viewportDataShape.ts relevant contents =="
sed -n '1,130p' extensions/cornerstone/src/utils/viewportDataShape.ts | cat -n
echo
echo "== hook call sites and surrounding logic =="
sed -n '1,220p' extensions/cornerstone/src/Viewport/Overlays/ViewportSliceProgressScrollbar/hooks.ts | cat -n
echo
echo "== ImageScrollbar call site =="
sed -n '1,110p' extensions/cornerstone/src/Viewport/Overlays/ViewportImageScrollbar.tsx | cat -n
echo
echo "== getViewportSliceCount call sites =="
rg -n -C 4 "getViewportSliceCount\\(" .Repository: OHIF/Viewers
Length of output: 22594
Cap slice counts before allocating the byte array.
useByteArray only floors the value before calling Uint8Array, while getViewportSliceCount rejects only non-finite/non-positive counts and otherwise returns any finite positive value. A huge slice count can still cause allocation failure; clamp/reject oversized slice counts at the shared source instead of relying on Uint8Array allocation to fail later.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platform/ui-next/src/components/SmartScrollbar/useByteArray.ts` around lines
25 - 28, Update the shared getViewportSliceCount flow to clamp or reject finite
positive slice counts above the supported allocation limit before useByteArray
receives them. Preserve the existing handling for non-finite and non-positive
counts, and ensure useByteArray’s Uint8Array allocation only receives bounded
values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Fixes #6174
Open the segmentation mode with the study from the issue (TCIA
…285242291560760827564488897577), load or create a segmentation while the default 2×2 layout is active, and the whole viewer goes blank with:The
isFullModeframe in the minified stack pointed at the slice progress scrollbar, and that's exactly where it is — this turns out to be an OHIF-side crash, not a cornerstone3D one.Changes & Results
Why this happens: cornerstone's
viewport.getNumberOfSlices()computes the count as(max − min) / spacingInNormalDirection + 1. For some SEG-derived volumes in the segmentation mode's multi-viewport layouts, the spacing in the camera's normal direction comes back as 0, so the division yields Infinity. TheViewportSliceProgressScrollbarfeeds that number straight intonew Uint8Array(size)(via theuseByteArrayhook that tracks per-slice loaded/viewed state), which throws, and the route error boundary blanks the entire app.The fix — guard at three layers so no path can feed a non-finite count into an allocation:
getViewportSliceCount(extensions/cornerstone/src/utils/viewportDataShape.ts) clamps non-finite or negative counts to 0hooks.ts) now routes through that same guarded helper instead of callinggetNumberOfSlices()rawuseByteArray(platform/ui-next) defensively sanitizes itssizeso no other consumer can crash theUint8Arrayallocation eitherWith the guard, the degenerate viewport simply renders without a slice scrollbar instead of taking down the viewer. The underlying "why is the spacing 0 for this volume" question belongs upstream in cornerstone3D —
getNumberOfSlices()arguably should never return Infinity — and I'm happy to file that issue with the captured details as a follow-up.Before (blank app after the RangeError) / After (SEG loads in the 2×2 layout, segments hydrate normally):
Testing
pnpm dev→ open/segmentation?StudyInstanceUIDs=1.3.6.1.4.1.14519.5.2.1.3098.5025.285242291560760827564488897577Reproduced the crash on unpatched master and verified the fix in Chromium via Playwright (page-error listener attached — no errors after the fix).
Screenshots
6174-before-crash-blank-app
6174-after-no-crash
6174-after-seg-loaded-no-crash
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
Tested Environment
🤖 Generated with Claude Code
Summary by CodeRabbit