Skip to content

Commit

Permalink
Review updates
Browse files Browse the repository at this point in the history
  • Loading branch information
jcjones committed Feb 7, 2025
1 parent 78ecd3b commit 2af7c09
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 17 deletions.
3 changes: 0 additions & 3 deletions aggregator/src/aggregator/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,6 @@ pub enum Error {
ClientDisconnected,
#[error("too many requests")]
TooManyRequests,
#[error("unable to parse media-type")]
MediaTypeError(#[from] mime::FromStrError),
}

/// A newtype around `Arc<Error>`. This is needed to host a customized implementation of
Expand Down Expand Up @@ -319,7 +317,6 @@ impl Error {
Error::DifferentialPrivacy(_) => "differential_privacy",
Error::ClientDisconnected => "client_disconnected",
Error::TooManyRequests => "too_many_requests",
Error::MediaTypeError(_) => "media_type_error",
}
}
}
Expand Down
14 changes: 4 additions & 10 deletions aggregator/src/aggregator/http_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use janus_messages::{
AggregationJobInitializeReq, AggregationJobResp, CollectionJobId, CollectionJobReq,
CollectionJobResp, HpkeConfigList, Report, TaskId,
};
use mime::Mime;
use mime::{FromStrError, Mime};
use opentelemetry::{
metrics::{Counter, Meter},
KeyValue,
Expand Down Expand Up @@ -169,14 +169,6 @@ async fn run_error_handler(error: &Error, mut conn: Conn) -> Conn {
"again later."
)),
),
Error::MediaTypeError(_) => conn.with_problem_document(
&ProblemDocument::new(
"https://docs.divviup.org/references/janus-errors#content_type",
"Bad Request.",
Status::BadRequest,
)
.with_detail(concat!("The request included an invalid Content-Type.")),
),
};

if matches!(conn.status(), Some(status) if status.is_server_error()) {
Expand Down Expand Up @@ -730,7 +722,9 @@ fn validate_content_type(conn: &Conn, expected_media_type: &'static str) -> Resu
"invalid Content-Type header: {content_type}"
)))?;

let mime: Mime = mime_str.parse()?;
let mime: Mime = mime_str.parse().map_err(|e: FromStrError| {
Error::BadRequest(format!("failed to parse Content-Type header: {e}"))
})?;

if mime.essence_str() != expected_media_type {
return Err(Error::BadRequest(format!(
Expand Down
13 changes: 9 additions & 4 deletions collector/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ use janus_messages::{
AggregateShareAad, BatchSelector, CollectionJobId, CollectionJobReq, CollectionJobResp,
PartialBatchSelector, Query, Role, TaskId,
};
use mime::{FromStrError, Mime};
use mime::Mime;
use prio::{
codec::{Decode, Encode, ParameterizedDecode},
vdaf,
Expand Down Expand Up @@ -132,8 +132,8 @@ pub enum Error {
ReportCountOverflow,
#[error("message error: {0}")]
Message(#[from] janus_messages::Error),
#[error("unable to parse media-type")]
MediaTypeError(#[from] FromStrError),
#[error("the response from the server was invalid: {0}")]
BadResponse(String),
}

impl From<HttpErrorResponse> for Error {
Expand Down Expand Up @@ -573,7 +573,12 @@ impl<V: vdaf::Collector> Collector<V> {
.headers()
.get(CONTENT_TYPE)
.ok_or(Error::BadContentType(None))?;
let mime: Mime = content_type.to_str()?.parse()?;
let mime: Mime = content_type
.to_str()?
.parse()
.map_err(|e: mime::FromStrError| {
Error::BadResponse(format!("failed to parse Content-Type header: {e}"))
})?;
if mime.essence_str() != CollectionJobResp::<TimeInterval>::MEDIA_TYPE {
return Err(Error::BadContentType(Some(content_type.clone())));
}
Expand Down

0 comments on commit 2af7c09

Please sign in to comment.