Skip to content

Commit

Permalink
fix(wado-rs): return HTTP 404 for empty streams by peeking
Browse files Browse the repository at this point in the history
  • Loading branch information
nickamzol committed Jul 2, 2024
1 parent 6bf56d9 commit ddee7e3
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 24 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Add `secret-key-env` and `access-key-env` options to load secrets from environment variables.

### Fixed

- Return HTTP 404 "Not Found" for empty DICOM streams

## [0.2.0] - 2024-06-27

### Added
Expand Down
44 changes: 30 additions & 14 deletions src/api/wado/routes.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::error::Error;
use crate::api::wado::RetrieveInstanceRequest;
use crate::backend::dimse::wado::DicomMultipartStream;
use crate::backend::ServiceProvider;
use crate::types::UI;
use crate::AppState;
Expand All @@ -9,6 +9,8 @@ use axum::http::{Response, StatusCode};
use axum::response::IntoResponse;
use axum::routing::get;
use axum::Router;
use futures::{StreamExt, TryStreamExt};
use std::pin::Pin;
use tracing::{error, instrument};

/// HTTP Router for the Retrieve Transaction
Expand Down Expand Up @@ -59,24 +61,38 @@ async fn instance_resource(
let response = wado.retrieve(request).await;

match response {
Ok(response) => Response::builder()
.header(
CONTENT_DISPOSITION,
format!(r#"attachment; filename="{study_instance_uid}""#,),
)
.header(
CONTENT_TYPE,
r#"multipart/related; type="application/dicom"; boundary=boundary"#,
)
.body(Body::from_stream(response.stream))
.unwrap(),
Ok(response) => {
let mut stream = response.stream.peekable();
let pinned_stream = Pin::new(&mut stream);
if pinned_stream.peek().await.is_none() {
return StatusCode::NOT_FOUND.into_response();
}

Response::builder()
.header(
CONTENT_DISPOSITION,
format!(r#"attachment; filename="{study_instance_uid}""#,),
)
.header(
CONTENT_TYPE,
r#"multipart/related; type="application/dicom"; boundary=boundary"#,
)
.body(Body::from_stream(DicomMultipartStream::new(
stream.into_stream(),
)))
.unwrap()
}
Err(err) => {
error!("{err:?}");
(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response()
},
}
}
} else {
(StatusCode::SERVICE_UNAVAILABLE, "WADO-RS endpoint is disabled").into_response()
(
StatusCode::SERVICE_UNAVAILABLE,
"WADO-RS endpoint is disabled",
)
.into_response()
}
}

Expand Down
12 changes: 7 additions & 5 deletions src/api/wado/service.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
use crate::backend::dimse::cmove::movescu::MoveError;
use crate::types::{AE, UI};
use crate::AppState;
use async_trait::async_trait;
use axum::extract::rejection::{PathRejection, QueryRejection};
use axum::extract::{FromRef, FromRequestParts, Path, Query};
use axum::http::request::Parts;
use axum::response::{IntoResponse, Response};
use dicom::object::{FileDicomObject, InMemDicomObject};
use futures::stream::BoxStream;
use serde::{Deserialize, Serialize};
use std::fmt::Debug;
use std::num::ParseIntError;
use std::str::FromStr;
use std::sync::Arc;
use thiserror::Error;

use crate::backend::dimse::wado::DicomMultipartStream;
use crate::types::{AE, UI};
use crate::AppState;

#[async_trait]
pub trait WadoService: Send + Sync {
async fn retrieve(
Expand Down Expand Up @@ -63,7 +65,7 @@ where
}

pub struct InstanceResponse {
pub stream: DicomMultipartStream<'static>,
pub stream: BoxStream<'static, Result<Arc<FileDicomObject<InMemDicomObject>>, MoveError>>,
}

#[derive(Debug, Deserialize)]
Expand Down
8 changes: 5 additions & 3 deletions src/backend/dimse/wado.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ impl WadoService for DimseWadoService {
)
.await;

Ok(InstanceResponse { stream })
Ok(InstanceResponse {
stream: stream.boxed(),
})
}
}

Expand Down Expand Up @@ -122,7 +124,7 @@ impl DimseWadoService {
aet: &str,
storescp_aet: &str,
identifier: InMemDicomObject,
) -> DicomMultipartStream<'static> {
) -> BoxStream<'static, Result<Arc<FileDicomObject<InMemDicomObject>>, MoveError>> {
let message_id = next_message_id();
let (tx, mut rx) = mpsc::channel::<Result<MoveSubOperation, MoveError>>(1);

Expand Down Expand Up @@ -174,7 +176,7 @@ impl DimseWadoService {
}
};

DicomMultipartStream::new(DropStream::new(rx_stream, subscription))
DropStream::new(rx_stream, subscription).boxed()
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/backend/s3/wado.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::api::wado::{InstanceResponse, RetrieveError, RetrieveInstanceRequest, WadoService};
use crate::backend::dimse::cmove::movescu::MoveError;
use crate::backend::dimse::wado::DicomMultipartStream;
use crate::config::{S3Config, S3EndpointStyle};
use async_trait::async_trait;
use aws_config::retry::RetryConfig;
Expand Down Expand Up @@ -99,6 +98,7 @@ impl WadoService for S3WadoService {
.await
.unwrap();

// TODO: Do not unwrap
let bytes = object.body.collect().await.unwrap().reader();
Result::<_, MoveError>::Ok(Arc::new(
FileDicomObject::from_reader(bytes).unwrap(),
Expand All @@ -112,7 +112,7 @@ impl WadoService for S3WadoService {
});

Ok(InstanceResponse {
stream: DicomMultipartStream::new(stream),
stream: stream.boxed(),
})
}
}

0 comments on commit ddee7e3

Please sign in to comment.