Merge pull request #752 from slingamn/multiline.1

remove oragono.io/maxline-2 and fmsgid
This commit is contained in:
Shivaram Lingamneni 2020-01-25 17:12:02 -08:00 committed by GitHub
commit fbfa32bf4c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 67 additions and 232 deletions

View File

@ -81,12 +81,6 @@ CAPDEFS = [
url="https://gist.github.com/DanielOaks/8126122f74b26012a3de37db80e4e0c6",
standard="proposed IRCv3",
),
CapDef(
identifier="MaxLine",
name="oragono.io/maxline-2",
url="https://oragono.io/maxline-2",
standard="Oragono-specific",
),
CapDef(
identifier="MessageTags",
name="message-tags",

View File

@ -58,7 +58,6 @@ const (
// More draft names associated with draft/multiline:
MultilineBatchType = "draft/multiline"
MultilineConcatTag = "draft/multiline-concat"
MultilineFmsgidTag = "draft/fmsgid"
)
func init() {

View File

@ -7,7 +7,7 @@ package caps
const (
// number of recognized capabilities:
numCapabs = 27
numCapabs = 26
// length of the uint64 array that represents the bitset:
bitsetLen = 1
)
@ -89,10 +89,6 @@ const (
// https://oragono.io/bnc
Bouncer Capability = iota
// MaxLine is the Oragono-specific capability named "oragono.io/maxline-2":
// https://oragono.io/maxline-2
MaxLine Capability = iota
// Nope is the Oragono vendor capability named "oragono.io/nope":
// https://oragono.io/nope
Nope Capability = iota
@ -144,7 +140,6 @@ var (
"message-tags",
"multi-prefix",
"oragono.io/bnc",
"oragono.io/maxline-2",
"oragono.io/nope",
"sasl",
"server-time",

View File

@ -643,7 +643,7 @@ func (channel *Channel) Join(client *Client, key string, isSajoin bool, rb *Resp
channel.regenerateMembersCache()
message = utils.MakeSplitMessage("", true)
message = utils.MakeMessage("")
histItem := history.Item{
Type: history.Join,
Nick: details.nickMask,
@ -766,7 +766,7 @@ func (channel *Channel) Part(client *Client, message string, rb *ResponseBuffer)
channel.Quit(client)
splitMessage := utils.MakeSplitMessage(message, true)
splitMessage := utils.MakeMessage(message)
details := client.Details()
params := make([]string, 1, 2)
@ -1233,7 +1233,7 @@ func (channel *Channel) Kick(client *Client, target *Client, comment string, rb
comment = comment[:kicklimit]
}
message := utils.MakeSplitMessage(comment, true)
message := utils.MakeMessage(comment)
clientMask := client.NickMaskString()
clientAccount := client.AccountName()

View File

@ -112,7 +112,6 @@ type Session struct {
quitMessage string
capabilities caps.Set
maxlenRest uint32
capState caps.State
capVersion caps.Version
@ -148,21 +147,6 @@ func (sd *Session) SetQuitMessage(message string) (set bool) {
}
}
// set the negotiated message length based on session capabilities
func (session *Session) SetMaxlenRest() {
maxlenRest := 512
if session.capabilities.Has(caps.MaxLine) {
maxlenRest = session.client.server.Config().Limits.LineLen.Rest
}
atomic.StoreUint32(&session.maxlenRest, uint32(maxlenRest))
}
// allow the negotiated message length limit to be read without locks; this is a convenience
// so that Session.SendRawMessage doesn't have to acquire any Client locks
func (session *Session) MaxlenRest() int {
return int(atomic.LoadUint32(&session.maxlenRest))
}
// returns whether the session was actively destroyed (for example, by ping
// timeout or NS GHOST).
// avoids a race condition between asynchronous idle-timing-out of sessions,
@ -240,9 +224,8 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) {
now := time.Now().UTC()
config := server.Config()
fullLineLenLimit := ircmsg.MaxlenTagsFromClient + config.Limits.LineLen.Rest
// give them 1k of grace over the limit:
socket := NewSocket(conn.Conn, fullLineLenLimit+1024, config.Server.MaxSendQBytes)
socket := NewSocket(conn.Conn, ircmsg.MaxlenTagsFromClient+512+1024, config.Server.MaxSendQBytes)
client := &Client{
atime: now,
channels: make(ChannelSet),
@ -271,7 +254,6 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) {
atime: now,
realIP: realIP,
}
session.SetMaxlenRest()
client.sessions = []*Session{session}
if conn.Config.TLSConfig != nil {
@ -493,8 +475,6 @@ func (client *Client) run(session *Session, proxyLine string) {
firstLine := !isReattach
for {
maxlenRest := session.MaxlenRest()
var line string
var err error
if proxyLine == "" {
@ -545,7 +525,7 @@ func (client *Client) run(session *Session, proxyLine string) {
}
}
msg, err := ircmsg.ParseLineStrict(line, true, maxlenRest)
msg, err := ircmsg.ParseLineStrict(line, true, 512)
if err == ircmsg.ErrorLineIsEmpty {
continue
} else if err == ircmsg.ErrorLineTooLong {
@ -1161,7 +1141,7 @@ func (client *Client) destroy(session *Session) {
// clean up monitor state
client.server.monitorManager.RemoveAll(client)
splitQuitMessage := utils.MakeSplitMessage(quitMessage, true)
splitQuitMessage := utils.MakeMessage(quitMessage)
// clean up channels
// (note that if this is a reattach, client has no channels and therefore no friends)
friends := make(ClientSet)
@ -1216,7 +1196,8 @@ func (client *Client) destroy(session *Session) {
// SendSplitMsgFromClient sends an IRC PRIVMSG/NOTICE coming from a specific client.
// Adds account-tag to the line as well.
func (session *Session) sendSplitMsgFromClientInternal(blocking bool, nickmask, accountName string, tags map[string]string, command, target string, message utils.SplitMessage) {
if message.Is512() || session.capabilities.Has(caps.MaxLine) {
// TODO no maxline support
if message.Is512() {
session.sendFromClientInternal(blocking, message.Time, message.Msgid, nickmask, accountName, tags, command, target, message.Message)
} else {
if message.IsMultiline() && session.capabilities.Has(caps.Multiline) {
@ -1224,8 +1205,12 @@ func (session *Session) sendSplitMsgFromClientInternal(blocking bool, nickmask,
session.SendRawMessage(msg, blocking)
}
} else {
for _, messagePair := range message.Wrapped {
session.sendFromClientInternal(blocking, message.Time, messagePair.Msgid, nickmask, accountName, tags, command, target, messagePair.Message)
for i, messagePair := range message.Split {
var msgid string
if i == 0 {
msgid = message.Msgid
}
session.sendFromClientInternal(blocking, message.Time, msgid, nickmask, accountName, tags, command, target, messagePair.Message)
}
}
}
@ -1268,10 +1253,9 @@ func (session *Session) composeMultilineBatch(fromNickMask, fromAccount string,
}
result = append(result, batchStart)
for _, msg := range message.Wrapped {
for _, msg := range message.Split {
message := ircmsg.MakeMessage(nil, fromNickMask, command, target, msg.Message)
message.SetTag("batch", batchID)
message.SetTag(caps.MultilineFmsgidTag, msg.Msgid)
if msg.Concat {
message.SetTag(caps.MultilineConcatTag, "")
}
@ -1310,8 +1294,7 @@ func (session *Session) SendRawMessage(message ircmsg.IrcMessage, blocking bool)
}
// assemble message
maxlenRest := session.MaxlenRest()
line, err := message.LineBytesStrict(false, maxlenRest)
line, err := message.LineBytesStrict(false, 512)
if err != nil {
logline := fmt.Sprintf("Error assembling message for sending: %v\n%s", err, debug.Stack())
session.client.server.logger.Error("internal", logline)

View File

@ -237,24 +237,18 @@ type OperConfig struct {
Modes string
}
// LineLenConfig controls line lengths.
type LineLenLimits struct {
Rest int
}
// Various server-enforced limits on data size.
type Limits struct {
AwayLen int `yaml:"awaylen"`
ChanListModes int `yaml:"chan-list-modes"`
ChannelLen int `yaml:"channellen"`
IdentLen int `yaml:"identlen"`
KickLen int `yaml:"kicklen"`
LineLen LineLenLimits `yaml:"linelen"`
MonitorEntries int `yaml:"monitor-entries"`
NickLen int `yaml:"nicklen"`
TopicLen int `yaml:"topiclen"`
WhowasEntries int `yaml:"whowas-entries"`
RegistrationMessages int `yaml:"registration-messages"`
AwayLen int `yaml:"awaylen"`
ChanListModes int `yaml:"chan-list-modes"`
ChannelLen int `yaml:"channellen"`
IdentLen int `yaml:"identlen"`
KickLen int `yaml:"kicklen"`
MonitorEntries int `yaml:"monitor-entries"`
NickLen int `yaml:"nicklen"`
TopicLen int `yaml:"topiclen"`
WhowasEntries int `yaml:"whowas-entries"`
RegistrationMessages int `yaml:"registration-messages"`
Multiline struct {
MaxBytes int `yaml:"max-bytes"`
MaxLines int `yaml:"max-lines"`
@ -671,7 +665,9 @@ func LoadConfig(filename string) (config *Config, err error) {
return nil, fmt.Errorf("STS port is incorrect, should be 0 if disabled: %d", config.Server.STS.Port)
}
if config.Server.STS.STSOnlyBanner != "" {
config.Server.STS.bannerLines = utils.WordWrap(config.Server.STS.STSOnlyBanner, 400)
for _, line := range strings.Split(config.Server.STS.STSOnlyBanner, "\n") {
config.Server.STS.bannerLines = append(config.Server.STS.bannerLines, strings.TrimSpace(line))
}
} else {
config.Server.STS.bannerLines = []string{fmt.Sprintf("This server is only accessible over TLS. Please reconnect using TLS on port %d.", config.Server.STS.Port)}
}
@ -705,16 +701,6 @@ func LoadConfig(filename string) (config *Config, err error) {
}
config.Server.WebIRC = newWebIRC
// process limits
if config.Limits.LineLen.Rest < 512 {
config.Limits.LineLen.Rest = 512
}
if config.Limits.LineLen.Rest == 512 {
config.Server.supportedCaps.Disable(caps.MaxLine)
} else {
config.Server.capValues[caps.MaxLine] = strconv.Itoa(config.Limits.LineLen.Rest)
}
if config.Limits.Multiline.MaxBytes <= 0 {
config.Server.supportedCaps.Disable(caps.Multiline)
} else {

View File

@ -352,8 +352,6 @@ func batchHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res
} else {
rb.session.batch.target = msg.Params[2]
// save the response label for later
// XXX changing the label inside a handler is a bit dodgy, but it works here
// because there's no way we could have triggered a flush up to this point
rb.session.batch.responseLabel = rb.Label
rb.Label = ""
}
@ -366,13 +364,15 @@ func batchHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res
} else {
batch := rb.session.batch
rb.session.batch = MultilineBatch{}
batch.message.Time = time.Now().UTC()
// time tag should correspond to the time when the message was completed
batch.message.SetTime()
histType, err := msgCommandToHistType(batch.command)
if err != nil {
histType = history.Privmsg
batch.command = "PRIVMSG"
}
// see previous caution about modifying ResponseBuffer.Label
// XXX changing the label inside a handler is a bit dodgy, but it works here
// because there's no way we could have triggered a flush up to this point
rb.Label = batch.responseLabel
dispatchMessageToTarget(client, batch.tags, histType, batch.command, batch.target, batch.message, rb)
}
@ -515,10 +515,6 @@ func capHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo
rb.session.SetResumeID(id)
}
}
// update maxlenrest, just in case they altered the maxline cap
rb.session.SetMaxlenRest()
case "END":
if !client.registered {
rb.session.capState = caps.NegotiatedState
@ -1962,7 +1958,7 @@ func messageHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *R
break
}
// each target gets distinct msgids
splitMsg := utils.MakeSplitMessage(message, !rb.session.capabilities.Has(caps.MaxLine))
splitMsg := utils.MakeMessage(message)
dispatchMessageToTarget(client, clientOnlyTags, histType, msg.Command, targetString, splitMsg, rb)
}
return false

View File

@ -50,15 +50,7 @@ type Item struct {
// HasMsgid tests whether a message has the message id `msgid`.
func (item *Item) HasMsgid(msgid string) bool {
if item.Message.Msgid == msgid {
return true
}
for _, pair := range item.Message.Wrapped {
if pair.Msgid == msgid {
return true
}
}
return false
return item.Message.Msgid == msgid
}
func (item *Item) isStorable() bool {

View File

@ -57,7 +57,7 @@ func performNickChange(server *Server, client *Client, target *Client, session *
return false
}
message := utils.MakeSplitMessage("", true)
message := utils.MakeMessage("")
histItem := history.Item{
Type: history.Nick,
Nick: origNickMask,

View File

@ -118,7 +118,7 @@ func (rb *ResponseBuffer) AddFromClient(time time.Time, msgid string, fromNickMa
// AddSplitMessageFromClient adds a new split message from a specific client to our queue.
func (rb *ResponseBuffer) AddSplitMessageFromClient(fromNickMask string, fromAccount string, tags map[string]string, command string, target string, message utils.SplitMessage) {
if message.Is512() || rb.session.capabilities.Has(caps.MaxLine) {
if message.Is512() {
rb.AddFromClient(message.Time, message.Msgid, fromNickMask, fromAccount, tags, command, target, message.Message)
} else {
if message.IsMultiline() && rb.session.capabilities.Has(caps.Multiline) {
@ -127,8 +127,12 @@ func (rb *ResponseBuffer) AddSplitMessageFromClient(fromNickMask string, fromAcc
rb.setNestedBatchTag(&batch[len(batch)-1])
rb.messages = append(rb.messages, batch...)
} else {
for _, messagePair := range message.Wrapped {
rb.AddFromClient(message.Time, messagePair.Msgid, fromNickMask, fromAccount, tags, command, target, messagePair.Message)
for i, messagePair := range message.Split {
var msgid string
if i == 0 {
msgid = message.Msgid
}
rb.AddFromClient(message.Time, msgid, fromNickMask, fromAccount, tags, command, target, messagePair.Message)
}
}
}

View File

@ -564,10 +564,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) {
globalCasemappingSetting = config.Server.Casemapping
} else {
// enforce configs that can't be changed after launch:
currentLimits := server.Config().Limits
if currentLimits.LineLen.Rest != config.Limits.LineLen.Rest {
return fmt.Errorf("Maximum line length (linelen) cannot be changed after launching the server, rehash aborted")
} else if server.name != config.Server.Name {
if server.name != config.Server.Name {
return fmt.Errorf("Server name cannot be changed after launching the server, rehash aborted")
} else if server.Config().Datastore.Path != config.Datastore.Path {
return fmt.Errorf("Datastore path cannot be changed after launching the server, rehash aborted")

View File

@ -15,89 +15,29 @@ func IsRestrictedCTCPMessage(message string) bool {
return strings.HasPrefix(message, "\x01") && !strings.HasPrefix(message, "\x01ACTION")
}
// WordWrap wraps the given text into a series of lines that don't exceed lineWidth characters.
func WordWrap(text string, lineWidth int) []string {
var lines []string
var cacheLine, cacheWord bytes.Buffer
for _, char := range text {
if char == '\r' {
continue
} else if char == '\n' {
cacheLine.Write(cacheWord.Bytes())
lines = append(lines, cacheLine.String())
cacheWord.Reset()
cacheLine.Reset()
} else if (char == ' ' || char == '-') && cacheLine.Len()+cacheWord.Len()+1 < lineWidth {
// natural word boundary
cacheLine.Write(cacheWord.Bytes())
cacheLine.WriteRune(char)
cacheWord.Reset()
} else if lineWidth <= cacheLine.Len()+cacheWord.Len()+1 {
// time to wrap to next line
if cacheLine.Len() < (lineWidth / 2) {
// this word takes up more than half a line... just split in the middle of the word
cacheLine.Write(cacheWord.Bytes())
cacheLine.WriteRune(char)
cacheWord.Reset()
} else {
cacheWord.WriteRune(char)
}
lines = append(lines, cacheLine.String())
cacheLine.Reset()
} else {
// normal character
cacheWord.WriteRune(char)
}
}
if 0 < cacheWord.Len() {
cacheLine.Write(cacheWord.Bytes())
}
if 0 < cacheLine.Len() {
lines = append(lines, cacheLine.String())
}
return lines
}
type MessagePair struct {
Message string
Msgid string
Concat bool // should be relayed with the multiline-concat tag
}
// SplitMessage represents a message that's been split for sending.
// Three possibilities:
// Two possibilities:
// (a) Standard message that can be relayed on a single 512-byte line
// (MessagePair contains the message, Wrapped == nil)
// (b) oragono.io/maxline-2 message that was split on the server side
// (MessagePair contains the unsplit message, Wrapped contains the split lines)
// (c) multiline message that was split on the client side
// (MessagePair is zero, Wrapped contains the split lines)
// (b) multiline message that was split on the client side
// (Message == "", Wrapped contains the split lines)
type SplitMessage struct {
MessagePair
Wrapped []MessagePair // if this is nil, `Message` didn't need wrapping and can be sent to anyone
Message string
Msgid string
Split []MessagePair
Time time.Time
}
const defaultLineWidth = 400
func MakeSplitMessage(original string, origIs512 bool) (result SplitMessage) {
func MakeMessage(original string) (result SplitMessage) {
result.Message = original
result.Msgid = GenerateSecretToken()
result.Time = time.Now().UTC()
if !origIs512 && defaultLineWidth < len(original) {
wrapped := WordWrap(original, defaultLineWidth)
result.Wrapped = make([]MessagePair, len(wrapped))
for i, wrappedMessage := range wrapped {
result.Wrapped[i] = MessagePair{
Message: wrappedMessage,
Msgid: GenerateSecretToken(),
}
}
}
return
}
@ -105,30 +45,33 @@ func (sm *SplitMessage) Append(message string, concat bool) {
if sm.Msgid == "" {
sm.Msgid = GenerateSecretToken()
}
sm.Wrapped = append(sm.Wrapped, MessagePair{
sm.Split = append(sm.Split, MessagePair{
Message: message,
Msgid: GenerateSecretToken(),
Concat: concat,
})
}
func (sm *SplitMessage) SetTime() {
sm.Time = time.Now().UTC()
}
func (sm *SplitMessage) LenLines() int {
if sm.Wrapped == nil {
if (sm.MessagePair == MessagePair{}) {
if sm.Split == nil {
if sm.Message == "" {
return 0
} else {
return 1
}
}
return len(sm.Wrapped)
return len(sm.Split)
}
func (sm *SplitMessage) LenBytes() (result int) {
if sm.Wrapped == nil {
if sm.Split == nil {
return len(sm.Message)
}
for i := 0; i < len(sm.Wrapped); i++ {
result += len(sm.Wrapped[i].Message)
for i := 0; i < len(sm.Split); i++ {
result += len(sm.Split[i].Message)
}
return
}
@ -137,8 +80,8 @@ func (sm *SplitMessage) IsRestrictedCTCPMessage() bool {
if IsRestrictedCTCPMessage(sm.Message) {
return true
}
for i := 0; i < len(sm.Wrapped); i++ {
if IsRestrictedCTCPMessage(sm.Wrapped[i].Message) {
for i := 0; i < len(sm.Split); i++ {
if IsRestrictedCTCPMessage(sm.Split[i].Message) {
return true
}
}
@ -146,11 +89,11 @@ func (sm *SplitMessage) IsRestrictedCTCPMessage() bool {
}
func (sm *SplitMessage) IsMultiline() bool {
return sm.Message == "" && len(sm.Wrapped) != 0
return sm.Message == "" && len(sm.Split) != 0
}
func (sm *SplitMessage) Is512() bool {
return sm.Message != "" && sm.Wrapped == nil
return sm.Message != ""
}
// TokenLineBuilder is a helper for building IRC lines composed of delimited tokens,

View File

@ -4,61 +4,14 @@
package utils
import (
"reflect"
"strings"
"testing"
)
const (
threeMusketeers = "In the meantime DArtagnan, who had plunged into a bypath, continued his route and reached St. Cloud; but instead of following the main street he turned behind the château, reached a sort of retired lane, and found himself soon in front of the pavilion named. It was situated in a very private spot. A high wall, at the angle of which was the pavilion, ran along one side of this lane, and on the other was a little garden connected with a poor cottage which was protected by a hedge from passers-by."
monteCristo = `Both the count and Baptistin had told the truth when they announced to Morcerf the proposed visit of the major, which had served Monte Cristo as a pretext for declining Albert's invitation. Seven o'clock had just struck, and M. Bertuccio, according to the command which had been given him, had two hours before left for Auteuil, when a cab stopped at the door, and after depositing its occupant at the gate, immediately hurried away, as if ashamed of its employment. The visitor was about fifty-two years of age, dressed in one of the green surtouts, ornamented with black frogs, which have so long maintained their popularity all over Europe. He wore trousers of blue cloth, boots tolerably clean, but not of the brightest polish, and a little too thick in the soles, buckskin gloves, a hat somewhat resembling in shape those usually worn by the gendarmes, and a black cravat striped with white, which, if the proprietor had not worn it of his own free will, might have passed for a halter, so much did it resemble one. Such was the picturesque costume of the person who rang at the gate, and demanded if it was not at No. 30 in the Avenue des Champs-Elysees that the Count of Monte Cristo lived, and who, being answered by the porter in the affirmative, entered, closed the gate after him, and began to ascend the steps.`
)
func assertWrapCorrect(text string, lineWidth int, allowSplitWords bool, t *testing.T) {
lines := WordWrap(text, lineWidth)
reconstructed := strings.Join(lines, "")
if text != reconstructed {
t.Errorf("text %v does not match original %v", text, reconstructed)
}
for _, line := range lines {
if len(line) > lineWidth {
t.Errorf("line too long: %d, %v", len(line), line)
}
}
if !allowSplitWords {
origWords := strings.Fields(text)
var newWords []string
for _, line := range lines {
newWords = append(newWords, strings.Fields(line)...)
}
if !reflect.DeepEqual(origWords, newWords) {
t.Errorf("words %v do not match wrapped words %v", origWords, newWords)
}
}
}
func TestWordWrap(t *testing.T) {
assertWrapCorrect("jackdaws love my big sphinx of quartz", 12, false, t)
// long word that will necessarily be split:
assertWrapCorrect("jackdawslovemybigsphinxofquartz", 12, true, t)
assertWrapCorrect(threeMusketeers, 40, true, t)
assertWrapCorrect(monteCristo, 20, false, t)
}
func BenchmarkWordWrap(b *testing.B) {
for i := 0; i < b.N; i++ {
WordWrap(threeMusketeers, 40)
WordWrap(monteCristo, 60)
}
}
func TestTokenLineBuilder(t *testing.T) {
lineLen := 400
var tl TokenLineBuilder

View File

@ -586,13 +586,6 @@ limits:
# maximum length of channel lists (beI modes)
chan-list-modes: 60
# maximum length of IRC lines
# this should generally be 1024-2048, and will only apply when negotiated by clients
linelen:
# ratified version of the message-tags cap fixes the max tag length at 8191 bytes
# configurable length for the rest of the message:
rest: 2048
# maximum number of messages to accept during registration (prevents
# DoS / resource exhaustion attacks):
registration-messages: 1024