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

Use thiserror for error handling #156

Merged
merged 1 commit into from
Aug 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ tag-prefix = ""

[dependencies]
base64 = "0.12"
error-chain = { version = "0.12", default-features = false }
futures = "0.3"
http = "0.2"
libflate = "1.0"
Expand All @@ -41,6 +40,8 @@ tokio = "0.2"
reqwest = { version = "0.10", default-features = false, features = ["json"] }
sha2 = "^0.9.0"
async-stream = "0.3"
thiserror = "1.0.19"
url = "2.1.1"

[dev-dependencies]
dirs = "3.0"
Expand Down
5 changes: 1 addition & 4 deletions examples/image-labels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,7 @@ async fn run(
let version = dkr_ref.version();

let dclient = client.authenticate(&[&login_scope]).await?;
let manifest = match dclient.get_manifest(&image, &version).await {
Ok(manifest) => Ok(manifest),
Err(e) => Err(format!("Got error {}", e)),
}?;
let manifest = dclient.get_manifest(&image, &version).await?;

if let Manifest::S1Signed(s1s) = manifest {
let labels = s1s.get_labels(0);
Expand Down
79 changes: 65 additions & 14 deletions src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,68 @@
//! Error chains, types and traits.
//! Defines root error type

error_chain! {
foreign_links {
Base64Decode(base64::DecodeError);
HeaderInvalid(http::header::InvalidHeaderValue);
HeaderParse(http::header::ToStrError);
Hyper(http::Error);
Io(std::io::Error);
Json(serde_json::Error);
Regex(regex::Error);
Reqwest(reqwest::Error);
UriParse(http::uri::InvalidUri);
Utf8Parse(std::string::FromUtf8Error);
StrumParse(strum::ParseError);
#[non_exhaustive]
#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error("base64 decode error")]
Base64Decode(#[from] base64::DecodeError),
#[error("header parse error")]
HeaderParse(#[from] http::header::ToStrError),
#[error("json error")]
Json(#[from] serde_json::Error),
#[error("http transport error")]
Reqwest(#[from] reqwest::Error),
#[error("URI parse error")]
Uri(#[from] url::ParseError),
#[error("input is not UTF-8")]
Ut8Parse(#[from] std::string::FromUtf8Error),
#[error("strum error")]
StrumParse(#[from] strum::ParseError),
#[error("authentication information missing for index {0}")]
AuthInfoMissing(String),
#[error("unknown media type {0:?}")]
UnknownMimeType(mime::Mime),
#[error("unknown media type {0:?}")]
UnsupportedMediaType(crate::mediatypes::MediaTypes),
#[error("mime parse error")]
MimeParse(#[from] mime::FromStrError),
#[error("missing authentication header {0}")]
MissingAuthHeader(&'static str),
#[error("unexpected HTTP status {0}")]
UnexpectedHttpStatus(http::StatusCode),
#[error("invalid auth token '{0}'")]
InvalidAuthToken(String),
#[error("API V2 not supported")]
V2NotSupported,
#[error("obtained token is invalid")]
LoginReturnedBadToken,
#[error("www-authenticate header parse error")]
Www(#[from] crate::v2::WwwHeaderParseError),
#[error("request failed with status {status} and body of size {len}: {}", String::from_utf8_lossy(&body))]
Client {
status: http::StatusCode,
len: usize,
body: Vec<u8>,
},
#[error("content digest error")]
ContentDigestParse(#[from] crate::v2::ContentDigestError),
#[error("no header Content-Type given and no workaround to apply")]
MediaTypeSniff,
#[error("manifest error")]
Manifest(#[from] crate::v2::manifest::ManifestError),
#[error("reference is invalid")]
ReferenceParse(#[from] crate::reference::ReferenceParseError),
#[error("requested operation requires that credentials are available")]
NoCredentials
}

pub type Result<T> = std::result::Result<T, Error>;

#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_error_bounds() {
fn check_bounds<T: Send + Sync + 'static>() {}
check_bounds::<Error>();
}
}
7 changes: 3 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@
#[macro_use]
extern crate serde;
#[macro_use]
extern crate error_chain;
#[macro_use]
extern crate log;
#[macro_use]
extern crate strum_macros;
Expand All @@ -48,10 +46,11 @@ pub mod reference;
pub mod render;
pub mod v2;

use errors::Result;
use errors::{Result, Error};
use std::collections::HashMap;
use std::io::Read;


/// Default User-Agent client identity.
pub static USER_AGENT: &str = "camallo-dkregistry/0.0";

Expand All @@ -71,7 +70,7 @@ pub fn get_credentials<T: Read>(
};
let auth = match map.auths.get(real_index) {
Some(x) => base64::decode(x.auth.as_str())?,
None => bail!("no auth for index {}", real_index),
None => return Err(Error::AuthInfoMissing(real_index.to_string())),
};
let s = String::from_utf8(auth)?;
let creds: Vec<&str> = s.splitn(2, ':').collect();
Expand Down
10 changes: 5 additions & 5 deletions src/mediatypes.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Media-types for API objects.

use crate::errors::{Error, Result};
use crate::errors::{Result};
use mime;
use strum::EnumProperty;

Expand Down Expand Up @@ -62,13 +62,13 @@ impl MediaTypes {
}
("vnd.docker.image.rootfs.diff.tar.gzip", _) => Ok(MediaTypes::ImageLayerTgz),
("vnd.docker.container.image.v1", "json") => Ok(MediaTypes::ContainerConfigV1),
_ => bail!("unknown mediatype {:?}", mtype),
_ => return Err(crate::Error::UnknownMimeType(mtype.clone())),
}
}
_ => bail!("unknown mediatype {:?}", mtype),
_ => return Err(crate::Error::UnknownMimeType(mtype.clone())),
}
}
pub fn to_mime(&self) -> Result<mime::Mime> {
pub fn to_mime(&self) -> mime::Mime {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub fn to_mime(&self) -> mime::Mime {
pub fn to_mime(&self) -> Result<mime::Mime> {

Please see the comment below

match self {
&MediaTypes::ApplicationJson => Ok(mime::APPLICATION_JSON),
ref m => {
Expand All @@ -79,6 +79,6 @@ impl MediaTypes {
}
}
}
.map_err(|e| Error::from(e.to_string()))
.expect("to_mime should be always successful")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't follow why this cannot fail. I'd prefer to leave the Result in place here, and let the caller make use of the ? operator which isn't expensive.

}
}
86 changes: 59 additions & 27 deletions src/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
// The `docker://` schema is not officially documented, but has a reference implementation:
// https://github.com/docker/distribution/blob/v2.6.1/reference/reference.go

use crate::errors::Error;
use regex;
use std::collections::VecDeque;
use std::str::FromStr;
Expand All @@ -46,20 +45,30 @@ pub enum Version {
Digest(String, String),
}

#[derive(thiserror::Error, Debug)]
pub enum VersionParseError {
#[error("wrong digest format: checksum missing")]
WrongDigestFormat,
#[error("unknown prefix: digest must start from : or @")]
UnknownPrefix,
#[error("empty string is invalid digest")]
Empty,
}

impl str::FromStr for Version {
type Err = Error;
type Err = VersionParseError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let v = match s.chars().nth(0) {
Some(':') => Version::Tag(s.trim_start_matches(':').to_string()),
Some('@') => {
let r: Vec<&str> = s.trim_start_matches('@').splitn(2, ':').collect();
if r.len() != 2 {
bail!("wrong digest format");
return Err(VersionParseError::WrongDigestFormat);
};
Version::Digest(r[0].to_string(), r[1].to_string())
}
Some(_) => bail!("unknown prefix"),
None => bail!("too short"),
Some(_) => return Err(VersionParseError::UnknownPrefix),
None => return Err(VersionParseError::Empty),
};
Ok(v)
}
Expand Down Expand Up @@ -146,13 +155,32 @@ impl fmt::Display for Reference {
}

impl str::FromStr for Reference {
type Err = Error;
type Err = ReferenceParseError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
parse_url(s)
}
}

fn parse_url(input: &str) -> Result<Reference, Error> {
#[derive(thiserror::Error, Debug)]
pub enum ReferenceParseError {
#[error("missing image name")]
MissingImageName,
#[error("version parse error")]
VersionParse(#[from] VersionParseError),
#[error("empty image name")]
EmptyImageName,
#[error("component '{component}' does not conform to regex '{regex}'")]
RegexViolation {
regex: &'static str,
component: String,
},
#[error("empty repository name")]
EmptyRepositoryName,
#[error("repository name too long")]
RepositoryNameTooLong,
}

fn parse_url(input: &str) -> Result<Reference, ReferenceParseError> {
// TODO(lucab): investigate using a grammar-based parser.
let mut rest = input;

Expand All @@ -173,7 +201,7 @@ fn parse_url(input: &str) -> Result<Reference, Error> {
// default registry if it's not.
let first = components
.pop_front()
.ok_or(Error::from("missing image name"))?;
.ok_or(ReferenceParseError::MissingImageName)?;

let registry = if regex::Regex::new(r"(?x)
^
Expand All @@ -183,7 +211,7 @@ fn parse_url(input: &str) -> Result<Reference, Error> {
# optional port
([:][0-9]{1,6})?
$
")?.is_match(&first) {
").expect("hardcoded regex is invalid").is_match(&first) {
first
} else {
components.push_front(first);
Expand All @@ -193,15 +221,17 @@ fn parse_url(input: &str) -> Result<Reference, Error> {
// Take image name and extract tag or digest-ref, if any.
let last = components
.pop_back()
.ok_or_else(|| Error::from("missing image name"))?;
.ok_or(ReferenceParseError::MissingImageName)?;
let (image_name, version) = match (last.rfind('@'), last.rfind(':')) {
(Some(i), _) | (None, Some(i)) => {
let s = last.split_at(i);
(String::from(s.0), Version::from_str(s.1)?)
}
(None, None) => (last, Version::default()),
};
ensure!(!image_name.is_empty(), "empty image name");
if image_name.is_empty() {
return Err(ReferenceParseError::EmptyImageName);
}

// Handle images in default library namespace, that is:
// `ubuntu` -> `library/ubuntu`
Expand All @@ -212,25 +242,27 @@ fn parse_url(input: &str) -> Result<Reference, Error> {

// Check if all path components conform to the regex at
// https://docs.docker.com/registry/spec/api/#overview.
let path_re = regex::Regex::new("^[a-z0-9]+(?:[._-][a-z0-9]+)*$")?;
components
.iter()
.try_for_each(|component| -> Result<(), Error> {
if !path_re.is_match(component) {
bail!(
"component '{}' doesn't conform to the regex '{}'",
component,
path_re.as_str()
)
};

Ok(())
})?;
const REGEX: &'static str = "^[a-z0-9]+(?:[._-][a-z0-9]+)*$";
let path_re = regex::Regex::new(REGEX).expect("hardcoded regex is invalid");
components.iter().try_for_each(|component| {
if !path_re.is_match(component) {
return Err(ReferenceParseError::RegexViolation {
component: component.clone(),
regex: REGEX,
});
};

Ok(())
})?;

// Re-assemble repository name.
let repository = components.into_iter().collect::<Vec<_>>().join("/");
ensure!(!repository.is_empty(), "empty repository name");
ensure!(repository.len() <= 127, "repository name too long");
if repository.is_empty() {
return Err(ReferenceParseError::EmptyRepositoryName);
}
if repository.len() > 127 {
return Err(ReferenceParseError::RepositoryNameTooLong);
}

Ok(Reference {
has_schema,
Expand Down
13 changes: 10 additions & 3 deletions src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,25 @@
// Docker image format is specified at
// https://github.com/moby/moby/blob/v17.05.0-ce/image/spec/v1.md

use crate::errors::*;
use libflate::gzip;
use std::{fs, path};
use tar;

#[derive(Debug, thiserror::Error)]
pub enum RenderError {
#[error("wrong target path {}: must be absolute path to existing directory", _0.display())]
WrongTargetPath(path::PathBuf),
#[error("io error")]
Io(#[from] std::io::Error)
}

/// Unpack an ordered list of layers to a target directory.
///
/// Layers must be provided as gzip-compressed tar archives, with lower layers
/// coming first. Target directory must be an existing absolute path.
pub fn unpack(layers: &[Vec<u8>], target_dir: &path::Path) -> Result<()> {
pub fn unpack(layers: &[Vec<u8>], target_dir: &path::Path) -> Result<(), RenderError> {
if !target_dir.is_absolute() || !target_dir.exists() || !target_dir.is_dir() {
bail!("wrong target path");
return Err(RenderError::WrongTargetPath(target_dir.to_path_buf()));
}
for l in layers {
// Unpack layers
Expand Down
Loading