refactor!: Result<Option<Self>, Error> -> Result<Self, Error>

BREAKING CHANGE: many of the `new(src: &str)` methods
now return `Result<Self, Error>` instead of `Result<Option<Self>, Error>`.

Author: SKalt

.vscode/settings.json

@@ -3,5 +3,5 @@
   "nixEnvSelector.nixFile": "${workspaceRoot}/flake.nix",
   "nix.enableLanguageServer": true,
   "nix.serverPath": "nil",
-  "cSpell.words": ["opencontainers", "repr"]
+  "cSpell.words": ["ebnf", "opencontainers", "repr"]
 }

README.md

@@ -6,7 +6,8 @@ A docker/OCI image reference parser.
 [![docs.rs](https://img.shields.io/docsrs/container_image_dist_ref)](https://docs.rs/container_image_dist_ref/latest/container_image_dist_ref/)
 
 This library is extensively tested against the authoritative image reference implementation, https://github.com/distribution/reference.
-`distribution/reference` uses the following [EBNF](https://www.w3.org/TR/xml11/#sec-notation) grammar:
+
+Image references follow this [EBNF](https://www.w3.org/TR/xml11/#sec-notation) grammar:
 
 <!-- {{{sh cat ./grammars/reference.ebnf }}}{{{out skip=2 -->
 
@@ -37,6 +38,15 @@ identifier           ::= [a-f0-9]{64}
 
 (This is translated from [https://github.com/distribution/reference/blob/main/reference.go](https://github.com/distribution/reference/blob/main/reference.go#L4-L26))
 
+To avoid worst-case performance, image references are restricted further:
+
+| part        | maximum length |
+| ----------- | -------------: |
+| `name`      |            255 |
+| `tag`       |            127 |
+| `digest`    |           1024 |
+| `algorithm` |            255 |
+
 ## Motivation
 
 <!-- TODO: rewrite -->

examples/parse_canonical.rs

@@ -14,8 +14,8 @@ fn main() {
         Ok(ref_str) => {
             let input = escape(&input);
             let name = escape(ref_str.name().to_str());
-            let domain = escape(ref_str.domain_str());
-            let path = escape(ref_str.path_str());
+            let domain = escape(ref_str.domain().to_str());
+            let path = escape(ref_str.path().to_str());
             let tag = escape(ref_str.tag().unwrap_or(""));
             let digest_algo = escape(ref_str.digest().algorithm().to_str());
             let digest_encoded = escape(ref_str.digest().encoded().to_str());

scripts/diff_digest.sh

@@ -6,12 +6,13 @@ use_rule() {  grep -E "^$2 " "$1"; }
 fake_diff_line() {
   local rule="$1"
   local oci ref
-  oci="$(use_rule ./grammars/oci_digest.ebnf "$rule")"
   ref="$(use_rule ./grammars/reference.ebnf "$rule")"
+  oci="$(use_rule ./grammars/oci_digest.ebnf "$rule")"
   if [ "$oci" = "$ref" ]; then
     printf " %s\n" "$oci";
   else
-    printf "-"; printf "%s\n+%s\n" "$ref" "$oci";
+   printf "-%s\n" "$ref";
+   printf "+%s\n" "$oci";
   fi
 }
 

src/ambiguous/domain_or_tagged_ref.rs

@@ -86,34 +86,27 @@ fn map_err(e: err::Error<u8>) -> err::Error<u16> {
 
 impl<'src> DomainOrRefSpan<'src> {
     pub(crate) fn new(src: &'src str) -> Result<Self, Error> {
-        let left = HostOrPathSpan::new(src, HostOrPathKind::Any)?
-            .ok_or(Error::at(0, err::Kind::HostOrPathMissing))?;
-        let right_src = &src[left.len()..];
+        let left = HostOrPathSpan::new(src, HostOrPathKind::Any)?;
+        let mut len = left.short_len().widen().upcast();
+        let right_src = &src[len as usize..];
         let right = match right_src.bytes().next() {
             Some(b':') => {
-                let len = left.short_len().widen().upcast() + 1;
-                // +1 for the leading ':'
-                let right = PortOrTagSpan::new(&right_src[1..], Port)
+                len += 1; // +1 for the leading ':'
+                PortOrTagSpan::new(&right_src[1..], Port)
                     .map_err(map_err)
-                    .map_err(|e| e + len)?;
-                Ok(Some(
-                    right.ok_or(Error::at(len, err::Kind::PortOrTagMissing))?,
-                ))
+                    .map(Some)
             }
             Some(b'/') | Some(b'@') | None => Ok(None),
-            Some(_) => Error::at(
-                left.short_len().upcast().into(),
-                err::Kind::PortOrTagInvalidChar,
-            )
-            .into(),
-        }?;
+            Some(_) => Error::at(0, err::Kind::PortOrTagInvalidChar).into(),
+        }
+        .map_err(|e| e + len)?;
 
-        let len = left.len() + right.map(|r| r.len() + 1).unwrap_or(0); // +1 for the leading ':'
-        let rest = &src[len..];
+        len += right.map(|r| r.short_len().widen().upcast()).unwrap_or(0);
+        let rest = &src[len as usize..];
         match rest.bytes().next() {
             Some(b'@') | None => {
                 // since the next section must be a digest, the right side must be a tag
-                let path = PathSpan::from_ambiguous(left).map_err(map_err)?;
+                let path = PathSpan::try_from(left).map_err(map_err)?;
                 let tag = if let Some(tag) = right {
                     Some(
                         tag.try_into()
@@ -136,9 +129,7 @@ impl<'src> DomainOrRefSpan<'src> {
                     match left.kind() {
                         Path => {
                             // need to extend the path
-                            let path = PathSpan::from_ambiguous(left)?
-                                .extend(rest)
-                                .map_err(map_err)?;
+                            let path = PathSpan::try_from(left)?.extend(rest).map_err(map_err)?;
 
                             let tag = if let Some(t) = right {
                                 Some(

src/ambiguous/host_or_path.rs

@@ -251,8 +251,8 @@ impl<'src> HostOrPathSpan<'src> {
         self.1
     }
 
-    /// can return 0-length spans if at EOF or the first character is a `/` or `@`
-    pub(crate) fn new(src: &'src str, kind: Kind) -> Result<Option<Self>, Error> {
+    /// can return None if at EOF or the first character is a `/` or `@`
+    pub(crate) fn new(src: &'src str, kind: Kind) -> Result<Self, Error> {
         let mut state = State {
             len: 0,
             scan: kind.into(), // <- scan's setters will enforce the kind's constraint(s)
@@ -264,11 +264,13 @@ impl<'src> HostOrPathSpan<'src> {
             #[cfg(test)]
             let _c = c.map(|c| c as char);
             match c {
-                None => return Ok(None),
+                None => return Error::at(0, err::Kind::HostOrPathMissing).into(),
                 Some(b'[') => {
                     return match kind {
-                        Kind::IpV6 | Kind::Any => Ok(ipv6::Ipv6Span::new(src)?
-                            .map(|span| Self(span.into_length().unwrap(), Kind::IpV6, 0))),
+                        Kind::IpV6 | Kind::Any => {
+                            let span = ipv6::Ipv6Span::new(src)?;
+                            Ok(Self(span.into_length().unwrap(), Kind::IpV6, 0))
+                        }
                         _ => Err(Error::at(0, InvalidChar)),
                     }
                 }
@@ -300,8 +302,9 @@ impl<'src> HostOrPathSpan<'src> {
             state.len,
             src.len()
         );
-        Ok(ShortLength::new(state.len)
-            .map(|length| Self(length, state.scan.into(), state.deciding_char.unwrap_or(0))))
+        ShortLength::new(state.len)
+            .ok_or(Error::at(0, err::Kind::HostOrPathMissing))
+            .map(|length| Self(length, state.scan.into(), state.deciding_char.unwrap_or(0)))
     }
     pub(crate) fn narrow(self, target_kind: Kind) -> Result<Self, Error> {
         use Kind::*;
@@ -337,7 +340,6 @@ mod tests {
                 );
             })
             .unwrap()
-            .unwrap()
     }
     fn should_parse_as(src: &str, expected: &str, kind: Kind) {
         let host_or_path = should_parse(src);
@@ -361,7 +363,6 @@ mod tests {
 
     fn should_fail_with(src: &str, err_kind: err::Kind, bad_char_index: u8) {
         let err = super::HostOrPathSpan::new(src, Kind::Any)
-            .map(|e| e.unwrap())
             .map(|e| {
                 assert!(
                     false,

src/ambiguous/port_or_tag.rs

@@ -97,8 +97,10 @@ impl<'src> PortOrTagSpan<'src> {
             first_tag_char: self.first_tag_char,
         })
     }
-    /// can match an empty span if the first character in src is a `/` or `@`
-    pub(crate) fn new(src: &str, kind: Kind) -> Result<Option<Self>, Error> {
+    /// Parse a port or tag from the start of a string.
+    /// Does NOT include the leading colon.
+    /// Can match an empty span if the first character in src is a `/` or `@`
+    pub(crate) fn new(src: &str, kind: Kind) -> Result<Self, Error> {
         let mut bytes = src.bytes();
 
         // the first character after the colon must be alphanumeric or an underscore
@@ -110,7 +112,7 @@ impl<'src> PortOrTagSpan<'src> {
             Some(b'a'..=b'z') | Some(b'A'..=b'Z') | Some(b'_') => kind
                 .update(Kind::Tag) // only tags can have non-numeric characters
                 .map_err(|_| err::Kind::PortInvalidChar),
-            None | Some(b'/') | Some(b'@') => return Ok(None),
+            None | Some(b'/') | Some(b'@') => Err(err::Kind::PortOrTagMissing),
             _ => Err(err::Kind::PortOrTagInvalidChar),
         }
         .map_err(|err_kind| Error::at(0, err_kind))?;
@@ -145,11 +147,11 @@ impl<'src> PortOrTagSpan<'src> {
             true
         });
 
-        Ok(Some(Self {
+        Ok(Self {
             length: ShortLength::from_nonzero(state.len),
             kind: state.kind,
             first_tag_char: state.first_tag_char,
-        }))
+        })
     }
 }
 
@@ -160,13 +162,10 @@ mod tests {
     fn should_parse_as(src: &str, kind: Kind) {
         let tag = PortOrTagSpan::new(src, kind);
         match tag {
-            Ok(Some(tag)) => {
+            Ok(tag) => {
                 assert_eq!(tag.span().span_of(src), src);
                 assert_eq!(tag.kind, kind);
             }
-            Ok(None) => {
-                assert!(src.is_empty() || src.starts_with('/') || src.starts_with('@'));
-            }
             Err(e) => panic!("failed to parse tag {src:?}: {:?}", e),
         }
     }

src/digest/algorithm.rs

@@ -1,4 +1,5 @@
-//! # Algorithm
+//! # Digest Algorithm
+//!
 //! There are two specifications for a digest algorithm string:
 //! - the [OCI Image Spec](https://github.com/opencontainers/image-spec/blob/v1.0.2/descriptor.md#digests)
 //! - [github.com/distribution/reference](https://github.com/distribution/reference/blob/v0.5.0/reference.go#L21-L23)
@@ -41,6 +42,7 @@ fn try_add(a: NonZeroU8, b: u8) -> Result<NonZeroU8, Error> {
     a.checked_add(b)
         .ok_or(Error::at(u8::MAX.into(), err::Kind::AlgorithmTooLong))
 }
+
 /// While there's no specification for the max length of an algorithm string,
 /// 255 characters is a reasonable upper bound.
 pub const MAX_LEN: u8 = u8::MAX;
@@ -80,32 +82,56 @@ impl<'src> AlgorithmSpan<'src> {
     }
 }
 
+/// The algorithm section of a digest.
+/// ```rust
+/// use container_image_dist_ref::digest::{
+///     algorithm::AlgorithmStr, Compliance, Standard
+/// };
+/// let (algorithm, compliance) = AlgorithmStr::new("sha256").unwrap();
+/// assert_eq!(algorithm.to_str(), "sha256");
+/// assert_eq!(compliance, Compliance::Universal);
+/// assert_eq!(algorithm.compliance(), Compliance::Universal);
+/// assert!(compliance.compliant_with(Standard::Oci));
+/// assert!(compliance.compliant_with(Standard::Distribution));
+///
+/// let (algorithm, _) = AlgorithmStr::new("a+b").unwrap();
+/// assert_eq!(algorithm.to_str(), "a+b");
+/// assert_eq!(algorithm.parts().collect::<Vec<_>>(), vec!["a", "b"]);
+/// ```
 pub struct AlgorithmStr<'src>(&'src str);
 impl<'src> AlgorithmStr<'src> {
+    #[allow(missing_docs)]
     #[inline]
     pub fn to_str(&self) -> &'src str {
         self.0
     }
+    #[allow(missing_docs)]
     pub fn len(&self) -> usize {
         self.to_str().len()
     }
+    #[allow(missing_docs)]
     pub fn is_empty(&self) -> bool {
         self.to_str().is_empty()
     }
-    pub fn from_prefix(src: &'src str) -> Result<(Self, Compliance), Error> {
+    /// Parse an algorithm from the start of the string. Parsing may not consume the entire string
+    /// if it reaches a valid stopping point, i.e. `:`.
+    pub fn new(src: &'src str) -> Result<(Self, Compliance), Error> {
         let (span, compliance) = AlgorithmSpan::new(src)?;
         Ok((Self(span.span_of(src)), compliance))
     }
+    /// checks that the entire source string is parsed.
     pub fn from_exact_match(src: &'src str) -> Result<(Self, Compliance), Error> {
         let (span, compliance) = AlgorithmSpan::from_exact_match(src)?;
         Ok((Self(span.span_of(src)), compliance))
     }
     pub(super) fn from_span(src: &'src str, span: AlgorithmSpan<'src>) -> Self {
         Self(span.span_of(src))
     }
+    /// Split the algorithm string into its components separated by `+`, `.`, `_`, or `-`.
     pub fn parts(&self) -> impl Iterator<Item = &str> {
         self.to_str().split(|c| is_separator(c as u8))
     }
+    /// Whether the algorithm is compliant with the OCI or distribution/reference specifications.
     pub fn compliance(&self) -> Compliance {
         let mut bytes = self.to_str().bytes();
         match bytes.next().unwrap() {

src/digest/encoded.rs

@@ -21,9 +21,11 @@
 use core::num::NonZeroU16;
 
 use super::{algorithm::AlgorithmStr, Compliance};
+use crate::err;
 use crate::span::{impl_span_methods_on_tuple, Lengthy, LongLength};
-
-pub const MAX_LEN: u16 = 1024; // arbitrary but realistic limit
+/// an arbitrary maximum length for the encoded section of a digest.
+/// This a realistic limit; hex-encoded sha512 digests are 128 characters long.
+pub const MAX_LEN: u16 = 1024;
 
 use crate::err::Kind::{
     EncodedInvalidChar, EncodedNonLowerHex, EncodingTooLong, EncodingTooShort,
@@ -35,10 +37,7 @@ type Error = crate::err::Error<u16>;
 pub(crate) struct EncodedSpan<'src>(LongLength<'src>);
 impl_span_methods_on_tuple!(EncodedSpan, u16, NonZeroU16);
 impl<'src> EncodedSpan<'src> {
-    pub(crate) fn new(
-        src: &'src str,
-        compliance: Compliance,
-    ) -> Result<Option<(Self, Compliance)>, Error> {
+    pub(crate) fn new(src: &'src str, compliance: Compliance) -> Result<(Self, Compliance), Error> {
         use Compliance::*;
         let mut len = 0;
         let mut compliance = compliance;
@@ -65,18 +64,27 @@ impl<'src> EncodedSpan<'src> {
 
         debug_assert!(len as usize == src.len(), "must have consume all src");
 
-        Ok(LongLength::new(len).map(|length| (Self(length), compliance)))
+        LongLength::new(len)
+            .ok_or(Error::at(0, err::Kind::EncodedMissing))
+            .map(|length| (Self(length), compliance))
     }
 }
 
+/// The encoded portion of a digest string. This may not be a hex-encoded value,
+/// since the OCI spec allows for base64 encoding.
 pub struct EncodedStr<'src>(&'src str);
 impl<'src> EncodedStr<'src> {
+    #[allow(missing_docs)]
     pub fn to_str(&self) -> &'src str {
         self.0
     }
-    // no implementation of from_prefix(&str) because digests MUST terminate a
-    // reference
-
+    /// Parses a string into an encoded digest value. The string must be a valid
+    /// with respect to the given standard (Oci, Distribution, or Universal).
+    /// Parsing always continues until the end of the string or an error.
+    pub fn new(src: &'src str, compliance: Compliance) -> Result<Self, Error> {
+        let (span, _compliance) = EncodedSpan::new(src, compliance)?;
+        Ok(Self::from_span(src, span))
+    }
     pub(crate) fn from_span(src: &'src str, span: EncodedSpan<'src>) -> Self {
         Self(span.span_of(src))
     }
@@ -160,7 +168,7 @@ mod tests {
     #[test]
     fn encoded_consumes_all() {
         fn consumes_all(src: &str) -> Result<(), Error> {
-            let (span, _) = EncodedSpan::new(src, Compliance::Oci)?.unwrap();
+            let (span, _) = EncodedSpan::new(src, Compliance::Oci)?;
             assert_eq!(span.len(), src.len());
             Ok(())
         }