refactor: reject mail with a reason identifier, not a sentence
dispatch and apply_commands returned English prose that became the reply subject, so the pure layers owned author-facing copy. They now return reason::Reason and the mail-building edge resolves it through the catalogue. core::rules needed no change: RuleError was already a typed enum, and its Display stays for author-side tools. The daemon maps the variants itself rather than borrowing that prose. Error-path tests now assert on variants, so rewording main.ftl cannot break them, and PruneModeUnknown carries the value the author actually sent. Reason::every_variant_resolves_to_copy walks the enum, so a variant added without a catalogue entry fails in tests rather than mailing someone "???reason-…???".
This commit is contained in:
parent
3279fd3a2d
commit
bf6e2eaa15
5 changed files with 244 additions and 42 deletions
|
|
@ -35,3 +35,39 @@ welcome-body =
|
|||
Post by sending mail to this address with the twt as the body. Manage it
|
||||
with Twt-Nick, Twt-Delete, Twt-Repost and Twt-Prune frontmatter fields
|
||||
(PLAN.md).
|
||||
|
||||
## Rejection reasons — one per `reason::Reason` variant, rendered into the
|
||||
## reply subject and body above. Each is a sentence fragment completing
|
||||
## "your mail could not be applied because …", so they start lowercase and
|
||||
## carry the fix where there is one.
|
||||
|
||||
reason-body-not-utf8 = body is not UTF-8
|
||||
reason-post-not-canonical = post mail is not in canonical form
|
||||
reason-repost-not-canonical = repost mail is not in canonical form
|
||||
reason-fields-beyond-reply-to =
|
||||
post mail carries fields beyond Reply-To (Subject, headers or an Accept
|
||||
token); resend the bare text
|
||||
reason-nick-invalid = nick must be 1-30 bytes of a-z, 0-9 or hyphen
|
||||
reason-delete-needs-timestamp = Twt-Delete needs an RFC 3339 UTC timestamp
|
||||
reason-prune-mode-unknown = unknown prune mode `{ $got }`, want fifo or off
|
||||
|
||||
## Post rules, from core::rules::RuleError
|
||||
|
||||
reason-too-many-codepoints = more than 160 Unicode code points
|
||||
reason-too-many-bytes = more than 640 bytes
|
||||
reason-forbidden-char = contains TAB, LF, CR or U+2028
|
||||
reason-bad-timestamp = not an RFC 3339 UTC timestamp at second precision
|
||||
reason-duplicate-timestamp = timestamp already exists in this feed
|
||||
|
||||
## Retention and quota, from the apply step
|
||||
|
||||
reason-over-quota-post = daily post quota exceeded; try tomorrow
|
||||
reason-over-quota-repost = daily repost quota exceeded; try tomorrow
|
||||
reason-collision = timestamp { $timestamp } already exists; wait a second and resend
|
||||
reason-feed-full =
|
||||
feed is at its size cap; delete records with Twt-Delete or choose
|
||||
Twt-Prune: fifo
|
||||
reason-reply-to-changed =
|
||||
your Reply-To does not match the feed's bound author { $author }; send from
|
||||
that address or mail the operator
|
||||
reason-no-record-at = no record at { $timestamp }
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ use smolboid_core::rules::validate_text;
|
|||
use smolboid_core::time;
|
||||
use smolboid_core::{repost_body, reply_to_body};
|
||||
|
||||
use crate::reason::Reason;
|
||||
use crate::state::PruneMode;
|
||||
|
||||
/// One fetched mail, opened and verified by the caller.
|
||||
|
|
@ -18,6 +19,7 @@ pub struct Incoming {
|
|||
pub body: Vec<u8>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub enum Action {
|
||||
Post {
|
||||
timestamp: String,
|
||||
|
|
@ -38,6 +40,7 @@ pub enum Action {
|
|||
Ignore,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub enum Command {
|
||||
Nick(String),
|
||||
Delete(String),
|
||||
|
|
@ -53,8 +56,11 @@ const F_PRUNE: &str = "twt-prune";
|
|||
const F_REPLY_TO: &str = "reply-to";
|
||||
|
||||
/// Fails closed: a block with unreconstructible fields is never published.
|
||||
pub fn dispatch(incoming: &Incoming) -> Result<Action, String> {
|
||||
let text = std::str::from_utf8(&incoming.body).map_err(|_| "body is not UTF-8".to_owned())?;
|
||||
///
|
||||
/// Returns a `Reason` rather than a sentence: this layer is pure, and what the
|
||||
/// author reads is the caller's business (see `reason`).
|
||||
pub fn dispatch(incoming: &Incoming) -> Result<Action, Reason> {
|
||||
let text = std::str::from_utf8(&incoming.body).map_err(|_| Reason::BodyNotUtf8)?;
|
||||
|
||||
if !text.starts_with("---\n") {
|
||||
// The bare post form: the body is the twt, byte for byte.
|
||||
|
|
@ -77,7 +83,7 @@ pub fn dispatch(incoming: &Incoming) -> Result<Action, String> {
|
|||
if beyond_reply_to.is_empty() {
|
||||
if let Some(reply_to) = &reply_to {
|
||||
if text != reply_to_body(reply_to, &rest) {
|
||||
return Err("post mail is not in canonical form".to_owned());
|
||||
return Err(Reason::PostNotCanonical);
|
||||
}
|
||||
return post(incoming, &rest, Some(reply_to.clone()));
|
||||
}
|
||||
|
|
@ -90,7 +96,7 @@ pub fn dispatch(incoming: &Incoming) -> Result<Action, String> {
|
|||
let canonical_no = repost_body(args, None);
|
||||
let canonical_yes = repost_body(args, reply_to.as_deref());
|
||||
if text == canonical_no || text == canonical_yes {
|
||||
validate_text(args).map_err(|e| e.to_string())?;
|
||||
validate_text(args)?;
|
||||
return Ok(Action::Repost {
|
||||
timestamp: time::render(incoming.time),
|
||||
args: args.clone(),
|
||||
|
|
@ -102,7 +108,7 @@ pub fn dispatch(incoming: &Incoming) -> Result<Action, String> {
|
|||
},
|
||||
});
|
||||
}
|
||||
return Err("repost mail is not in canonical form".to_owned());
|
||||
return Err(Reason::RepostNotCanonical);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -113,23 +119,19 @@ pub fn dispatch(incoming: &Incoming) -> Result<Action, String> {
|
|||
&& !fields.contains_key(F_PRUNE)
|
||||
&& !fields.contains_key(F_REPOST)
|
||||
{
|
||||
return Err(
|
||||
"post mail carries fields beyond Reply-To (Subject, headers or an Accept token); \
|
||||
resend the bare text"
|
||||
.to_owned(),
|
||||
);
|
||||
return Err(Reason::FieldsBeyondReplyTo);
|
||||
}
|
||||
|
||||
let mut commands = Vec::new();
|
||||
if let Some(nick) = fields.get(F_NICK) {
|
||||
if !crate::feedstore::valid_username(nick) {
|
||||
return Err("nick must be 1-30 bytes of a-z, 0-9 or hyphen".to_owned());
|
||||
return Err(Reason::NickInvalid);
|
||||
}
|
||||
commands.push(Command::Nick(nick.clone()));
|
||||
}
|
||||
if let Some(timestamp) = fields.get(F_DELETE) {
|
||||
if time::parse(timestamp).is_none() {
|
||||
return Err("Twt-Delete needs an RFC 3339 UTC timestamp".to_owned());
|
||||
return Err(Reason::DeleteNeedsTimestamp);
|
||||
}
|
||||
commands.push(Command::Delete(timestamp.clone()));
|
||||
}
|
||||
|
|
@ -137,7 +139,7 @@ pub fn dispatch(incoming: &Incoming) -> Result<Action, String> {
|
|||
commands.push(Command::Prune(match mode.as_str() {
|
||||
"fifo" => PruneMode::Fifo,
|
||||
"off" => PruneMode::Off,
|
||||
other => return Err(format!("unknown prune mode `{other}`, want fifo or off")),
|
||||
other => return Err(Reason::PruneModeUnknown { got: other.to_owned() }),
|
||||
}));
|
||||
}
|
||||
// A mixed mail is a command mail; the body is ignored, never posted.
|
||||
|
|
@ -148,8 +150,8 @@ pub fn dispatch(incoming: &Incoming) -> Result<Action, String> {
|
|||
}
|
||||
}
|
||||
|
||||
fn post(incoming: &Incoming, text: &str, reply_to: Option<String>) -> Result<Action, String> {
|
||||
validate_text(text).map_err(|e| e.to_string())?;
|
||||
fn post(incoming: &Incoming, text: &str, reply_to: Option<String>) -> Result<Action, Reason> {
|
||||
validate_text(text)?;
|
||||
Ok(Action::Post {
|
||||
timestamp: time::render(incoming.time),
|
||||
text: text.to_owned(),
|
||||
|
|
@ -207,19 +209,36 @@ mod tests {
|
|||
assert_eq!(text, "--- dashed opening");
|
||||
}
|
||||
|
||||
/// Asserting the variant, not the sentence: the copy lives in main.ftl
|
||||
/// and may be reworded without these tests caring.
|
||||
#[test]
|
||||
fn post_rules_are_enforced_with_reasons() {
|
||||
assert!(dispatch(&incoming(&"a".repeat(161))).is_err());
|
||||
assert!(dispatch(&incoming("a\tb")).is_err());
|
||||
use smolboid_core::rules::RuleError;
|
||||
assert_eq!(
|
||||
dispatch(&incoming(&"a".repeat(161))).unwrap_err(),
|
||||
Reason::Rule(RuleError::TooManyCodepoints)
|
||||
);
|
||||
assert_eq!(
|
||||
dispatch(&incoming("a\tb")).unwrap_err(),
|
||||
Reason::Rule(RuleError::ForbiddenChar)
|
||||
);
|
||||
let bad = Incoming { body: vec![0xff, 0xfe], ..incoming("") };
|
||||
assert!(dispatch(&bad).is_err());
|
||||
assert_eq!(dispatch(&bad).unwrap_err(), Reason::BodyNotUtf8);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extra_fields_make_a_rejected_post() {
|
||||
// A Subject, or the Accept token of an accepted recipient.
|
||||
assert!(dispatch(&incoming("---\nReply-To: smol://a/b\nSubject: hi\n---\nbody\n")).is_err());
|
||||
assert!(dispatch(&incoming("---\nAccept: token\n---\nbody\n")).is_err());
|
||||
for body in [
|
||||
"---\nReply-To: smol://a/b\nSubject: hi\n---\nbody\n",
|
||||
"---\nAccept: token\n---\nbody\n",
|
||||
] {
|
||||
assert_eq!(
|
||||
dispatch(&incoming(body)).unwrap_err(),
|
||||
Reason::FieldsBeyondReplyTo,
|
||||
"{body:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -274,9 +293,20 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn command_values_are_validated() {
|
||||
assert!(dispatch(&incoming("---\nTwt-Nick: Bad Nick\n---\n")).is_err());
|
||||
assert!(dispatch(&incoming("---\nTwt-Delete: not-a-date\n---\n")).is_err());
|
||||
assert!(dispatch(&incoming("---\nTwt-Prune: sometimes\n---\n")).is_err());
|
||||
assert_eq!(
|
||||
dispatch(&incoming("---\nTwt-Nick: Bad Nick\n---\n")).unwrap_err(),
|
||||
Reason::NickInvalid
|
||||
);
|
||||
assert_eq!(
|
||||
dispatch(&incoming("---\nTwt-Delete: not-a-date\n---\n")).unwrap_err(),
|
||||
Reason::DeleteNeedsTimestamp
|
||||
);
|
||||
// The offending value comes back with the reason, so the author is
|
||||
// told what they actually sent.
|
||||
assert_eq!(
|
||||
dispatch(&incoming("---\nTwt-Prune: sometimes\n---\n")).unwrap_err(),
|
||||
Reason::PruneModeUnknown { got: "sometimes".into() }
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ use smolboid_core::envelope;
|
|||
use crate::config::Config;
|
||||
use crate::dispatch::{self, Action, Command, Incoming};
|
||||
use crate::feedstore::FeedStore;
|
||||
use crate::reason::Reason;
|
||||
use crate::state::{payload_key, reply_allowed, FeedState, PruneMode, State};
|
||||
|
||||
use std::sync::{Mutex, MutexGuard};
|
||||
|
|
@ -131,7 +132,7 @@ pub fn apply_commands(
|
|||
feed: &mut Feed,
|
||||
fs: &mut FeedState,
|
||||
commands: &[Command],
|
||||
) -> Result<usize, String> {
|
||||
) -> Result<usize, Reason> {
|
||||
for command in commands {
|
||||
match command {
|
||||
Command::Nick(nick) => {
|
||||
|
|
@ -140,7 +141,7 @@ pub fn apply_commands(
|
|||
}
|
||||
Command::Delete(timestamp) => {
|
||||
if !feed.delete(timestamp) {
|
||||
return Err(format!("no record at {timestamp}"));
|
||||
return Err(Reason::NoRecordAt { timestamp: timestamp.clone() });
|
||||
}
|
||||
}
|
||||
Command::Prune(mode) => fs.prune = *mode,
|
||||
|
|
@ -368,20 +369,13 @@ impl Daemon {
|
|||
Ok(true)
|
||||
}
|
||||
Err(e) => {
|
||||
let reason = match &e {
|
||||
ApplyError::OverQuota(repost) => format!(
|
||||
"daily {} quota exceeded; try tomorrow",
|
||||
if *repost { "repost" } else { "post" }
|
||||
),
|
||||
ApplyError::Collision(ts) => {
|
||||
format!("timestamp {ts} already exists; wait a second and resend")
|
||||
}
|
||||
ApplyError::FeedFull => "feed is at its size cap; delete records with Twt-Delete or choose Twt-Prune: fifo".to_owned(),
|
||||
ApplyError::ReplyToChanged => format!(
|
||||
"your Reply-To does not match the feed's bound author {}; \
|
||||
send from that address or mail the operator",
|
||||
feed.author().unwrap_or_default()
|
||||
),
|
||||
let reason = match e {
|
||||
ApplyError::OverQuota(repost) => Reason::OverQuota { repost },
|
||||
ApplyError::Collision(timestamp) => Reason::Collision { timestamp },
|
||||
ApplyError::FeedFull => Reason::FeedFull,
|
||||
ApplyError::ReplyToChanged => Reason::ReplyToChanged {
|
||||
author: feed.author().unwrap_or_default().to_owned(),
|
||||
},
|
||||
};
|
||||
self.queue_reply(state, incoming, address, &reason, replies);
|
||||
Ok(false)
|
||||
|
|
@ -395,7 +389,7 @@ impl Daemon {
|
|||
state: &mut MutexGuard<'_, State>,
|
||||
incoming: &Incoming,
|
||||
address: &str,
|
||||
reason: &str,
|
||||
reason: &Reason,
|
||||
replies: &mut Vec<(String, String, String)>,
|
||||
) {
|
||||
let key = payload_key(&incoming.sender, incoming.time, &incoming.body);
|
||||
|
|
@ -405,12 +399,13 @@ impl Daemon {
|
|||
let text = std::str::from_utf8(&incoming.body)
|
||||
.map(|t| t.chars().take(200).collect::<String>())
|
||||
.unwrap_or_default();
|
||||
let sentence = reason.text();
|
||||
let mut args = fluent_bundle::FluentArgs::new();
|
||||
args.set("reason", reason.to_owned());
|
||||
args.set("reason", sentence.clone());
|
||||
args.set("quoted", text);
|
||||
replies.push((
|
||||
address.to_owned(),
|
||||
crate::i18n::t1("reply-subject", "reason", reason.to_owned()),
|
||||
crate::i18n::t1("reply-subject", "reason", sentence),
|
||||
crate::i18n::t_args("reply-body", Some(&args)),
|
||||
));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ mod feedstore;
|
|||
mod http;
|
||||
mod i18n;
|
||||
mod mailloop;
|
||||
mod reason;
|
||||
mod state;
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
|
|
|||
140
daemon/src/reason.rs
Normal file
140
daemon/src/reason.rs
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
//! Why a mail was rejected, as an identifier rather than a sentence.
|
||||
//!
|
||||
//! The pure layers — `dispatch`, `core::rules`, `apply_record` — decide *that*
|
||||
//! a mail fails and *why*; they do not decide what the author reads. Keeping
|
||||
//! the two apart is what lets `dispatch` stay pure and testable against
|
||||
//! variants instead of prose, and what lets the copy live in one Fluent
|
||||
//! catalogue (`locales/en-US/main.ftl`).
|
||||
//!
|
||||
//! Resolution happens at the mail-building edge, in `mailloop::queue_reply`.
|
||||
|
||||
use fluent_bundle::FluentArgs;
|
||||
use smolboid_core::rules::RuleError;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum Reason {
|
||||
BodyNotUtf8,
|
||||
PostNotCanonical,
|
||||
RepostNotCanonical,
|
||||
FieldsBeyondReplyTo,
|
||||
NickInvalid,
|
||||
DeleteNeedsTimestamp,
|
||||
PruneModeUnknown { got: String },
|
||||
/// A post-rule violation from `core::rules`, carried whole so the daemon
|
||||
/// maps it to copy and the library keeps its own `Display` for
|
||||
/// author-side tools.
|
||||
Rule(RuleError),
|
||||
OverQuota { repost: bool },
|
||||
Collision { timestamp: String },
|
||||
FeedFull,
|
||||
ReplyToChanged { author: String },
|
||||
NoRecordAt { timestamp: String },
|
||||
}
|
||||
|
||||
impl From<RuleError> for Reason {
|
||||
fn from(e: RuleError) -> Reason {
|
||||
Reason::Rule(e)
|
||||
}
|
||||
}
|
||||
|
||||
impl Reason {
|
||||
/// The Fluent message id for this reason. Every one of these must appear
|
||||
/// in `main.ftl`; `i18n`'s catalogue test is what enforces that.
|
||||
pub fn id(&self) -> &'static str {
|
||||
match self {
|
||||
Reason::BodyNotUtf8 => "reason-body-not-utf8",
|
||||
Reason::PostNotCanonical => "reason-post-not-canonical",
|
||||
Reason::RepostNotCanonical => "reason-repost-not-canonical",
|
||||
Reason::FieldsBeyondReplyTo => "reason-fields-beyond-reply-to",
|
||||
Reason::NickInvalid => "reason-nick-invalid",
|
||||
Reason::DeleteNeedsTimestamp => "reason-delete-needs-timestamp",
|
||||
Reason::PruneModeUnknown { .. } => "reason-prune-mode-unknown",
|
||||
Reason::Rule(RuleError::TooManyCodepoints) => "reason-too-many-codepoints",
|
||||
Reason::Rule(RuleError::TooManyBytes) => "reason-too-many-bytes",
|
||||
Reason::Rule(RuleError::ForbiddenChar) => "reason-forbidden-char",
|
||||
Reason::Rule(RuleError::BadTimestamp) => "reason-bad-timestamp",
|
||||
Reason::Rule(RuleError::DuplicateTimestamp) => "reason-duplicate-timestamp",
|
||||
Reason::OverQuota { repost: false } => "reason-over-quota-post",
|
||||
Reason::OverQuota { repost: true } => "reason-over-quota-repost",
|
||||
Reason::Collision { .. } => "reason-collision",
|
||||
Reason::FeedFull => "reason-feed-full",
|
||||
Reason::ReplyToChanged { .. } => "reason-reply-to-changed",
|
||||
Reason::NoRecordAt { .. } => "reason-no-record-at",
|
||||
}
|
||||
}
|
||||
|
||||
/// The variables its message interpolates, if any.
|
||||
pub fn args(&self) -> Option<FluentArgs<'static>> {
|
||||
let mut args = FluentArgs::new();
|
||||
match self {
|
||||
Reason::PruneModeUnknown { got } => args.set("got", got.clone()),
|
||||
Reason::Collision { timestamp } | Reason::NoRecordAt { timestamp } => {
|
||||
args.set("timestamp", timestamp.clone())
|
||||
}
|
||||
Reason::ReplyToChanged { author } => args.set("author", author.clone()),
|
||||
_ => return None,
|
||||
}
|
||||
Some(args)
|
||||
}
|
||||
|
||||
/// The author-facing sentence, used both as the reply's subject suffix and
|
||||
/// in its body.
|
||||
pub fn text(&self) -> String {
|
||||
crate::i18n::t_args(self.id(), self.args().as_ref())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Every variant must resolve to real copy. `i18n`'s own test covers the
|
||||
/// ids it knows about; this one walks the enum, so a variant added without
|
||||
/// a catalogue entry fails here.
|
||||
#[test]
|
||||
fn every_variant_resolves_to_copy() {
|
||||
let all = [
|
||||
Reason::BodyNotUtf8,
|
||||
Reason::PostNotCanonical,
|
||||
Reason::RepostNotCanonical,
|
||||
Reason::FieldsBeyondReplyTo,
|
||||
Reason::NickInvalid,
|
||||
Reason::DeleteNeedsTimestamp,
|
||||
Reason::PruneModeUnknown { got: "sideways".into() },
|
||||
Reason::Rule(RuleError::TooManyCodepoints),
|
||||
Reason::Rule(RuleError::TooManyBytes),
|
||||
Reason::Rule(RuleError::ForbiddenChar),
|
||||
Reason::Rule(RuleError::BadTimestamp),
|
||||
Reason::Rule(RuleError::DuplicateTimestamp),
|
||||
Reason::OverQuota { repost: false },
|
||||
Reason::OverQuota { repost: true },
|
||||
Reason::Collision { timestamp: "2026-10-01T12:47:00Z".into() },
|
||||
Reason::FeedFull,
|
||||
Reason::ReplyToChanged { author: "tobias@smol.place".into() },
|
||||
Reason::NoRecordAt { timestamp: "2026-10-01T12:47:00Z".into() },
|
||||
];
|
||||
for reason in all {
|
||||
let text = reason.text();
|
||||
assert!(
|
||||
!text.starts_with("???"),
|
||||
"{:?} has no entry in main.ftl (id `{}`)",
|
||||
reason,
|
||||
reason.id()
|
||||
);
|
||||
assert!(!text.is_empty(), "{reason:?} resolves to empty copy");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn interpolated_reasons_carry_their_value() {
|
||||
assert!(Reason::PruneModeUnknown { got: "sideways".into() }
|
||||
.text()
|
||||
.contains("sideways"));
|
||||
let ts = "2026-10-01T12:47:00Z";
|
||||
assert!(Reason::Collision { timestamp: ts.into() }.text().contains(ts));
|
||||
assert!(Reason::NoRecordAt { timestamp: ts.into() }.text().contains(ts));
|
||||
assert!(Reason::ReplyToChanged { author: "a@b".into() }
|
||||
.text()
|
||||
.contains("a@b"));
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue