Skip to content

Commit

Permalink
Split messages (#43)
Browse files Browse the repository at this point in the history
All of the below should apply:
1. Input limit uses multi-byte check. One thing I'm not sure about is if servers that provide their limits are based off runes, or just based off bytes, but the current implementation uses rune length (to a single multi-byte character counts as single character).
2. Tries to keep colors/backgrounds/color resets/etc between splits. It's not perfect, but it should work "good enough".
3. If a very long single word is split, it will prefer to keep it together (but on another line) rather than splitting off small chunks at the end (e.g. URLs).
4. Default is word spliting, and if a word is above 30 characters, I try to split be special characters (e.g. `lalkmadlkmasdlka-adasdasdasdasd` would split at `-`).
5. Worst case, it splits at the exact point necessary to make it fit, if none of the above apply and the entire word is above the maximum length.

As far as logic changes:
1. girc sets a default max line length of **510**, but some of this is subtracted with prefix padding, user length, host length, etc. Some of the subtractions aren't smart checks, just "assume a host will be this long" rather than keeping track of the actual sizes. There might be room for improvement on this item.
2. Servers can raise this limit with ISUPPORT or similar -- girc should respect that raised limit.
3. During the connection `Send` method, we invoke `Event.split` -- note that not all events are split (only `PRIVMSG` and `NOTICE` for now, `JOIN`'s have always had custom logic to split already). Could apply the logic to all commands, but there are some commands that **will not support splitting**, so prefer to be conservative for now.

Also merged in changes from branch `bugfix/issue-50`, which includes the swap to a ctx group for keeping track of the core handlers (ping/read/send/etc).
  • Loading branch information
lrstanley committed May 3, 2023
1 parent fefe1ca commit 658eba5
Show file tree
Hide file tree
Showing 24 changed files with 743 additions and 209 deletions.
42 changes: 42 additions & 0 deletions builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,48 @@ func handleISUPPORT(c *Client, e Event) {
c.state.serverOptions[name] = val
}
c.state.Unlock()

// Check for max line/nick/user/host lengths here.
c.state.RLock()
maxLineLength := c.state.maxLineLength
c.state.RUnlock()
maxNickLength := defaultNickLength
maxUserLength := defaultUserLength
maxHostLength := defaultHostLength

var ok bool
var tmp int

if tmp, ok = c.GetServerOptionInt("LINELEN"); ok {
maxLineLength = tmp
c.state.Lock()
c.state.maxLineLength = maxTagLength - 2 // -2 for CR-LF.
c.state.Unlock()
}

if tmp, ok = c.GetServerOptionInt("NICKLEN"); ok {
maxNickLength = tmp
}
if tmp, ok = c.GetServerOptionInt("MAXNICKLEN"); ok && tmp > maxNickLength {
maxNickLength = tmp
}
if tmp, ok = c.GetServerOptionInt("USERLEN"); ok && tmp > maxUserLength {
maxUserLength = tmp
}
if tmp, ok = c.GetServerOptionInt("HOSTLEN"); ok && tmp > maxHostLength {
maxHostLength = tmp
}

prefixLen := defaultPrefixPadding + maxNickLength + maxUserLength + maxHostLength
if prefixLen >= maxLineLength {
// Give up and go with defaults.
c.state.notify(c, UPDATE_GENERAL)
return
}
c.state.Lock()
c.state.maxPrefixLength = prefixLen
c.state.Unlock()

c.state.notify(c, UPDATE_GENERAL)
}

Expand Down
4 changes: 2 additions & 2 deletions cap.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ func handleCAP(c *Client, e Event) {
}

if isError {
c.rx <- &Event{Command: ERROR, Params: []string{
c.receive(&Event{Command: ERROR, Params: []string{
fmt.Sprintf("closing connection: strict transport policy provided by server is invalid; possible MITM? config: %#v", sts),
}}
}})
return
}

Expand Down
6 changes: 3 additions & 3 deletions cap_sasl.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ func handleSASL(c *Client, e Event) {
// some reason. The SASL spec and IRCv3 spec do not define a clear
// way to abort a SASL exchange, other than to disconnect, or proceed
// with CAP END.
c.rx <- &Event{Command: ERROR, Params: []string{
c.receive(&Event{Command: ERROR, Params: []string{
fmt.Sprintf("closing connection: SASL %s failed: %s", c.Config.SASL.Method(), e.Last()),
}}
}})
return
}

Expand Down Expand Up @@ -131,5 +131,5 @@ func handleSASLError(c *Client, e Event) {
// Authentication failed. The SASL spec and IRCv3 spec do not define a
// clear way to abort a SASL exchange, other than to disconnect, or
// proceed with CAP END.
c.rx <- &Event{Command: ERROR, Params: []string{"closing connection: " + e.Last()}}
c.receive(&Event{Command: ERROR, Params: []string{"closing connection: " + e.Last()}})
}
7 changes: 5 additions & 2 deletions cap_tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ type Tags map[string]string

// ParseTags parses out the key-value map of tags. raw should only be the tag
// data, not a full message. For example:
// @aaa=bbb;ccc;example.com/ddd=eee
//
// @aaa=bbb;ccc;example.com/ddd=eee
//
// NOT:
// @aaa=bbb;ccc;example.com/ddd=eee :nick!ident@host.com PRIVMSG me :Hello
//
// @aaa=bbb;ccc;example.com/ddd=eee :nick!ident@host.com PRIVMSG me :Hello
//
// Technically, there is a length limit of 4096, but the server should reject
// tag messages longer than this.
Expand Down
81 changes: 65 additions & 16 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,13 @@ type Config struct {
// server.
//
// Client expectations:
// - Perform any proxy resolution.
// - Check the reverse DNS and forward DNS match.
// - Check the IP against suitable access controls (ipaccess, dnsbl, etc).
// - Perform any proxy resolution.
// - Check the reverse DNS and forward DNS match.
// - Check the IP against suitable access controls (ipaccess, dnsbl, etc).
//
// More information:
// - https://ircv3.net/specs/extensions/webirc.html
// - https://kiwiirc.com/docs/webirc
// - https://ircv3.net/specs/extensions/webirc.html
// - https://kiwiirc.com/docs/webirc
type WebIRC struct {
// Password that authenticates the WEBIRC command from this client.
Password string
Expand Down Expand Up @@ -308,6 +308,23 @@ func New(config Config) *Client {
return c
}

// receive is a wrapper for sending to the Client.rx channel. It will timeout if
// the event can't be sent within 30s.
func (c *Client) receive(e *Event) {
t := time.NewTimer(30 * time.Second)
defer func() {
if !t.Stop() {
<-t.C
}
}()

select {
case c.rx <- e:
case <-t.C:
c.debugLogEvent(e, true)
}
}

// String returns a brief description of the current client state.
func (c *Client) String() string {
connected := c.IsConnected()
Expand Down Expand Up @@ -388,7 +405,7 @@ func (e *ErrEvent) Error() string {
return e.Event.Last()
}

func (c *Client) execLoop(ctx context.Context, errs chan error, wg *sync.WaitGroup) {
func (c *Client) execLoop(ctx context.Context) error {
c.debug.Print("starting execLoop")
defer c.debug.Print("closing execLoop")

Expand All @@ -411,9 +428,10 @@ func (c *Client) execLoop(ctx context.Context, errs chan error, wg *sync.WaitGro
}

done:
wg.Done()
return
return nil
case event = <-c.rx:
c.RunHandlers(event)

if event != nil && event.Command == ERROR {
// Handles incoming ERROR responses. These are only ever sent
// by the server (with the exception that this library may use
Expand All @@ -423,13 +441,9 @@ func (c *Client) execLoop(ctx context.Context, errs chan error, wg *sync.WaitGro
// some reason the server doesn't disconnect the client, or
// if this library is the source of the error, this should
// signal back up to the main connect loop, to disconnect.
errs <- &ErrEvent{Event: event}

// Make sure to not actually exit, so we can let any handlers
// actually handle the ERROR event.
return &ErrEvent{Event: event}
}

c.RunHandlers(event)
}
}
}
Expand Down Expand Up @@ -677,8 +691,7 @@ func (c *Client) IsInChannel(channel string) (in bool) {
// during client connection. This is also known as ISUPPORT (or RPL_PROTOCTL).
// Will panic if used when tracking has been disabled. Examples of usage:
//
// nickLen, success := GetServerOption("MAXNICKLEN")
//
// nickLen, success := GetServerOption("MAXNICKLEN")
func (c *Client) GetServerOption(key string) (result string, ok bool) {
c.panicIfNotTracking()

Expand All @@ -688,6 +701,42 @@ func (c *Client) GetServerOption(key string) (result string, ok bool) {
return result, ok
}

// GetServerOptionInt retrieves a server capability setting (as an integer) that was
// retrieved during client connection. This is also known as ISUPPORT (or RPL_PROTOCTL).
// Will panic if used when tracking has been disabled. Examples of usage:
//
// nickLen, success := GetServerOption("MAXNICKLEN")
func (c *Client) GetServerOptionInt(key string) (result int, ok bool) {
var data string
var err error

data, ok = c.GetServerOption(key)
if !ok {
return result, ok
}
result, err = strconv.Atoi(data)
if err != nil {
ok = false
}

return result, ok
}

// MaxEventLength returns the maximum supported server length of an event. This is the
// maximum length of the command and arguments, excluding the source/prefix supported
// by the protocol. If state tracking is enabled, this will utilize ISUPPORT/IRCv3
// information to more accurately calculate the maximum supported length (i.e. extended
// length events).
func (c *Client) MaxEventLength() (max int) {
if !c.Config.disableTracking {
c.state.RLock()
max = c.state.maxLineLength - c.state.maxPrefixLength
c.state.RUnlock()
return max
}
return DefaultMaxLineLength - DefaultMaxPrefixLength
}

// NetworkName returns the network identifier. E.g. "EsperNet", "ByteIRC".
// May be empty if the server does not support RPL_ISUPPORT (or RPL_PROTOCTL).
// Will panic if used when tracking has been disabled.
Expand Down Expand Up @@ -781,7 +830,7 @@ func (c *Client) debugLogEvent(e *Event, dropped bool) {
var prefix string

if dropped {
prefix = "dropping event (disconnected):"
prefix = "dropping event (disconnected or timeout):"
} else {
prefix = ">"
}
Expand Down
10 changes: 7 additions & 3 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,12 @@ func TestClientClose(t *testing.T) {
errchan := make(chan error, 1)
done := make(chan struct{}, 1)

c.Handlers.AddBg(CLOSED, func(c *Client, e Event) { close(done) })
c.Handlers.AddBg(INITIALIZED, func(c *Client, e Event) { c.Close() })
c.Handlers.AddBg(CLOSED, func(c *Client, e Event) {
close(done)
})
c.Handlers.AddBg(INITIALIZED, func(c *Client, e Event) {
c.Close()
})

go func() { errchan <- c.MockConnect(server) }()

Expand All @@ -193,7 +197,7 @@ func TestClientClose(t *testing.T) {
}

t.Fatalf("connect returned with error when close was invoked: %s", err)
case <-time.After(5 * time.Second):
case <-time.After(35 * time.Second):
t.Fatal("Client.Close() timed out")
case <-done:
}
Expand Down
8 changes: 4 additions & 4 deletions commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ func (cmd *Commands) Nick(name string) {
// prevent sending extensive JOIN commands.
func (cmd *Commands) Join(channels ...string) {
// We can join multiple channels at once, however we need to ensure that
// we are not exceeding the line length. (see maxLength)
max := maxLength - len(JOIN) - 1
// we are not exceeding the line length (see Client.MaxEventLength()).
max := cmd.c.MaxEventLength() - len(JOIN) - 1

var buffer string

Expand Down Expand Up @@ -329,8 +329,8 @@ func (cmd *Commands) List(channels ...string) {
}

// We can LIST multiple channels at once, however we need to ensure that
// we are not exceeding the line length. (see maxLength)
max := maxLength - len(JOIN) - 1
// we are not exceeding the line length (see Client.MaxEventLength()).
max := cmd.c.MaxEventLength() - len(JOIN) - 1

var buffer string

Expand Down
Loading

0 comments on commit 658eba5

Please sign in to comment.