diff --git a/README.md b/README.md index 73c00b6a..d8167aca 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,9 @@ The configuration file defines: - The endpoints used to upload the anonymised DICOM data and the public and radiology [parquet files](./docs/file_types/parquet_files.md). We currently support the following endpoints: - `"none"`: no upload - - `"ftps"`: a secure FTP server (for both _DICOM_ and _parquet_ files) + - `"ftps"`: a secure FTP server using implicit TLS (for both _DICOM_ and _parquet_ files), e.g. UCL DSH + - `"ftpes"`: a secure FTP server using explicit TLS (for both _DICOM_ and _parquet_ files) + - `"dicomweb"`: a DICOMweb server (for _DICOM_ files only). Requires the `DICOMWEB_*` environment variables to be set in `.env` - `"xnat"`: an [XNAT](https://www.xnat.org/) instance (for _DICOM_ files only) diff --git a/pixl_core/src/core/project_config/pixl_config_model.py b/pixl_core/src/core/project_config/pixl_config_model.py index f52c3584..c4b31d77 100644 --- a/pixl_core/src/core/project_config/pixl_config_model.py +++ b/pixl_core/src/core/project_config/pixl_config_model.py @@ -146,6 +146,7 @@ class _DestinationEnum(enum.StrEnum): none = "none" ftps = "ftps" + ftpes = "ftpes" dicomweb = "dicomweb" xnat = "xnat" tre = "tre" diff --git a/pixl_core/src/core/uploader/__init__.py b/pixl_core/src/core/uploader/__init__.py index b244ebdc..b5fc58ad 100644 --- a/pixl_core/src/core/uploader/__init__.py +++ b/pixl_core/src/core/uploader/__init__.py @@ -28,7 +28,7 @@ from core.project_config import load_project_config from ._dicomweb import DicomWebUploader -from ._ftps import FTPSUploader +from ._ftps import FTPESUploader, FTPSUploader from ._treapi import TreApiUploader from ._xnat import XNATUploader @@ -41,6 +41,7 @@ def get_uploader(project_slug: str) -> Uploader: """Uploader Factory, returns uploader instance based on destination.""" choices: dict[str, type[Uploader]] = { "ftps": FTPSUploader, + "ftpes": FTPESUploader, "dicomweb": DicomWebUploader, "xnat": XNATUploader, "tre": TreApiUploader, diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 1fa91b7e..652772db 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -38,6 +38,7 @@ class ImplicitFtpTls(ftplib.FTP_TLS): """ FTP_TLS subclass that automatically wraps sockets in SSL to support implicit FTPS. + Use explicit TLS where possible. https://stackoverflow.com/questions/12164470/python-ftp-implicit-tls-connection-issue """ @@ -63,6 +64,8 @@ def sock(self, value: socket | None) -> None: class FTPSUploader(Uploader): """Upload strategy for an FTPS server.""" + ftp_tls_class: type[ftplib.FTP_TLS] = ImplicitFtpTls + def __init__(self, project_slug: str, keyvault_alias: str | None) -> None: """Create instance of parent class""" super().__init__(project_slug, keyvault_alias) @@ -95,7 +98,7 @@ def send_via_ftps( ) -> None: """Send the zip content to the FTPS server.""" # Create the remote directory if it doesn't exist - ftp = _connect_to_ftp(self.host, self.port, self.user, self.password) + ftp = _connect_to_ftp(self.host, self.port, self.user, self.password, self.ftp_tls_class) _create_and_set_as_cwd(ftp, remote_directory) command = f"STOR {pseudo_anon_image_id}.zip" logger.debug("Running {}", command) @@ -133,7 +136,7 @@ def upload_parquet_files(self, parquet_export: ParquetExport) -> None: source_root_dir = parquet_export.current_extract_base # Create the remote directory if it doesn't exist - ftp = _connect_to_ftp(self.host, self.port, self.user, self.password) + ftp = _connect_to_ftp(self.host, self.port, self.user, self.password, self.ftp_tls_class) _create_and_set_as_cwd(ftp, parquet_export.project_slug) _create_and_set_as_cwd(ftp, parquet_export.extract_time_slug) _create_and_set_as_cwd(ftp, "parquet") @@ -168,10 +171,22 @@ def upload_parquet_files(self, parquet_export: ParquetExport) -> None: logger.info("Finished FTPS upload of files for '{}'", parquet_export.project_slug) -def _connect_to_ftp(ftp_host: str, ftp_port: int, ftp_user: str, ftp_password: str) -> FTP_TLS: +class FTPESUploader(FTPSUploader): + """Upload strategy for an FTPES server (explicit rather than implicit TLS).""" + + ftp_tls_class = ftplib.FTP_TLS + + +def _connect_to_ftp( + ftp_host: str, + ftp_port: int, + ftp_user: str, + ftp_password: str, + ftp_tls_class: type[ftplib.FTP_TLS], +) -> FTP_TLS: # Connect to the server and login try: - ftp = ImplicitFtpTls() + ftp = ftp_tls_class() ftp.connect(ftp_host, int(ftp_port)) ftp.login(ftp_user, ftp_password) ftp.prot_p() diff --git a/pixl_dcmd/src/pixl_dcmd/main.py b/pixl_dcmd/src/pixl_dcmd/main.py index 95b73c92..8fbbfad3 100644 --- a/pixl_dcmd/src/pixl_dcmd/main.py +++ b/pixl_dcmd/src/pixl_dcmd/main.py @@ -262,7 +262,11 @@ def _clean_dicom_image_pixels( burned_pixels = has_burned_pixels(dataset, deid=deid_recipe) cleaned_pixels = clean_pixel_data(dicom_file=dataset, results=burned_pixels) - dataset.PixelData = cleaned_pixels.tobytes() + pixel_bytes = cleaned_pixels.tobytes() + if dataset.file_meta.TransferSyntaxUID.is_compressed: + dataset.PixelData = pydicom.encaps.encapsulate([pixel_bytes]) + else: + dataset.PixelData = pixel_bytes def _anonymise_dicom_from_scheme( diff --git a/projects/configs/tag-operations/us.yaml b/projects/configs/tag-operations/us.yaml index f755dfbc..77a66c89 100644 --- a/projects/configs/tag-operations/us.yaml +++ b/projects/configs/tag-operations/us.yaml @@ -23,4 +23,48 @@ - name: Sequence of Ultrasound Regions group: 0x0018 element: 0x6011 - op: keep \ No newline at end of file + op: keep +- name: Region Spatial Format + group: 0x0018 + element: 0x6012 + op: keep +- name: Region Data Type + group: 0x0018 + element: 0x6014 + op: keep +- name: Region Flags + group: 0x0018 + element: 0x6016 + op: keep +- name: Region Location Min X0 + group: 0x0018 + element: 0x6018 + op: keep +- name: Region Location Min Y0 + group: 0x0018 + element: 0x601A + op: keep +- name: Region Location Max X1 + group: 0x0018 + element: 0x601C + op: keep +- name: Region Location Max Y1 + group: 0x0018 + element: 0x601E + op: keep +- name: Physical Units X Direction + group: 0x0018 + element: 0x6024 + op: keep +- name: Physical Units Y Direction + group: 0x0018 + element: 0x6026 + op: keep +- name: Physical Delta X + group: 0x0018 + element: 0x602C + op: keep +- name: Physical Delta Y + group: 0x0018 + element: 0x602E + op: keep diff --git a/projects/configs/ultrasound-learning-for-timely-rapid-assessment-of-cts.yaml b/projects/configs/ultrasound-learning-for-timely-rapid-assessment-of-cts.yaml index 883b52c2..d43ca880 100644 --- a/projects/configs/ultrasound-learning-for-timely-rapid-assessment-of-cts.yaml +++ b/projects/configs/ultrasound-learning-for-timely-rapid-assessment-of-cts.yaml @@ -39,5 +39,5 @@ series_filters: - "positioning" destination: - dicom: "ftps" - parquet: "ftps" + dicom: "ftpes" + parquet: "ftpes"