From 15c54e80dec3e577384b1dfd4364759ab40c8e63 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 3 Aug 2020 23:44:44 -0400 Subject: [PATCH 1/3] clean up some error handling --- irc/errors.go | 7 ------- irc/ircconn.go | 9 +++++++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/irc/errors.go b/irc/errors.go index 01758997..5e3efc74 100644 --- a/irc/errors.go +++ b/irc/errors.go @@ -73,13 +73,6 @@ var ( errRegisteredOnly = errors.New("Cannot join registered-only channel without an account") ) -// Socket Errors -var ( - errNoPeerCerts = errors.New("Client did not provide a certificate") - errNotTLS = errors.New("Not a TLS connection") - errReadQ = errors.New("ReadQ Exceeded") -) - // String Errors var ( errCouldNotStabilize = errors.New("Could not stabilize string while casefolding") diff --git a/irc/ircconn.go b/irc/ircconn.go index 9ddb0da7..c0ffb948 100644 --- a/irc/ircconn.go +++ b/irc/ircconn.go @@ -3,6 +3,7 @@ package irc import ( "bufio" "bytes" + "errors" "net" "unicode/utf8" @@ -17,7 +18,8 @@ const ( ) var ( - crlf = []byte{'\r', '\n'} + crlf = []byte{'\r', '\n'} + errReadQ = errors.New("ReadQ Exceeded") ) // IRCConn abstracts away the distinction between a regular @@ -31,7 +33,7 @@ type IRCConn interface { // these take an IRC line or lines, correctly terminated with CRLF: WriteLine([]byte) error WriteLines([][]byte) error - // this returns an IRC line without the terminating CRLF: + // this returns an IRC line, possibly terminated with CRLF, LF, or nothing: ReadLine() (line []byte, err error) Close() error @@ -127,6 +129,9 @@ func (wc IRCWSConn) ReadLine() (line []byte, err error) { messageType, line, err = wc.conn.ReadMessage() // on empty message or non-text message, try again, block if necessary if err != nil || (messageType == websocket.TextMessage && len(line) != 0) { + if err == websocket.ErrReadLimit { + err = errReadQ + } return } } From ddac7d94a8d3981c0d5055a97278457513667394 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Tue, 4 Aug 2020 01:10:22 -0400 Subject: [PATCH 2/3] use ChannelSet --- irc/handlers.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/irc/handlers.go b/irc/handlers.go index 5bcf027f..62ba2c2c 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2998,9 +2998,9 @@ func whoHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo } } else { // Construct set of channels the client is in. - userChannels := make(map[*Channel]bool) + userChannels := make(ChannelSet) for _, channel := range client.Channels() { - userChannels[channel] = true + userChannels[channel] = empty{} } // Another client is a friend if they share at least one channel, or they are the same client. @@ -3010,7 +3010,7 @@ func whoHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo } for _, channel := range otherClient.Channels() { - if userChannels[channel] { + if _, present := userChannels[channel]; present { return true } } From df8be72c6f36321e348775a7c1e07cd11c13cd67 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Tue, 4 Aug 2020 21:46:16 -0400 Subject: [PATCH 3/3] move StringSet to utils package --- irc/channelmanager.go | 16 +++++++++------- irc/channelreg.go | 11 ++++++----- irc/client.go | 4 ++-- irc/client_test.go | 4 +++- irc/config.go | 6 +++--- irc/types.go | 11 ----------- irc/utils/semaphores.go | 14 ++++++-------- irc/utils/types.go | 17 +++++++++++++++++ irc/znc.go | 7 ++++--- 9 files changed, 50 insertions(+), 40 deletions(-) create mode 100644 irc/utils/types.go diff --git a/irc/channelmanager.go b/irc/channelmanager.go index 19430d41..60ceebab 100644 --- a/irc/channelmanager.go +++ b/irc/channelmanager.go @@ -5,6 +5,8 @@ package irc import ( "sync" + + "github.com/oragono/oragono/irc/utils" ) type channelManagerEntry struct { @@ -23,17 +25,17 @@ type ChannelManager struct { sync.RWMutex // tier 2 // chans is the main data structure, mapping casefolded name -> *Channel chans map[string]*channelManagerEntry - chansSkeletons StringSet // skeletons of *unregistered* chans - registeredChannels StringSet // casefolds of registered chans - registeredSkeletons StringSet // skeletons of registered chans - purgedChannels StringSet // casefolds of purged chans + chansSkeletons utils.StringSet // skeletons of *unregistered* chans + registeredChannels utils.StringSet // casefolds of registered chans + registeredSkeletons utils.StringSet // skeletons of registered chans + purgedChannels utils.StringSet // casefolds of purged chans server *Server } // NewChannelManager returns a new ChannelManager. func (cm *ChannelManager) Initialize(server *Server) { cm.chans = make(map[string]*channelManagerEntry) - cm.chansSkeletons = make(StringSet) + cm.chansSkeletons = make(utils.StringSet) cm.server = server cm.loadRegisteredChannels(server.Config()) @@ -47,8 +49,8 @@ func (cm *ChannelManager) loadRegisteredChannels(config *Config) { } rawNames := cm.server.channelRegistry.AllChannels() - registeredChannels := make(StringSet, len(rawNames)) - registeredSkeletons := make(StringSet, len(rawNames)) + registeredChannels := make(utils.StringSet, len(rawNames)) + registeredSkeletons := make(utils.StringSet, len(rawNames)) for _, name := range rawNames { cfname, err := CasefoldChannel(name) if err == nil { diff --git a/irc/channelreg.go b/irc/channelreg.go index 31b68254..47931c03 100644 --- a/irc/channelreg.go +++ b/irc/channelreg.go @@ -4,15 +4,16 @@ package irc import ( + "encoding/json" "fmt" "strconv" "strings" "time" - "encoding/json" + "github.com/tidwall/buntdb" "github.com/oragono/oragono/irc/modes" - "github.com/tidwall/buntdb" + "github.com/oragono/oragono/irc/utils" ) // this is exclusively the *persistence* layer for channel registration; @@ -140,8 +141,8 @@ func (reg *ChannelRegistry) AllChannels() (result []string) { } // PurgedChannels returns the set of all casefolded channel names that have been purged -func (reg *ChannelRegistry) PurgedChannels() (result map[string]empty) { - result = make(map[string]empty) +func (reg *ChannelRegistry) PurgedChannels() (result utils.StringSet) { + result = make(utils.StringSet) prefix := fmt.Sprintf(keyChannelPurged, "") reg.server.store.View(func(tx *buntdb.Tx) error { @@ -150,7 +151,7 @@ func (reg *ChannelRegistry) PurgedChannels() (result map[string]empty) { return false } channel := strings.TrimPrefix(key, prefix) - result[channel] = empty{} + result.Add(channel) return true }) }) diff --git a/irc/client.go b/irc/client.go index edbf44dd..ad3009c0 100644 --- a/irc/client.go +++ b/irc/client.go @@ -64,7 +64,7 @@ type Client struct { destroyed bool modes modes.ModeSet hostname string - invitedTo StringSet + invitedTo utils.StringSet isSTSOnly bool languages []string lastActive time.Time // last time they sent a command that wasn't PONG or similar @@ -1641,7 +1641,7 @@ func (client *Client) Invite(casefoldedChannel string) { defer client.stateMutex.Unlock() if client.invitedTo == nil { - client.invitedTo = make(StringSet) + client.invitedTo = make(utils.StringSet) } client.invitedTo.Add(casefoldedChannel) diff --git a/irc/client_test.go b/irc/client_test.go index 2d1addc1..d371c11f 100644 --- a/irc/client_test.go +++ b/irc/client_test.go @@ -5,11 +5,13 @@ package irc import ( "testing" + + "github.com/oragono/oragono/irc/utils" ) func TestGenerateBatchID(t *testing.T) { var session Session - s := make(StringSet) + s := make(utils.StringSet) count := 100000 for i := 0; i < count; i++ { diff --git a/irc/config.go b/irc/config.go index 31a4fdbe..0a9a76bc 100644 --- a/irc/config.go +++ b/irc/config.go @@ -624,8 +624,8 @@ type Config struct { // OperClass defines an assembled operator class. type OperClass struct { Title string - WhoisLine string `yaml:"whois-line"` - Capabilities StringSet // map to make lookups much easier + WhoisLine string `yaml:"whois-line"` + Capabilities utils.StringSet // map to make lookups much easier } // OperatorClasses returns a map of assembled operator classes from the given config. @@ -663,7 +663,7 @@ func (conf *Config) OperatorClasses() (map[string]*OperClass, error) { // create new operclass var oc OperClass - oc.Capabilities = make(StringSet) + oc.Capabilities = make(utils.StringSet) // get inhereted info from other operclasses if len(info.Extends) > 0 { diff --git a/irc/types.go b/irc/types.go index 7307a725..5fb8508a 100644 --- a/irc/types.go +++ b/irc/types.go @@ -28,17 +28,6 @@ func (clients ClientSet) Has(client *Client) bool { return ok } -type StringSet map[string]empty - -func (s StringSet) Has(str string) bool { - _, ok := s[str] - return ok -} - -func (s StringSet) Add(str string) { - s[str] = empty{} -} - // MemberSet is a set of members with modes. type MemberSet map[*Client]*modes.ModeSet diff --git a/irc/utils/semaphores.go b/irc/utils/semaphores.go index 64ad6b92..2268cde7 100644 --- a/irc/utils/semaphores.go +++ b/irc/utils/semaphores.go @@ -10,27 +10,25 @@ import ( "time" ) -type e struct{} - // Semaphore is a counting semaphore. // A semaphore of capacity 1 can be used as a trylock. -type Semaphore (chan e) +type Semaphore (chan empty) // Initialize initializes a semaphore to a given capacity. func (semaphore *Semaphore) Initialize(capacity int) { - *semaphore = make(chan e, capacity) + *semaphore = make(chan empty, capacity) } // Acquire acquires a semaphore, blocking if necessary. func (semaphore *Semaphore) Acquire() { - (*semaphore) <- e{} + (*semaphore) <- empty{} } // TryAcquire tries to acquire a semaphore, returning whether the acquire was // successful. It never blocks. func (semaphore *Semaphore) TryAcquire() (acquired bool) { select { - case (*semaphore) <- e{}: + case (*semaphore) <- empty{}: return true default: return false @@ -47,7 +45,7 @@ func (semaphore *Semaphore) AcquireWithTimeout(timeout time.Duration) (acquired timer := time.NewTimer(timeout) select { - case (*semaphore) <- e{}: + case (*semaphore) <- empty{}: acquired = true case <-timer.C: acquired = false @@ -61,7 +59,7 @@ func (semaphore *Semaphore) AcquireWithTimeout(timeout time.Duration) (acquired // Note that if the context is already expired, the acquire may succeed anyway. func (semaphore *Semaphore) AcquireWithContext(ctx context.Context) (acquired bool) { select { - case (*semaphore) <- e{}: + case (*semaphore) <- empty{}: acquired = true case <-ctx.Done(): acquired = false diff --git a/irc/utils/types.go b/irc/utils/types.go new file mode 100644 index 00000000..249b486e --- /dev/null +++ b/irc/utils/types.go @@ -0,0 +1,17 @@ +// Copyright (c) 2020 Shivaram Lingamneni +// released under the MIT license + +package utils + +type empty struct{} + +type StringSet map[string]empty + +func (s StringSet) Has(str string) bool { + _, ok := s[str] + return ok +} + +func (s StringSet) Add(str string) { + s[str] = empty{} +} diff --git a/irc/znc.go b/irc/znc.go index 95bd800f..f0d5da6e 100644 --- a/irc/znc.go +++ b/irc/znc.go @@ -10,6 +10,7 @@ import ( "time" "github.com/oragono/oragono/irc/history" + "github.com/oragono/oragono/irc/utils" ) const ( @@ -62,7 +63,7 @@ func timeToZncWireTime(t time.Time) (result string) { type zncPlaybackTimes struct { start time.Time end time.Time - targets StringSet // nil for "*" (everything), otherwise the channel names + targets utils.StringSet // nil for "*" (everything), otherwise the channel names setAt time.Time } @@ -122,7 +123,7 @@ func zncPlaybackPlayHandler(client *Client, command string, params []string, rb end = zncWireTimeToTime(params[3]) } - var targets StringSet + var targets utils.StringSet var nickTargets []string // three cases: @@ -145,7 +146,7 @@ func zncPlaybackPlayHandler(client *Client, command string, params []string, rb if params[1] == "*" { playPrivmsgs = true // XXX nil `targets` means "every channel" } else { - targets = make(StringSet) + targets = make(utils.StringSet) for _, targetName := range strings.Split(targetString, ",") { if targetName == "*self" { playPrivmsgs = true