Skip to content

Commit

Permalink
Redesign error handling in the decode module (#65)
Browse files Browse the repository at this point in the history
This commit changes the entire way that errors are handled while
decoding.

Sources now provide errors related to failing to get more data when
more is requested via the associated Source::Error type. This has been
renamed from Source::Err for consistency with other such associated types.

A new type ContentError is introduced for errors where incorrectly encoded
data is encountered, i.e., actual encoding errors or data that isn't
following the ASN.1 definition or informal profiles. This type wraps an
error message that currently can be either a static str or a boxed Display
trait object but can be extended if necessary later.

Since both source and content errors can happen during decoding, the
compound type DecodeError wraps both these types. For content errors it
also stores where in the source the error happened to facilitate debugging.
Currently, this type only allows displaying the error. It can be extended
if additional access to the internally stored error is necessary.

The various content decoding methods now return this DecodeError.

In order to implement all these changes, the Source trait had to be
adjusted.  First, it needs to be able to provide the current position.
This meant that it couldn’t be implemented on Bytes and &[u8] directly
anymore. Therefore, the new trait IntoSource allows to convert a type into
its Source implementation.

Finally, the trait's methods got cleaned up a bit. Specifically,
Source::advance now only allows advancing as far as the length most
recently returned by Source::request which also means it cannot fail
anymore but needs to panic if the length is too large. This consistent to
how Source::bytes already behaves.
  • Loading branch information
partim authored Jul 18, 2022
1 parent ed1f813 commit 30c3809
Show file tree
Hide file tree
Showing 18 changed files with 1,369 additions and 722 deletions.
8 changes: 1 addition & 7 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "bcder"
version = "0.6.2-dev"
version = "0.7.0-dev"
edition = "2018"
authors = ["The NLnet Labs RPKI Team <rpki-team@nlnetlabs.nl>"]
description = "Handling of data encoded in BER, CER, and DER."
Expand All @@ -11,12 +11,6 @@ categories = ["encoding", "network-programming", "parsing"]
license = "BSD-3-Clause"

[dependencies]
backtrace = { version = "^0.3.15", optional = true }
bytes = "^1.0"
smallvec = "^1.1"

[features]
# Print a backtrace when a parsing error occurs. This feature is intended for
# development use exclusively and MUST NOT be enabled in release builds.
extra-debug = [ "backtrace" ]

52 changes: 40 additions & 12 deletions src/captured.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
//!
//! This is a private module. Its public items are re-exported by the parent.
use std::{fmt, io, ops};
use std::{fmt, io, mem, ops};
use bytes::{Bytes, BytesMut};
use crate::{decode, encode};
use crate::decode::{BytesSource, DecodeError, IntoSource, Pos};
use crate::mode::Mode;


Expand Down Expand Up @@ -39,8 +40,14 @@ use crate::mode::Mode;
/// [`Mode`]: ../enum.Mode.html
#[derive(Clone)]
pub struct Captured {
/// The captured data.
bytes: Bytes,

/// The encoding mode of the captured data.
mode: Mode,

/// The start position of the data in the original source.
start: Pos,
}

impl Captured {
Expand All @@ -49,8 +56,8 @@ impl Captured {
/// Because we can’t guarantee that the bytes are properly encoded, we
/// keep this function crate public. The type, however, doesn’t rely on
/// content being properly encoded so this method isn’t unsafe.
pub(crate) fn new(bytes: Bytes, mode: Mode) -> Self {
Captured { bytes, mode }
pub(crate) fn new(bytes: Bytes, mode: Mode, start: Pos) -> Self {
Captured { bytes, mode, start }
}

/// Creates a captured value by encoding data.
Expand All @@ -68,7 +75,8 @@ impl Captured {
pub fn empty(mode: Mode) -> Self {
Captured {
bytes: Bytes::new(),
mode
mode,
start: Pos::default(),
}
}

Expand All @@ -90,11 +98,13 @@ impl Captured {
/// The method consumes the value. If you want to keep it around, simply
/// clone it first. Since bytes values are cheap to clone, this is
/// relatively cheap.
pub fn decode<F, T>(self, op: F) -> Result<T, decode::Error>
pub fn decode<F, T>(
self, op: F
) -> Result<T, DecodeError<<BytesSource as decode::Source>::Error>>
where
F: FnOnce(
&mut decode::Constructed<Bytes>
) -> Result<T, decode::Error>
&mut decode::Constructed<BytesSource>
) -> Result<T, DecodeError<<BytesSource as decode::Source>::Error>>
{
self.mode.decode(self.bytes, op)
}
Expand All @@ -104,13 +114,20 @@ impl Captured {
/// The method calls `op` to parse a number of values from the beginning
/// of the value and then advances the content of the captured value until
/// after the end of these decoded values.
pub fn decode_partial<F, T>(&mut self, op: F) -> Result<T, decode::Error>
pub fn decode_partial<F, T>(
&mut self, op: F
) -> Result<T, DecodeError<<BytesSource as decode::Source>::Error>>
where
F: FnOnce(
&mut decode::Constructed<&mut Bytes>
) -> Result<T, decode::Error>
&mut decode::Constructed<&mut BytesSource>
) -> Result<T, DecodeError<<BytesSource as decode::Source>::Error>>
{
self.mode.decode(&mut self.bytes, op)
let mut source = mem::replace(
&mut self.bytes, Bytes::new()
).into_source();
let res = self.mode.decode(&mut source, op);
self.bytes = source.into_bytes();
res
}

/// Trades the value for a bytes value with the raw data.
Expand Down Expand Up @@ -148,6 +165,17 @@ impl AsRef<[u8]> for Captured {
}


//--- IntoSource

impl IntoSource for Captured {
type Source = BytesSource;

fn into_source(self) -> Self::Source {
BytesSource::with_offset(self.bytes, self.start)
}
}


//--- encode::Values

impl encode::Values for Captured {
Expand Down Expand Up @@ -223,7 +251,7 @@ impl CapturedBuilder {
}

pub fn freeze(self) -> Captured {
Captured::new(self.bytes.freeze(), self.mode)
Captured::new(self.bytes.freeze(), self.mode, Pos::default())
}
}

Expand Down
50 changes: 0 additions & 50 deletions src/debug.rs

This file was deleted.

Loading

0 comments on commit 30c3809

Please sign in to comment.