Libera-Chat / sable

GNU Affero General Public License v3.0
88 stars 8 forks source link

Empty trailing parameter #56

Closed progval closed 1 year ago

progval commented 1 year ago

Currently, Sable treats empty trailing parameters as if the trailing was absent:

https://github.com/Libera-Chat/sable/blob/5be1f25bd1e89bcb2f8f5de39ce96b6d0e0487cd/sable_ircd/src/client_message.rs#L81-84

This means that, like irc2, PRIVMSG #chan : returns ERR_NEEDMOREPARAMS, as opposed to Charybdis/Hybrid/Plexus4/Solanum, Ergo, InspIRCd, ircu2/Nefarious/(probably snircd too), and UnrealIRCd, which use ERR_NOTEXTTOSEND instead. (while ngIRCd prefers to silently drop the message).

It also means that, like ircu2/Nefarious/snircd MODE #chan +k : returns ERR_NEEDMOREPARAMS, as opposed to Charybdis/Hybrid/Plexus4/Solanum, Ergo, InspIRCd, irc2, and UnrealIRCd which return either ERR_INVALIDMODEPARAM or ERR_INVALIDKEY. (while ngIRCd allows the key change).

Is this intended behavior? I assume so given that it makes command handlers easier to write (in particular, no need for special handling in KICK/PART/QUIT where the lack of trailing is equivalent to an empty trailing); I'd just like to check.

progval commented 1 year ago

Ah, I got another one where it's an issue: TOPIC #chan : should empty the topic (it's the only way to do so), but on Sable it's interpreted as TOPIC #chan so it returns the current topic.

spb commented 1 year ago

This isn't a deliberate decision, more an artefact of the parser being another "get the job done so I can focus on the difficult parts" component.

The KICK/PART/QUIT use case could be handled equally well by a new wrapper type to put in the command handler's argument list, if doing it by hand looks cumbersome or repetitive.