Merge pull request #1586 from slingamn/issue1577_no_truncation.2

deprecate message truncation

default.yaml

@@ -235,6 +235,13 @@ server:
         # this works around that bug, allowing them to use SASL.
         send-unprefixed-sasl: true
 
+        # traditionally, IRC servers will truncate and send messages that are
+        # too long to be relayed intact. this behavior can be disabled by setting
+        # allow-truncation to false, in which case Oragono will reject the message
+        # and return an error to the client. (note that this option defaults to true
+        # when unset.)
+        allow-truncation: false
+
     # IP-based DoS protection
     ip-limits:
         # whether to limit the total number of concurrent connections per IP/CIDR

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/go-sql-driver/mysql v1.5.0
 	github.com/go-test/deep v1.0.6 // indirect
 	github.com/gorilla/websocket v1.4.2
-	github.com/goshuirc/irc-go v0.0.0-20210301225436-2c4b83d64847
+	github.com/goshuirc/irc-go v0.0.0-20210304031553-cf78e9176f96
 	github.com/onsi/ginkgo v1.12.0 // indirect
 	github.com/onsi/gomega v1.9.0 // indirect
 	github.com/oragono/confusables v0.0.0-20201108231250-4ab98ab61fb1
@@ -24,4 +24,4 @@ require (
 	gopkg.in/yaml.v2 v2.3.0
 )
 
-replace github.com/gorilla/websocket  => github.com/oragono/websocket v1.4.2-oragono1
+replace github.com/gorilla/websocket => github.com/oragono/websocket v1.4.2-oragono1

go.sum

@@ -40,6 +40,8 @@ github.com/goshuirc/irc-go v0.0.0-20210223005429-8d38e43fc6ed h1:cwwqHrmLafgEucS
 github.com/goshuirc/irc-go v0.0.0-20210223005429-8d38e43fc6ed/go.mod h1:q/JhvvKLmif3y9q8MDQM+gRCnjEKnu5ClF298TTXJug=
 github.com/goshuirc/irc-go v0.0.0-20210301225436-2c4b83d64847 h1:MmsZRpAsMxyw0P5/SFn2L6edhmIXRlolgXvOF+fgEiQ=
 github.com/goshuirc/irc-go v0.0.0-20210301225436-2c4b83d64847/go.mod h1:q/JhvvKLmif3y9q8MDQM+gRCnjEKnu5ClF298TTXJug=
+github.com/goshuirc/irc-go v0.0.0-20210304031553-cf78e9176f96 h1:sihI3HsrJWyS4MtBmxh5W4gDZD34SWodkWyUvJltswY=
+github.com/goshuirc/irc-go v0.0.0-20210304031553-cf78e9176f96/go.mod h1:q/JhvvKLmif3y9q8MDQM+gRCnjEKnu5ClF298TTXJug=
 github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
 github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=

irc/channel.go

@@ -1361,6 +1361,15 @@ func (channel *Channel) SendSplitMessage(command string, minPrefixMode modes.Mod
 	details := client.Details()
 	chname := channel.Name()
 
+	if !client.server.Config().Server.Compatibility.allowTruncation {
+		if !validateSplitMessageLen(histType, details.nickMask, chname, message) {
+			rb.Add(nil, client.server.name, ERR_INPUTTOOLONG, details.nick, client.t("Line too long to be relayed without truncation"))
+			// TODO(#1577) remove this logline:
+			client.server.logger.Debug("internal", "rejected truncation-requiring DM from client", details.nick)
+			return
+		}
+	}
+
 	// STATUSMSG targets are prefixed with the supplied min-prefix, e.g., @#channel
 	if minPrefixMode != modes.Mode(0) {
 		chname = fmt.Sprintf("%s%s", modes.ChannelModePrefixes[minPrefixMode], chname)

irc/client.go

@@ -740,8 +740,13 @@ func (client *Client) run(session *Session) {
 		msg, err := ircmsg.ParseLineStrict(line, true, MaxLineLen)
 		if err == ircmsg.ErrorLineIsEmpty {
 			continue
-		} else if err == ircmsg.ErrorLineTooLong {
+		} else if err == ircmsg.ErrorTagsTooLong {
+			session.Send(nil, client.server.name, ERR_INPUTTOOLONG, client.Nick(), client.t("Input line contained excess tag data"))
+			continue
+		} else if err == ircmsg.ErrorBodyTooLong && !client.server.Config().Server.Compatibility.allowTruncation {
 			session.Send(nil, client.server.name, ERR_INPUTTOOLONG, client.Nick(), client.t("Input line too long"))
+			// TODO(#1577) remove this logline:
+			client.server.logger.Debug("internal", "rejected MaxLineLen-exceeding line from client", client.Nick())
 			continue
 		} else if err != nil {
 			client.Quit(client.t("Received malformed line"), session)
@@ -1711,7 +1716,7 @@ func (session *Session) SendRawMessage(message ircmsg.IRCMessage, blocking bool)
 
 	// assemble message
 	line, err := message.LineBytesStrict(false, MaxLineLen)
-	if err != nil {
+	if !(err == nil || err == ircmsg.ErrorBodyTooLong) {
 		errorParams := []string{"Error assembling message for sending", err.Error(), message.Command}
 		errorParams = append(errorParams, message.Params...)
 		session.client.server.logger.Error("internal", errorParams...)

irc/config.go

@@ -569,7 +569,9 @@ type Config struct {
 		Compatibility        struct {
 			ForceTrailing      *bool `yaml:"force-trailing"`
 			forceTrailing      bool
-			SendUnprefixedSasl bool `yaml:"send-unprefixed-sasl"`
+			SendUnprefixedSasl bool  `yaml:"send-unprefixed-sasl"`
+			AllowTruncation    *bool `yaml:"allow-truncation"`
+			allowTruncation    bool
 		}
 		isupport                 isupport.List
 		IPLimits                 connection_limits.LimiterConfig `yaml:"ip-limits"`
@@ -1378,6 +1380,7 @@ func LoadConfig(filename string) (config *Config, err error) {
 	}
 
 	config.Server.Compatibility.forceTrailing = utils.BoolDefaultTrue(config.Server.Compatibility.ForceTrailing)
+	config.Server.Compatibility.allowTruncation = utils.BoolDefaultTrue(config.Server.Compatibility.AllowTruncation)
 
 	config.loadMOTD()
 

irc/handlers.go

@@ -1981,6 +1981,43 @@ func nickHandler(server *Server, client *Client, msg ircmsg.IRCMessage, rb *Resp
 	return false
 }
 
+// check whether a PRIVMSG or NOTICE is too long to be relayed without truncation
+func validateLineLen(msgType history.ItemType, source, target, payload string) (ok bool) {
+	// :source PRIVMSG #target :payload\r\n
+	// 1: initial colon on prefix
+	// 1: space between prefix and command
+	// 1: space between command and target (first parameter)
+	// 1: space between target and payload (second parameter)
+	// 1: colon to send the payload as a trailing (we force trailing for PRIVMSG and NOTICE)
+	// 2: final \r\n
+	limit := MaxLineLen - 7
+	limit -= len(source)
+	switch msgType {
+	case history.Privmsg:
+		limit -= 7
+	case history.Notice:
+		limit -= 6
+	default:
+		return true
+	}
+	limit -= len(payload)
+	return limit >= 0
+}
+
+// check validateLineLen for an entire SplitMessage (which may consist of multiple lines)
+func validateSplitMessageLen(msgType history.ItemType, source, target string, message utils.SplitMessage) (ok bool) {
+	if message.Is512() {
+		return validateLineLen(msgType, source, target, message.Message)
+	} else {
+		for _, messagePair := range message.Split {
+			if !validateLineLen(msgType, source, target, messagePair.Message) {
+				return false
+			}
+		}
+		return true
+	}
+}
+
 // helper to store a batched PRIVMSG in the session object
 func absorbBatchedMessage(server *Server, client *Client, msg ircmsg.IRCMessage, batchTag string, histType history.ItemType, rb *ResponseBuffer) {
 	var errorCode, errorMessage string
@@ -2033,7 +2070,6 @@ func messageHandler(server *Server, client *Client, msg ircmsg.IRCMessage, rb *R
 		return false
 	}
 
-	cnick := client.Nick()
 	clientOnlyTags := msg.ClientOnlyTags()
 	if histType == history.Tagmsg && len(clientOnlyTags) == 0 {
 		// nothing to do
@@ -2046,7 +2082,7 @@ func messageHandler(server *Server, client *Client, msg ircmsg.IRCMessage, rb *R
 		message = msg.Params[1]
 	}
 	if histType != history.Tagmsg && message == "" {
-		rb.Add(nil, server.name, ERR_NOTEXTTOSEND, cnick, client.t("No text to send"))
+		rb.Add(nil, server.name, ERR_NOTEXTTOSEND, client.Nick(), client.t("No text to send"))
 		return false
 	}
 
@@ -2147,6 +2183,14 @@ func dispatchMessageToTarget(client *Client, tags map[string]string, histType hi
 			rb.Add(nil, server.name, ERR_NEEDREGGEDNICK, client.Nick(), tnick, client.t("You must be registered to send a direct message to this user"))
 			return
 		}
+		if !client.server.Config().Server.Compatibility.allowTruncation {
+			if !validateSplitMessageLen(histType, client.NickMaskString(), tnick, message) {
+				rb.Add(nil, server.name, ERR_INPUTTOOLONG, client.Nick(), client.t("Line too long to be relayed without truncation"))
+				// TODO(#1577) remove this logline:
+				client.server.logger.Debug("internal", "rejected truncation-requiring channel message from client", details.nick)
+				return
+			}
+		}
 		nickMaskString := details.nickMask
 		accountName := details.accountName
 		var deliverySessions []*Session

irc/message_cache.go

@@ -54,7 +54,7 @@ func addAllTags(msg *ircmsg.IRCMessage, tags map[string]string, serverTime time.
 }
 
 func (m *MessageCache) handleErr(server *Server, err error) bool {
-	if err != nil {
+	if !(err == nil || err == ircmsg.ErrorBodyTooLong) {
 		server.logger.Error("internal", "Error assembling message for sending", err.Error())
 		// blank these out so Send will be a no-op
 		m.fullTags = nil

traditional.yaml

@@ -208,6 +208,13 @@ server:
         # this works around that bug, allowing them to use SASL.
         send-unprefixed-sasl: true
 
+        # traditionally, IRC servers will truncate and send messages that are
+        # too long to be relayed intact. this behavior can be disabled by setting
+        # allow-truncation to false, in which case Oragono will reject the message
+        # and return an error to the client. (note that this option defaults to true
+        # when unset.)
+        allow-truncation: true
+
     # IP-based DoS protection
     ip-limits:
         # whether to limit the total number of concurrent connections per IP/CIDR

vendor/modules.txt

@@ -21,7 +21,7 @@ github.com/go-sql-driver/mysql
 # github.com/gorilla/websocket v1.4.2 => github.com/oragono/websocket v1.4.2-oragono1
 ## explicit
 github.com/gorilla/websocket
-# github.com/goshuirc/irc-go v0.0.0-20210301225436-2c4b83d64847
+# github.com/goshuirc/irc-go v0.0.0-20210304031553-cf78e9176f96
 ## explicit
 github.com/goshuirc/irc-go/ircfmt
 github.com/goshuirc/irc-go/ircmsg