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

age: Document that Identity::unwrap_stanza is only called via Identity::unwrap_stanzas #509

Closed
str4d opened this issue Aug 4, 2024 · 1 comment
Milestone

Comments

@str4d
Copy link
Owner

str4d commented Aug 4, 2024

Per #507 (comment), I'm going to remove Identity::unwrap_stanza, and make Identity::unwrap_stanzas a required method. The latter will also be documented as always receiving all stanzas from the same file (rather than the current documentation that this is assumed).

@str4d
Copy link
Owner Author

str4d commented Aug 4, 2024

Hmm, after deciding to do this, I took another look at @FiloSottile's age and realised that although its Identity only exposes multi-stanza unwrapping, every single impl uses an internal multiUnwrap helper that does exactly what the default impl of Identity::unwrap_stanzas does:

rage/age/src/lib.rs

Lines 216 to 218 in a510e76

fn unwrap_stanzas(&self, stanzas: &[Stanza]) -> Option<Result<FileKey, DecryptError>> {
stanzas.iter().find_map(|stanza| self.unwrap_stanza(stanza))
}

The Rust impl is significantly cleaner because Rust has Option and can return Option<Result<_, _>>, whereas Go matches on the returned err looking for a sentinel ErrIncorrectIdentity that serves the same role as None in our API.

So, I'm now back on the fence about whether to remove Identity::unwrap_stanza. It's currently the case that Decryptor only ever calls Identity::unwrap_stanzas, so we could just document on Identity that its canonical entry point is Identity::unwrap_stanzas, and then identity impls that want to add file-level stanza checks can do so by overriding the default impl.

@str4d str4d changed the title age: Remove Identity::unwrap_stanza age: Document that Identity::unwrap_stanza is only called via Identity::unwrap_stanzas Nov 3, 2024
@str4d str4d closed this as completed in d0889c9 Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant