Skip to content

Commit

Permalink
feat(ic-http-gateway): return internal error in response metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
@nathanosdev
nathanosdev committed May 31, 2024
1 parent 5852982 commit d4cb9e4
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 69 deletions.
3 changes: 3 additions & 0 deletions packages/ic-http-gateway/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ pub type HttpGatewayResult<T = ()> = Result<T, HttpGatewayError>;
/// HTTP gateway error type.
#[derive(thiserror::Error, Debug)]
pub enum HttpGatewayError {
#[error(r#"Response verification error: "{0}""#)]
ResponseVerificationError(#[from] ic_response_verification::ResponseVerificationError),

/// Inner error from agent.
#[error(r#"Agent error: "{0}""#)]
AgentError(#[from] ic_agent::AgentError),
Expand Down
113 changes: 66 additions & 47 deletions packages/ic-http-gateway/src/protocol/handler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,17 @@ pub async fn process_request(
let agent_response = match query_result {
Ok((response,)) => response,
Err(e) => {
let err_res = handle_agent_error(e)?;

return Ok(HttpGatewayResponse {
canister_response: err_res,
metadata: HttpGatewayResponseMetadata {
upgraded_to_update_call: false,
response_verification_version: None,
},
});
return match handle_agent_error(&e) {
None => Err(e.into()),
Some(err_res) => Ok(HttpGatewayResponse {
canister_response: err_res,
metadata: HttpGatewayResponseMetadata {
upgraded_to_update_call: true,
response_verification_version: None,
internal_error: Some(e.into()),
},
}),
}
}
};

Expand All @@ -118,15 +120,17 @@ pub async fn process_request(
match update_result {
Ok((response,)) => response,
Err(e) => {
let err_res = handle_agent_error(e)?;

return Ok(HttpGatewayResponse {
canister_response: err_res,
metadata: HttpGatewayResponseMetadata {
upgraded_to_update_call: true,
response_verification_version: None,
},
});
return match handle_agent_error(&e) {
None => Err(e.into()),
Some(err_res) => Ok(HttpGatewayResponse {
canister_response: err_res,
metadata: HttpGatewayResponseMetadata {
upgraded_to_update_call: true,
response_verification_version: None,
internal_error: Some(e.into()),
},
}),
}
}
}
} else {
Expand Down Expand Up @@ -164,11 +168,14 @@ pub async fn process_request(
return Ok(HttpGatewayResponse {
canister_response: Response::builder()
.status(StatusCode::INTERNAL_SERVER_ERROR)
.body(HttpGatewayResponseBody::Bytes(err.as_bytes().to_vec()))
.body(HttpGatewayResponseBody::Bytes(
err.to_string().as_bytes().to_vec(),
))
.unwrap(),
metadata: HttpGatewayResponseMetadata {
upgraded_to_update_call: is_update_call,
response_verification_version: None,
internal_error: Some(err),
},
});
}
Expand Down Expand Up @@ -210,6 +217,7 @@ pub async fn process_request(
response_verification_version: Some(
validation_info.verification_version,
),
internal_error: None,
},
});
}
Expand Down Expand Up @@ -249,23 +257,26 @@ pub async fn process_request(
metadata: HttpGatewayResponseMetadata {
upgraded_to_update_call: is_update_call,
response_verification_version: validation_info.map(|e| e.verification_version),
internal_error: None,
},
})
}

fn handle_agent_error(error: AgentError) -> HttpGatewayResult<CanisterResponse> {
fn handle_agent_error(error: &AgentError) -> Option<CanisterResponse> {
match error {
// Turn all `DestinationInvalid`s into 404
AgentError::CertifiedReject(RejectResponse {
reject_code: RejectCode::DestinationInvalid,
reject_message,
..
}) => Ok(Response::builder()
.status(StatusCode::NOT_FOUND)
.body(HttpGatewayResponseBody::Bytes(
reject_message.as_bytes().to_vec(),
))
.unwrap()),
}) => Some(
Response::builder()
.status(StatusCode::NOT_FOUND)
.body(HttpGatewayResponseBody::Bytes(
reject_message.as_bytes().to_vec(),
))
.unwrap(),
),

// If the result is a Replica error, returns the 500 code and message. There is no information
// leak here because a user could use `dfx` to get the same reply.
Expand All @@ -275,22 +286,26 @@ fn handle_agent_error(error: AgentError) -> HttpGatewayResult<CanisterResponse>
response.reject_code, response.reject_message, response.error_code,
);

Ok(Response::builder()
.status(StatusCode::BAD_GATEWAY)
.body(HttpGatewayResponseBody::Bytes(msg.as_bytes().to_vec()))
.unwrap())
Some(
Response::builder()
.status(StatusCode::BAD_GATEWAY)
.body(HttpGatewayResponseBody::Bytes(msg.as_bytes().to_vec()))
.unwrap(),
)
}

AgentError::UncertifiedReject(RejectResponse {
reject_code: RejectCode::DestinationInvalid,
reject_message,
..
}) => Ok(Response::builder()
.status(StatusCode::NOT_FOUND)
.body(HttpGatewayResponseBody::Bytes(
reject_message.as_bytes().to_vec(),
))
.unwrap()),
}) => Some(
Response::builder()
.status(StatusCode::NOT_FOUND)
.body(HttpGatewayResponseBody::Bytes(
reject_message.as_bytes().to_vec(),
))
.unwrap(),
),

// If the result is a Replica error, returns the 500 code and message. There is no information
// leak here because a user could use `dfx` to get the same reply.
Expand All @@ -300,21 +315,25 @@ fn handle_agent_error(error: AgentError) -> HttpGatewayResult<CanisterResponse>
response.reject_code, response.reject_message, response.error_code,
);

Ok(Response::builder()
.status(StatusCode::BAD_GATEWAY)
.body(HttpGatewayResponseBody::Bytes(msg.as_bytes().to_vec()))
.unwrap())
Some(
Response::builder()
.status(StatusCode::BAD_GATEWAY)
.body(HttpGatewayResponseBody::Bytes(msg.as_bytes().to_vec()))
.unwrap(),
)
}

AgentError::ResponseSizeExceededLimit() => Ok(Response::builder()
.status(StatusCode::INSUFFICIENT_STORAGE)
.body(HttpGatewayResponseBody::Bytes(
b"Response size exceeds limit".to_vec(),
))
.unwrap()),
AgentError::ResponseSizeExceededLimit() => Some(
Response::builder()
.status(StatusCode::INSUFFICIENT_STORAGE)
.body(HttpGatewayResponseBody::Bytes(
b"Response size exceeds limit".to_vec(),
))
.unwrap(),
),

// Handle all other errors
e => Err(e.into()),
_ => None,
}
}

Expand Down
12 changes: 4 additions & 8 deletions packages/ic-http-gateway/src/protocol/validate.rs
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
use crate::IC_CERTIFICATE_HEADER_NAME;
use crate::{HttpGatewayResult, IC_CERTIFICATE_HEADER_NAME};
use candid::Principal;
use ic_agent::Agent;
use ic_http_certification::{HttpRequest, HttpResponse};
use ic_response_verification::{
types::VerificationInfo, verify_request_response_pair, MIN_VERIFICATION_VERSION,
};
use std::{
borrow::Cow,
time::{SystemTime, UNIX_EPOCH},
};
use std::time::{SystemTime, UNIX_EPOCH};

const MAX_CERT_TIME_OFFSET_NS: u128 = 300_000_000_000;

Expand All @@ -18,7 +15,7 @@ pub fn validate(
request: HttpRequest,
response: HttpResponse,
allow_skip_verification: bool,
) -> Result<Option<VerificationInfo>, Cow<'static, str>> {
) -> HttpGatewayResult<Option<VerificationInfo>> {
match (allow_skip_verification, has_ic_certificate(&response)) {
// TODO: Remove this (FOLLOW-483)
// Canisters don't have to provide certified variables
Expand All @@ -34,8 +31,7 @@ pub fn validate(
MAX_CERT_TIME_OFFSET_NS,
ic_public_key.as_slice(),
MIN_VERIFICATION_VERSION,
)
.map_err(|_| "Body does not pass verification")?;
)?;
Ok(Some(verification_info))
}
}
Expand Down
29 changes: 17 additions & 12 deletions packages/ic-http-gateway/src/response/http_gateway_response.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use std::{
task::{Context, Poll},
};

use crate::HttpGatewayError;

pub type CanisterResponse = Response<HttpGatewayResponseBody>;

/// A response from the HTTP gateway.
Expand All @@ -22,6 +24,21 @@ pub struct HttpGatewayResponse {
pub metadata: HttpGatewayResponseMetadata,
}

/// Additional metadata regarding the response.
#[derive(Debug)]
pub struct HttpGatewayResponseMetadata {
/// Whether the original query call was upgraded to an update call.
pub upgraded_to_update_call: bool,

/// The version of response verification that was used to verify the response.
/// If the protocol fails before getting to the verification step, or the
/// original query call is upgraded to an update call, this field will be `None`.
pub response_verification_version: Option<u16>,

/// The internal error that resulted in the HTTP response being an error response.
pub internal_error: Option<HttpGatewayError>,
}

/// The body of an HTTP gateway response.
#[derive(Debug)]
pub enum HttpGatewayResponseBody {
Expand Down Expand Up @@ -106,15 +123,3 @@ impl Stream for ResponseBodyStream {
self.inner.as_mut().poll_next(cx)
}
}

/// Additional metadata regarding the response.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct HttpGatewayResponseMetadata {
/// Whether the original query call was upgraded to an update call.
pub upgraded_to_update_call: bool,

/// The version of response verification that was used to verify the response.
/// If the protocol fails before getting to the verification step, or the
/// original query call is upgraded to an update call, this field will be `None`.
pub response_verification_version: Option<u16>,
}
20 changes: 18 additions & 2 deletions packages/ic-http-gateway/tests/custom_assets.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,27 @@ fn test_custom_assets_index_html() {
response.canister_response.body(),
HttpGatewayResponseBody::Bytes(body) if body == index_html
);
assert_eq!(

assert_response_metadata(
response.metadata,
HttpGatewayResponseMetadata {
upgraded_to_update_call: false,
response_verification_version: Some(2),
}
internal_error: None,
},
);
}

fn assert_response_metadata(
response_metadata: HttpGatewayResponseMetadata,
expected_response_metadata: HttpGatewayResponseMetadata,
) {
assert_eq!(
response_metadata.upgraded_to_update_call,
expected_response_metadata.upgraded_to_update_call
);
assert_eq!(
response_metadata.response_verification_version,
expected_response_metadata.response_verification_version
);
}

0 comments on commit d4cb9e4

Please sign in to comment.