Skip to content

Conversation

@jose-rfj
Copy link
Contributor

@jose-rfj jose-rfj commented Apr 1, 2024

First, I included the tag "model" in the dict "label_info" because I needed the name of the model that generated the label to be further saved.
Then, I included a checkBox in the plugin settings to enable dicom export between server and client. This feature is important because some models can use metadata included in the dicom header, such as slice thickness.
Finally, I added a function call to enable image upload right after segmentation is called, due to the same problem previously described.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added DICOM file support with optional toggle in settings
    • Enabled bidirectional DICOM file exchange between client and server
  • Improvements

    • Enhanced upload workflow with automatic image uploading before segmentation
    • Improved progress reporting for multi-step file transfers

✏️ Tip: You can customize this high-level summary in your review settings.

@SachidanandAlle
Copy link
Collaborator

why docs build failing? otherwise changes r ok
@diazandr3s

@SachidanandAlle
Copy link
Collaborator

can @diazandr3s or @tangy5 run the plugin with these changes.. and see if functionality is working...
also please share some snapshots for this new feature

@YanxuanLiu
Copy link
Collaborator

/build

@github-actions
Copy link

👎 Promotion blocked, new vulnerability found

Vulnerability report

Component Vulnerability Description Severity
Apache Ivy CVE-2022-46751 Improper Restriction of XML External Entity Reference, XML Injection (aka Blind XPath Injection) vulnerability in Apache Software Foundation Apache Ivy.This issue affects any version of Apache Ivy prior to 2.5.2.

When Apache Ivy prior to 2.5.2 parses XML files - either its own configuration, Ivy files or Apache Maven POMs - it will allow downloading external document type definitions and expand any entity references contained therein when used.

This can be used to exfiltrate data, access resources only the machine running Ivy has access to or disturb the execution of Ivy in different ways.

Starting with Ivy 2.5.2 DTD processing is disabled by default except when parsing Maven POMs where the default is to allow DTD processing but only to include a DTD snippet shipping with Ivy that is needed to deal with existing Maven POMs that are not valid XML files but are nevertheless accepted by Maven. Access can be be made more lenient via newly introduced system properties where needed.

Users of Ivy prior to version 2.5.2 can use Java system properties to restrict processing of external DTDs, see the section about "JAXP Properties for External Access restrictions" inside Oracle's "Java API for XML Processing (JAXP) Security Guide".
|HIGH
Netty Project|CVE-2023-34462|Netty is an asynchronous event-driven network application framework for rapid development of maintainable high performance protocol servers & clients. The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap. The SniHandler class is a handler that waits for the TLS handshake to configure a SslHandler according to the indicated server name by the ClientHello record. For this matter it allocates a ByteBuf using the value defined in the ClientHello record. Normally the value of the packet should be smaller than the handshake packet but there are not checks done here and the way the code is written, it is possible to craft a packet that makes the SslClientHelloHandler. This vulnerability has been fixed in version 4.1.94.Final.|MEDIUM
Netty Project|CVE-2023-44487|The HTTP/2 protocol allows a denial of service (server resource consumption) because request cancellation can reset many streams quickly, as exploited in the wild in August through October 2023.|HIGH

@diazandr3s
Copy link
Collaborator

Hi @jose-rfj,

Sorry i'm just catching up on comments/PRs. Many thanks for your contribution.

I just tested your branch and recorded this video for reference:

dicom-files.mp4

My question is how could we manage the DICOM files stored in the JSON file? As you see in the video, the MONAI Label updates the JSON file and considers each dcm file as a new volume.

Additionally, it stores the converted nii.gz file for the DICOM itself.

Regarding this:

This feature is important because some models can use metadata included in the dicom header, such as slice thickness.

Do you think NRRD firle format will help? What do you think @SachidanandAlle?

Please let us know,

@jose-rfj
Copy link
Contributor Author

jose-rfj commented Jul 5, 2024

Dear @diazandr3s

Thanks for reaching out. I actually updated locally the plugin to store only the first slice, as all of them store the same header, so only one is needed. How do I update that in here? Do I have to create another PR?

Cheers,
Raniery

Hi @jose-rfj,

Sorry i'm just catching up on comments/PRs. Many thanks for your contribution.

I just tested your branch and recorded this video for reference:

dicom-files.mp4
My question is how could we manage the DICOM files stored in the JSON file? As you see in the video, the MONAI Label updates the JSON file and considers each dcm file as a new volume.

Additionally, it stores the converted nii.gz file for the DICOM itself.

Regarding this:

This feature is important because some models can use metadata included in the dicom header, such as slice thickness.

Do you think NRRD firle format will help? What do you think @SachidanandAlle?

Please let us know,

@diazandr3s
Copy link
Collaborator

Dear @diazandr3s

Thanks for reaching out. I actually updated locally the plugin to store only the first slice, as all of them store the same header, so only one is needed. How do I update that in here? Do I have to create another PR?

Cheers, Raniery

Hi @jose-rfj,
Sorry i'm just catching up on comments/PRs. Many thanks for your contribution.
I just tested your branch and recorded this video for reference:
dicom-files.mp4
My question is how could we manage the DICOM files stored in the JSON file? As you see in the video, the MONAI Label updates the JSON file and considers each dcm file as a new volume.
Additionally, it stores the converted nii.gz file for the DICOM itself.
Regarding this:

This feature is important because some models can use metadata included in the dicom header, such as slice thickness.

Do you think NRRD firle format will help? What do you think @SachidanandAlle?
Please let us know,

Thanks @jose-rfj.
I guess you can update this PR. No need to create another one.
Let us know,

@SachidanandAlle SachidanandAlle merged commit 058ddfc into Project-MONAI:main Dec 24, 2025
1 of 3 checks passed
@coderabbitai
Copy link

coderabbitai bot commented Dec 24, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Added DICOM file handling integration to MONAILabel plugin, including imports for DICOM modules, a new UI checkbox setting to enable DICOM file inclusion, server-side DICOM export functionality upon image upload, client-side DICOM file collection and upload workflow, and updates to label-saving and segmentation flows to include model information and ensure proper image upload sequencing.

Changes

Cohort / File(s) Summary
DICOM Integration & Imports
plugins/slicer/MONAILabel/MONAILabel.py
Added imports for DICOMScalarVolumePlugin and pydicom.dcmread to enable DICOM file handling on both client and server sides.
UI Settings
plugins/slicer/MONAILabel/MONAILabel.py
Introduced new "Include DICOM files" checkbox in MONAILabel settings with tooltip and property binding to MONAILabel/includeDicomFiles configuration.
DICOM Upload & Export Workflow
plugins/slicer/MONAILabel/MONAILabel.py
Implemented server-side DICOM export (via DICOMScalarVolumePlugin) when uploading images with the setting enabled. Added client-side DICOM file collection using new get_dicom_files() method, uploads first DICOM file to reduce transfer overhead, and updates progress tracking via last_report_progress.
Label Saving & Segmentation Flow
plugins/slicer/MONAILabel/MONAILabel.py
Updated save_label() calls to include model name in payload ({"label_info": label_info, "model": model}). Modified onClickSegmentation() to invoke onUploadImage() before proceeding if an active session exists.
Utility Methods
plugins/slicer/MONAILabel/MONAILabel.py
Added new public method get_dicom_files() to recursively collect .dcm files from temporary directory for client-side DICOM transfer.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant FileSystem as File System

    rect rgb(240, 248, 255)
    Note over Client,Server: Image Upload with DICOM Export
    Client->>Server: Upload image (Include DICOM enabled)
    Server->>FileSystem: Export original DICOM files<br/>(DICOMScalarVolumePlugin)
    Server->>Server: Advance progress
    Server-->>Client: Upload complete
    end

    rect rgb(240, 248, 255)
    Note over Client,FileSystem: Client-Side DICOM Collection & Upload
    Client->>FileSystem: Collect local .dcm files<br/>(get_dicom_files)
    FileSystem-->>Client: Return DICOM file list
    Client->>Client: Take first DICOM file<br/>(mitigate transfer overhead)
    Client->>Server: Upload DICOM file
    Client->>Client: Update progress (last_report_progress)
    Server-->>Client: DICOM received
    end

    rect rgb(240, 248, 255)
    Note over Client,Server: Segmentation with Model Info
    Client->>Server: Trigger segmentation
    Server->>Server: Process with model name
    Server-->>Client: Segmentation result
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hoppy new leap in DICOM's embrace,
Where files find their place in the digital space,
From client to server, a dance of progress precise,
With checkboxes checked, the integration's so nice!
Models named, uploads staged, the workflow's complete—
DICOM integration makes MONAILabel neat! 🎉

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between a786e6b and 9f4bf3e.

📒 Files selected for processing (1)
  • plugins/slicer/MONAILabel/MONAILabel.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants