NOTICE: Do not return error on invalid/non-existent target

like the RFCs say, and all IRCds (but InspIRCd UnrealIRCd) do

This implements PositionalArgument for `Result<T, &'a str>`, and re-uses it in
NAMES which also needs to do some special handling in case of error while
parsing its arguments

Author: progval

sable_ircd/src/command/handlers/names.rs

@@ -5,18 +5,17 @@ fn handle_names(
     server: &ClientServer,
     response: &dyn CommandResponse,
     source: UserSource,
-    channel: &str,
+    channel: Result<wrapper::Channel, &str>,
 ) -> CommandResult {
-    if let Ok(channel_name) = &ChannelName::from_str(channel) {
-        if let Ok(channel) = server.network().channel_by_name(channel_name) {
-            crate::utils::send_channel_names(server, &response, &source, &channel)?;
-            return Ok(());
+    match channel {
+        Ok(channel) => Ok(crate::utils::send_channel_names(
+            server, &response, &source, &channel,
+        )?),
+        Err(channel_name) => {
+            // "If the channel name is invalid or the channel does not exist, one RPL_ENDOFNAMES numeric
+            // containing the given channel name should be returned." -- https://modern.ircdocs.horse/#names-message
+            response.numeric(make_numeric!(EndOfNames, channel_name));
+            Ok(())
         }
     }
-
-    // "If the channel name is invalid or the channel does not exist, one RPL_ENDOFNAMES numeric
-    // containing the given channel name should be returned." -- https://modern.ircdocs.horse/#names-message
-    response.numeric(make_numeric!(EndOfNames, channel));
-
-    Ok(())
 }

sable_ircd/src/command/handlers/notice.rs

@@ -5,11 +5,35 @@ async fn handle_notice(
     server: &ClientServer,
     source: UserSource<'_>,
     cmd: &dyn Command,
-    target: TargetParameter<'_>,
+    target: Result<TargetParameter<'_>, &str>,
     msg: &str,
 ) -> CommandResult {
+    let Ok(target) = target else {
+        /* No such target. However, the spec say we should not send an error:
+         *
+         * "automatic replies must never be
+         * sent in response to a NOTICE message.  This rule applies to servers
+         * too - they must not send any error reply back to the client on
+         * receipt of a notice"
+         * -- <https://tools.ietf.org/html/rfc1459#section-4.4.2>
+         *
+         * "automatic replies MUST NEVER be sent in response to a NOTICE message.
+         * This rule applies to servers too - they MUST NOT send any error repl
+         * back to the client on receipt of a notice."
+         * -- <https://tools.ietf.org/html/rfc2812#section-3.3.2>
+         *
+         * "This rule also applies to servers – they must not send any error back
+         * to the client on receipt of a NOTICE command"
+         * -- https://modern.ircdocs.horse/#notice-message
+         *
+         * and most other servers agree with the specs, at least on non-existent
+         * channels.
+         */
+        return Ok(());
+    };
     if msg.len() == 0 {
-        return numeric_error!(NoTextToSend);
+        // Ditto
+        return Ok(());
     }
 
     if let Some(user) = target.user() {
@@ -19,7 +43,10 @@ async fn handle_notice(
         }
     }
     if let Some(channel) = target.channel() {
-        server.policy().can_send(&source, &channel, msg)?;
+        if server.policy().can_send(&source, &channel, msg).is_err() {
+            // Silent error, see above
+            return Ok(());
+        }
     }
 
     let details = event::details::NewMessage {

sable_ircd/src/command/plumbing/argument_type.rs

@@ -144,11 +144,33 @@ impl<'a> PositionalArgument<'a> for u32 {
 }
 
 impl<'a, T: PositionalArgument<'a>> PositionalArgument<'a> for Option<T> {
-    fn parse<'b>(ctx: &'a dyn Command, arg: &'b mut ArgListIter<'a>) -> Result<Self, CommandError>
+    fn parse<'b>(
+        ctx: &'a dyn Command,
+        arg_list: &'b mut ArgListIter<'a>,
+    ) -> Result<Self, CommandError>
     where
         'a: 'b,
     {
-        Ok(T::parse(ctx, arg).ok())
+        Ok(T::parse(ctx, arg_list).ok())
+    }
+
+    fn parse_str(_ctx: &'a dyn Command, _value: &'a str) -> Result<Self, CommandError> {
+        unreachable!();
+    }
+}
+
+/// Either Ok(parsed_value) or Err(raw_value) if it cannot be parsed.
+/// Never actually returns a CommandError.
+impl<'a, T: PositionalArgument<'a>> PositionalArgument<'a> for Result<T, &'a str> {
+    fn parse<'b>(
+        ctx: &'a dyn Command,
+        arg_list: &'b mut ArgListIter<'a>,
+    ) -> Result<Self, CommandError>
+    where
+        'a: 'b,
+    {
+        let s = arg_list.next().ok_or(CommandError::NotEnoughParameters)?;
+        Ok(T::parse_str(ctx, s).map_err(|_| s))
     }
 
     fn parse_str(_ctx: &'a dyn Command, _value: &'a str) -> Result<Self, CommandError> {