Skip to content

Commit

Permalink
Return HTTP 400 on unrecognized task ID (#908)
Browse files Browse the repository at this point in the history
DAP-02's section on `/hpke_config` included a requirement that requests
for HPKE configs where the task ID is unrecognized should yield HTTP 404
Not Found. Our implementation returned 404 from *all* endpoints if the
request referenced an unrecognized task ID, which was not in compliance
with DAP-02, since the errors section says that HTTP 400 Bad Request
should be used "unless otherwise specified" (it was only otherwise
specified for `/hpke_config`).

DAP-03 improves this by removing the HTTP 404 Not Found requirement from
`/hpke_config`, so now we can uniformly return HTTP 400 Bad Request for
all `unrecognizedTask` errors, and in fact all errors since no protocol
text ever specifies otherwise.

This situation still isn't optimal, because the protocol shouldn't force
us to use Bad Request everywhere, but we will have to wait for future
DAP revisions to fix that.

Closes #705
  • Loading branch information
tgeoghegan authored Jan 12, 2023
1 parent f8f3e7c commit f1c4e87
Showing 1 changed file with 12 additions and 14 deletions.
26 changes: 12 additions & 14 deletions aggregator/src/aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2742,12 +2742,10 @@ impl DapProblemTypeExt for DapProblemType {
/// Returns the HTTP status code that should be used in responses whose body is a problem
/// document of this type.
fn http_status(&self) -> StatusCode {
match self {
// https://www.ietf.org/archive/id/draft-ietf-ppm-dap-02.html#section-4.3.1
Self::UnrecognizedTask => StatusCode::NOT_FOUND,
// So far, 400 Bad Request seems to be the appropriate choice for most problem types.
_ => StatusCode::BAD_REQUEST,
}
// Per the errors section of the protocol, error responses should use "HTTP status code 400
// Bad Request unless explicitly specified otherwise."
// https://datatracker.ietf.org/doc/html/draft-ietf-ppm-dap-03#name-errors
StatusCode::BAD_REQUEST
}
}

Expand Down Expand Up @@ -3415,13 +3413,13 @@ mod tests {
.into_response();
// Expected status and problem type should be per the protocol
// https://www.ietf.org/archive/id/draft-ietf-ppm-dap-02.html#section-4.3.1
assert_eq!(response.status(), StatusCode::NOT_FOUND);
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
let problem_details: serde_json::Value =
serde_json::from_slice(&body::to_bytes(response.body_mut()).await.unwrap()).unwrap();
assert_eq!(
problem_details,
json!({
"status": 404u16,
"status": 400u16,
"type": "urn:ietf:params:ppm:dap:error:unrecognizedTask",
"title": "An endpoint received a message with an unknown task ID.",
"taskid": format!("{unknown_task_id}"),
Expand Down Expand Up @@ -3852,7 +3850,7 @@ mod tests {
assert_eq!(
problem_details,
json!({
"status": 404,
"status": 400,
"type": "urn:ietf:params:ppm:dap:error:unrecognizedTask",
"title": "An endpoint received a message with an unknown task ID.",
"taskid": format!("{}", task.id()),
Expand Down Expand Up @@ -4146,7 +4144,7 @@ mod tests {
assert_eq!(
problem_details,
json!({
"status": 404,
"status": 400,
"type": "urn:ietf:params:ppm:dap:error:unrecognizedTask",
"title": "An endpoint received a message with an unknown task ID.",
"taskid": format!("{}", task.id()),
Expand Down Expand Up @@ -6444,13 +6442,13 @@ mod tests {
.into_response()
.into_parts();

assert_eq!(parts.status, StatusCode::NOT_FOUND);
assert_eq!(parts.status, StatusCode::BAD_REQUEST);
let problem_details: serde_json::Value =
serde_json::from_slice(&body::to_bytes(body).await.unwrap()).unwrap();
assert_eq!(
problem_details,
json!({
"status": StatusCode::NOT_FOUND.as_u16(),
"status": StatusCode::BAD_REQUEST.as_u16(),
"type": "urn:ietf:params:ppm:dap:error:unrecognizedTask",
"title": "An endpoint received a message with an unknown task ID.",
"taskid": format!("{}", task.id()),
Expand Down Expand Up @@ -7394,13 +7392,13 @@ mod tests {
.into_response()
.into_parts();

assert_eq!(parts.status, StatusCode::NOT_FOUND);
assert_eq!(parts.status, StatusCode::BAD_REQUEST);
let problem_details: serde_json::Value =
serde_json::from_slice(&body::to_bytes(body).await.unwrap()).unwrap();
assert_eq!(
problem_details,
json!({
"status": StatusCode::NOT_FOUND.as_u16(),
"status": StatusCode::BAD_REQUEST.as_u16(),
"type": "urn:ietf:params:ppm:dap:error:unrecognizedTask",
"title": "An endpoint received a message with an unknown task ID.",
"taskid": format!("{}", task.id()),
Expand Down

0 comments on commit f1c4e87

Please sign in to comment.