Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revamp errors for aws-sigv4 #1937

Merged
merged 2 commits into from
Nov 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions aws/rust-runtime/aws-sig-auth/external-types.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
allowed_external_types = [
"aws_sigv4::http_request::sign::SignableBody",
"aws_sigv4::http_request::error::SigningError",
"aws_smithy_http::*",
"aws_types::*",
"http::request::Request",
Expand Down
4 changes: 2 additions & 2 deletions aws/rust-runtime/aws-sig-auth/src/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl From<SigningError> for SigningStageError {
impl Error for SigningStageError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match &self {
SigningStageError::SigningFailure(err) => Some(err.as_ref()),
SigningStageError::SigningFailure(err) => Some(err),
_ => None,
}
}
Expand Down Expand Up @@ -160,7 +160,7 @@ impl MapRequest for SigV4SigningStage {
let signature = self
.signer
.sign(operation_config, &request_config, &creds, &mut req)
.map_err(|err| SigningStageError::SigningFailure(err))?;
.map_err(SigningStageError::SigningFailure)?;
config.insert(signature);
Ok(req)
})
Expand Down
4 changes: 1 addition & 3 deletions aws/rust-runtime/aws-sig-auth/src/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ use aws_smithy_http::body::SdkBody;
use aws_types::region::SigningRegion;
use aws_types::Credentials;
use aws_types::SigningService;
use std::error::Error;
use std::fmt;
use std::time::{Duration, SystemTime};

pub use aws_sigv4::http_request::SignableBody;
pub type SigningError = aws_sigv4::http_request::SigningError;

const EXPIRATION_WARNING: &str = "Presigned request will expire before the given \
`expires_in` duration because the credentials used to sign it will expire first.";
Expand Down Expand Up @@ -121,8 +121,6 @@ impl fmt::Debug for SigV4Signer {
}
}

pub type SigningError = Box<dyn Error + Send + Sync>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉


impl SigV4Signer {
pub fn new() -> Self {
SigV4Signer { _private: () }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
* SPDX-License-Identifier: Apache-2.0
*/

use super::query_writer::QueryWriter;
use super::{Error, PayloadChecksumKind, SignableBody, SignatureLocation, SigningParams};
use crate::date_time::{format_date, format_date_time};
use crate::http_request::error::CanonicalRequestError;
use crate::http_request::query_writer::QueryWriter;
use crate::http_request::sign::SignableRequest;
use crate::http_request::url_escape::percent_encode_path;
use crate::http_request::PercentEncodingMode;
use crate::http_request::{PayloadChecksumKind, SignableBody, SignatureLocation, SigningParams};
use crate::sign::sha256_hex_string;
use http::header::{HeaderName, HOST};
use http::{HeaderMap, HeaderValue, Method, Uri};
Expand Down Expand Up @@ -124,7 +125,7 @@ impl<'a> CanonicalRequest<'a> {
pub(super) fn from<'b>(
req: &'b SignableRequest<'b>,
params: &'b SigningParams<'b>,
) -> Result<CanonicalRequest<'b>, Error> {
) -> Result<CanonicalRequest<'b>, CanonicalRequestError> {
// Path encoding: if specified, re-encode % as %25
// Set method and path into CanonicalRequest
let path = req.uri().path();
Expand Down Expand Up @@ -182,7 +183,7 @@ impl<'a> CanonicalRequest<'a> {
params: &SigningParams<'_>,
payload_hash: &str,
date_time: &str,
) -> Result<(Vec<CanonicalHeaderName>, HeaderMap), Error> {
) -> Result<(Vec<CanonicalHeaderName>, HeaderMap), CanonicalRequestError> {
// Header computation:
// The canonical request will include headers not present in the input. We need to clone and
// normalize the headers from the original request and add:
Expand Down Expand Up @@ -375,9 +376,15 @@ fn trim_spaces_from_byte_string(bytes: &[u8]) -> &[u8] {

/// Works just like [trim_all] but acts on HeaderValues instead of bytes.
/// Will ensure that the underlying bytes are valid UTF-8.
fn normalize_header_value(header_value: &HeaderValue) -> Result<HeaderValue, Error> {
fn normalize_header_value(
header_value: &HeaderValue,
) -> Result<HeaderValue, CanonicalRequestError> {
let trimmed_value = trim_all(header_value.as_bytes());
HeaderValue::from_str(std::str::from_utf8(&trimmed_value)?).map_err(Error::from)
HeaderValue::from_str(
std::str::from_utf8(&trimmed_value)
.map_err(CanonicalRequestError::invalid_utf8_in_header_value)?,
)
.map_err(CanonicalRequestError::from)
}

#[derive(Debug, PartialEq, Default)]
Expand Down
104 changes: 104 additions & 0 deletions aws/rust-runtime/aws-sigv4/src/http_request/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

use http::header::{InvalidHeaderName, InvalidHeaderValue};
use std::error::Error;
use std::fmt;
use std::str::Utf8Error;

#[derive(Debug)]
enum SigningErrorKind {
FailedToCreateCanonicalRequest { source: CanonicalRequestError },
}

/// Error signing request
#[derive(Debug)]
pub struct SigningError {
kind: SigningErrorKind,
}

impl fmt::Display for SigningError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.kind {
SigningErrorKind::FailedToCreateCanonicalRequest { .. } => {
write!(f, "failed to create canonical request")
}
}
}
}

impl Error for SigningError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match &self.kind {
SigningErrorKind::FailedToCreateCanonicalRequest { source } => Some(source),
}
}
}
Comment on lines +22 to +38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this follows #1345?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The source doesn't get written in the Display impl so that it's not duplicated for error reporters that follow the source chain.


impl From<CanonicalRequestError> for SigningError {
fn from(source: CanonicalRequestError) -> Self {
Self {
kind: SigningErrorKind::FailedToCreateCanonicalRequest { source },
}
}
}

#[derive(Debug)]
enum CanonicalRequestErrorKind {
InvalidHeaderName { source: InvalidHeaderName },
InvalidHeaderValue { source: InvalidHeaderValue },
InvalidUtf8InHeaderValue { source: Utf8Error },
}

#[derive(Debug)]
pub(crate) struct CanonicalRequestError {
kind: CanonicalRequestErrorKind,
}

impl fmt::Display for CanonicalRequestError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use CanonicalRequestErrorKind::*;
match self.kind {
InvalidHeaderName { .. } => write!(f, "invalid header name"),
InvalidHeaderValue { .. } => write!(f, "invalid header value"),
InvalidUtf8InHeaderValue { .. } => write!(f, "invalid UTF-8 in header value"),
}
}
}

impl Error for CanonicalRequestError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
use CanonicalRequestErrorKind::*;
match &self.kind {
InvalidHeaderName { source } => Some(source),
InvalidHeaderValue { source } => Some(source),
InvalidUtf8InHeaderValue { source } => Some(source),
}
}
}

impl CanonicalRequestError {
pub(crate) fn invalid_utf8_in_header_value(source: Utf8Error) -> Self {
Self {
kind: CanonicalRequestErrorKind::InvalidUtf8InHeaderValue { source },
}
}
}

impl From<InvalidHeaderName> for CanonicalRequestError {
fn from(source: InvalidHeaderName) -> Self {
Self {
kind: CanonicalRequestErrorKind::InvalidHeaderName { source },
}
}
}

impl From<InvalidHeaderValue> for CanonicalRequestError {
fn from(source: InvalidHeaderValue) -> Self {
Self {
kind: CanonicalRequestErrorKind::InvalidHeaderValue { source },
}
}
}
6 changes: 4 additions & 2 deletions aws/rust-runtime/aws-sigv4/src/http_request/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! # Example: Signing an HTTP request
//!
//! ```rust
//! # fn test() -> Result<(), aws_sigv4::http_request::Error> {
//! # fn test() -> Result<(), aws_sigv4::http_request::SigningError> {
//! use aws_sigv4::http_request::{sign, SigningSettings, SigningParams, SignableRequest};
//! use http;
//! use std::time::SystemTime;
Expand Down Expand Up @@ -42,6 +42,7 @@
//!

mod canonical_request;
mod error;
mod query_writer;
mod settings;
mod sign;
Expand All @@ -50,7 +51,8 @@ mod url_escape;
#[cfg(test)]
pub(crate) mod test;

pub use error::SigningError;
pub use settings::{
PayloadChecksumKind, PercentEncodingMode, SignatureLocation, SigningParams, SigningSettings,
};
pub use sign::{sign, Error, SignableBody, SignableRequest};
pub use sign::{sign, SignableBody, SignableRequest};
11 changes: 4 additions & 7 deletions aws/rust-runtime/aws-sigv4/src/http_request/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

use super::error::SigningError;
use super::{PayloadChecksumKind, SignatureLocation};
use crate::http_request::canonical_request::header;
use crate::http_request::canonical_request::param;
Expand All @@ -15,12 +16,8 @@ use http::header::HeaderValue;
use http::{HeaderMap, Method, Uri};
use std::borrow::Cow;
use std::convert::TryFrom;
use std::error::Error as StdError;
use std::str;

/// Signing error type
pub type Error = Box<dyn StdError + Send + Sync + 'static>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉


/// Represents all of the information necessary to sign an HTTP request.
#[derive(Debug)]
#[non_exhaustive]
Expand Down Expand Up @@ -155,7 +152,7 @@ impl SigningInstructions {
pub fn sign<'a>(
request: SignableRequest<'a>,
params: &'a SigningParams<'a>,
) -> Result<SigningOutput<SigningInstructions>, Error> {
) -> Result<SigningOutput<SigningInstructions>, SigningError> {
tracing::trace!(request = ?request, params = ?params, "signing request");
match params.settings.signature_location {
SignatureLocation::Headers => {
Expand All @@ -181,7 +178,7 @@ type CalculatedParams = Vec<(&'static str, Cow<'static, str>)>;
fn calculate_signing_params<'a>(
request: &'a SignableRequest<'a>,
params: &'a SigningParams<'a>,
) -> Result<(CalculatedParams, String), Error> {
) -> Result<(CalculatedParams, String), SigningError> {
let creq = CanonicalRequest::from(request, params)?;
tracing::trace!(canonical_request = %creq);

Expand Down Expand Up @@ -230,7 +227,7 @@ fn calculate_signing_params<'a>(
fn calculate_signing_headers<'a>(
request: &'a SignableRequest<'a>,
params: &'a SigningParams<'a>,
) -> Result<SigningOutput<HeaderMap<HeaderValue>>, Error> {
) -> Result<SigningOutput<HeaderMap<HeaderValue>>, SigningError> {
// Step 1: https://docs.aws.amazon.com/en_pv/general/latest/gr/sigv4-create-canonical-request.html.
let creq = CanonicalRequest::from(request, params)?;
tracing::trace!(canonical_request = %creq);
Expand Down