From b2f798cf03ad10f0bb1ba0f1045f2ecf0b0ead69 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Fri, 13 Apr 2018 15:47:57 -0400 Subject: [PATCH 01/12] eliminate dedicated RunSocketWriter goroutine --- irc/client.go | 1 - irc/socket.go | 97 ++++++++++++++++++++++++++++++--------------------- 2 files changed, 58 insertions(+), 40 deletions(-) diff --git a/irc/client.go b/irc/client.go index d309102a..7e2cdde8 100644 --- a/irc/client.go +++ b/irc/client.go @@ -90,7 +90,6 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { limits := server.Limits() fullLineLenLimit := limits.LineLen.Tags + limits.LineLen.Rest socket := NewSocket(conn, fullLineLenLimit*2, server.MaxSendQBytes()) - go socket.RunSocketWriter() client := &Client{ atime: now, authorized: server.Password() == nil, diff --git a/irc/socket.go b/irc/socket.go index 0f98a6f9..f4ac6917 100644 --- a/irc/socket.go +++ b/irc/socket.go @@ -31,23 +31,26 @@ type Socket struct { maxSendQBytes int - // coordination system for asynchronous writes - buffer []byte - lineToSendExists chan bool + // this is a trylock enforcing that only one goroutine can write to `conn` at a time + writerSlotOpen chan bool + buffer []byte closed bool sendQExceeded bool finalData string // what to send when we die + finalized bool } // NewSocket returns a new Socket. func NewSocket(conn net.Conn, maxReadQBytes int, maxSendQBytes int) Socket { - return Socket{ - conn: conn, - reader: bufio.NewReaderSize(conn, maxReadQBytes), - maxSendQBytes: maxSendQBytes, - lineToSendExists: make(chan bool, 1), + result := Socket{ + conn: conn, + reader: bufio.NewReaderSize(conn, maxReadQBytes), + maxSendQBytes: maxSendQBytes, + writerSlotOpen: make(chan bool, 1), } + result.writerSlotOpen <- true + return result } // Close stops a Socket from being able to send/receive any more data. @@ -56,7 +59,7 @@ func (socket *Socket) Close() { socket.closed = true socket.Unlock() - socket.wakeWriter() + go socket.send() } // CertFP returns the fingerprint of the certificate provided by the client. @@ -114,7 +117,11 @@ func (socket *Socket) Read() (string, error) { return line, nil } -// Write sends the given string out of Socket. +// Write sends the given string out of Socket. Requirements: +// 1. MUST NOT block for macroscopic amounts of time +// 2. MUST NOT reorder messages +// 3. MUST provide mutual exclusion for socket.conn.Write +// 4. SHOULD NOT tie up additional goroutines, beyond the one blocked on socket.conn.Write func (socket *Socket) Write(data string) (err error) { socket.Lock() if socket.closed { @@ -127,19 +134,10 @@ func (socket *Socket) Write(data string) (err error) { } socket.Unlock() - socket.wakeWriter() + go socket.send() return } -// wakeWriter wakes up the goroutine that actually performs the write, without blocking -func (socket *Socket) wakeWriter() { - // nonblocking send to the channel, no-op if it's full - select { - case socket.lineToSendExists <- true: - default: - } -} - // SetFinalData sets the final data to send when the SocketWriter closes. func (socket *Socket) SetFinalData(data string) { socket.Lock() @@ -154,32 +152,53 @@ func (socket *Socket) IsClosed() bool { return socket.closed } -// RunSocketWriter starts writing messages to the outgoing socket. -func (socket *Socket) RunSocketWriter() { - localBuffer := make([]byte, 0) - shouldStop := false - for !shouldStop { - // wait for new lines +// is there data to write? +func (socket *Socket) readyToWrite() bool { + socket.Lock() + defer socket.Unlock() + // on the first time observing socket.closed, we still have to write socket.finalData + return !socket.finalized && (len(socket.buffer) > 0 || socket.closed || socket.sendQExceeded) +} + +// send actually writes messages to socket.Conn; it may block +func (socket *Socket) send() { + // one of these checks happens-after every call to Write(), so we can't miss writes + for socket.readyToWrite() { select { - case <-socket.lineToSendExists: - // retrieve the buffered data, clear the buffer - socket.Lock() - localBuffer = append(localBuffer, socket.buffer...) - socket.buffer = socket.buffer[:0] - socket.Unlock() - - _, err := socket.conn.Write(localBuffer) - localBuffer = localBuffer[:0] - - socket.Lock() - shouldStop = (err != nil) || socket.closed || socket.sendQExceeded - socket.Unlock() + case <-socket.writerSlotOpen: + // got the trylock: actually do the write + socket.performWrite() + socket.writerSlotOpen <- true + default: + // another goroutine is in progress; exit and wait for them to loop back around + // and observe readyToWrite() again + return } } +} + +// write the contents of the buffer, then see if we need to close +func (socket *Socket) performWrite() { + // retrieve the buffered data, clear the buffer + socket.Lock() + buffer := socket.buffer + socket.buffer = nil + socket.Unlock() + + _, err := socket.conn.Write(buffer) + + socket.Lock() + shouldClose := (err != nil) || socket.closed || socket.sendQExceeded + socket.Unlock() + + if !shouldClose { + return + } // mark the socket closed (if someone hasn't already), then write error lines socket.Lock() socket.closed = true + socket.finalized = true finalData := socket.finalData if socket.sendQExceeded { finalData = "\r\nERROR :SendQ Exceeded\r\n" From 4778e7bcc7024c7784eda17074ee9986dc0e8eaa Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 15 Apr 2018 01:21:32 -0400 Subject: [PATCH 02/12] fixes * Placate `go vet` * Reorder the `send` loop, clarify things a little --- irc/client.go | 2 +- irc/socket.go | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/irc/client.go b/irc/client.go index 7e2cdde8..3f4e2c8f 100644 --- a/irc/client.go +++ b/irc/client.go @@ -100,7 +100,7 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { ctime: now, flags: make(map[modes.Mode]bool), server: server, - socket: &socket, + socket: socket, nick: "*", // * is used until actual nick is given nickCasefolded: "*", nickMaskString: "*", // * is used until actual nick is given diff --git a/irc/socket.go b/irc/socket.go index f4ac6917..c78c7d57 100644 --- a/irc/socket.go +++ b/irc/socket.go @@ -42,7 +42,7 @@ type Socket struct { } // NewSocket returns a new Socket. -func NewSocket(conn net.Conn, maxReadQBytes int, maxSendQBytes int) Socket { +func NewSocket(conn net.Conn, maxReadQBytes int, maxSendQBytes int) *Socket { result := Socket{ conn: conn, reader: bufio.NewReaderSize(conn, maxReadQBytes), @@ -50,7 +50,7 @@ func NewSocket(conn net.Conn, maxReadQBytes int, maxSendQBytes int) Socket { writerSlotOpen: make(chan bool, 1), } result.writerSlotOpen <- true - return result + return &result } // Close stops a Socket from being able to send/receive any more data. @@ -162,16 +162,20 @@ func (socket *Socket) readyToWrite() bool { // send actually writes messages to socket.Conn; it may block func (socket *Socket) send() { - // one of these checks happens-after every call to Write(), so we can't miss writes - for socket.readyToWrite() { + for { select { case <-socket.writerSlotOpen: // got the trylock: actually do the write socket.performWrite() + // surrender the trylock: socket.writerSlotOpen <- true + // check if more data came in while we held the trylock: + if !socket.readyToWrite() { + return + } default: - // another goroutine is in progress; exit and wait for them to loop back around - // and observe readyToWrite() again + // someone else has the trylock; if there's more data to write, + // they'll see if after they release it return } } From f54561171e9d84a2afd57f56cc8c056deb8fae5a Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 15 Apr 2018 19:05:22 -0400 Subject: [PATCH 03/12] try to reduce redundant goroutines --- irc/socket.go | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/irc/socket.go b/irc/socket.go index c78c7d57..5b0d2e19 100644 --- a/irc/socket.go +++ b/irc/socket.go @@ -59,7 +59,7 @@ func (socket *Socket) Close() { socket.closed = true socket.Unlock() - go socket.send() + socket.wakeWriter() } // CertFP returns the fingerprint of the certificate provided by the client. @@ -134,10 +134,22 @@ func (socket *Socket) Write(data string) (err error) { } socket.Unlock() - go socket.send() + socket.wakeWriter() return } +// wakeWriter starts the goroutine that actually performs the write, without blocking +func (socket *Socket) wakeWriter() { + // attempt to acquire the trylock + select { + case <-socket.writerSlotOpen: + // acquired the trylock; send() will release it + go socket.send() + default: + // failed to acquire; the holder will check for more data after releasing it + } +} + // SetFinalData sets the final data to send when the SocketWriter closes. func (socket *Socket) SetFinalData(data string) { socket.Lock() @@ -163,19 +175,21 @@ func (socket *Socket) readyToWrite() bool { // send actually writes messages to socket.Conn; it may block func (socket *Socket) send() { for { + // we are holding the trylock: actually do the write + socket.performWrite() + // surrender the trylock, avoiding a race where a write comes in after we've + // checked readyToWrite() and it returned false, but while we still hold the trylock: + socket.writerSlotOpen <- true + // check if more data came in while we held the trylock: + if !socket.readyToWrite() { + return + } select { case <-socket.writerSlotOpen: - // got the trylock: actually do the write - socket.performWrite() - // surrender the trylock: - socket.writerSlotOpen <- true - // check if more data came in while we held the trylock: - if !socket.readyToWrite() { - return - } + // got the trylock, loop back around and write default: - // someone else has the trylock; if there's more data to write, - // they'll see if after they release it + // failed to acquire; exit and wait for the holder to observe readyToWrite() + // after releasing it return } } From 69fd3ac3244344d1c1398e288935b66c8f7cfcd4 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 16 Apr 2018 16:28:31 -0400 Subject: [PATCH 04/12] implement database auto-upgrades (#243) --- irc/config.go | 3 +- irc/database.go | 139 ++++++++++++++++++++++++++++++++++++++++-------- irc/server.go | 12 ++--- oragono.go | 5 +- oragono.yaml | 4 ++ 5 files changed, 131 insertions(+), 32 deletions(-) diff --git a/irc/config.go b/irc/config.go index ada0a578..3a1282ac 100644 --- a/irc/config.go +++ b/irc/config.go @@ -229,7 +229,8 @@ type Config struct { } Datastore struct { - Path string + Path string + AutoUpgrade *bool } Accounts AccountConfig diff --git a/irc/database.go b/irc/database.go index 1e1a3987..d92c2027 100644 --- a/irc/database.go +++ b/irc/database.go @@ -8,9 +8,11 @@ import ( "encoding/base64" "encoding/json" "fmt" + "io" "log" "os" "strings" + "time" "github.com/oragono/oragono/irc/modes" "github.com/oragono/oragono/irc/passwd" @@ -38,6 +40,22 @@ type SchemaChange struct { // maps an initial version to a schema change capable of upgrading it var schemaChanges map[string]SchemaChange +type incompatibleSchemaError struct { + currentVersion string + requiredVersion string +} + +func IncompatibleSchemaError(currentVersion string) (result *incompatibleSchemaError) { + return &incompatibleSchemaError{ + currentVersion: currentVersion, + requiredVersion: latestDbSchema, + } +} + +func (err *incompatibleSchemaError) Error() string { + return fmt.Sprintf("Database requires update. Expected schema v%s, got v%s", err.requiredVersion, err.currentVersion) +} + // InitDB creates the database. func InitDB(path string) { // prepare kvstore db @@ -69,36 +87,107 @@ func InitDB(path string) { } // OpenDatabase returns an existing database, performing a schema version check. -func OpenDatabase(path string) (*buntdb.DB, error) { - // open data store - db, err := buntdb.Open(path) +func OpenDatabase(config *Config) (*buntdb.DB, error) { + allowAutoupgrade := true + if config.Datastore.AutoUpgrade != nil { + allowAutoupgrade = *config.Datastore.AutoUpgrade + } + return openDatabaseInternal(config, allowAutoupgrade) +} + +// open the database, giving it at most one chance to auto-upgrade the schema +func openDatabaseInternal(config *Config, allowAutoupgrade bool) (db *buntdb.DB, err error) { + db, err = buntdb.Open(config.Datastore.Path) if err != nil { - return nil, err + return } - // check db version - err = db.View(func(tx *buntdb.Tx) error { - version, _ := tx.Get(keySchemaVersion) - if version != latestDbSchema { - return fmt.Errorf("Database must be updated. Expected schema v%s, got v%s", latestDbSchema, version) + defer func() { + if err != nil && db != nil { + db.Close() + db = nil } - return nil - }) + }() + // read the current version string + var version string + err = db.View(func(tx *buntdb.Tx) error { + version, err = tx.Get(keySchemaVersion) + return err + }) if err != nil { - // close the db - db.Close() - return nil, err + return } - return db, nil + if version == latestDbSchema { + // success + return + } + + // XXX quiesce the DB so we can be sure it's safe to make a backup copy + db.Close() + db = nil + if allowAutoupgrade { + err = performAutoUpgrade(version, config) + if err != nil { + return + } + // successful autoupgrade, let's try this again: + return openDatabaseInternal(config, false) + } else { + err = IncompatibleSchemaError(version) + return + } +} + +// implementation of `cp` (go should really provide this...) +func cpFile(src string, dst string) (err error) { + in, err := os.Open(src) + if err != nil { + return + } + defer in.Close() + out, err := os.Create(dst) + if err != nil { + return + } + defer func() { + closeError := out.Close() + if err == nil { + err = closeError + } + }() + if _, err = io.Copy(out, in); err != nil { + return + } + return +} + +func performAutoUpgrade(currentVersion string, config *Config) (err error) { + path := config.Datastore.Path + log.Printf("attempting to auto-upgrade schema from version %s to %s\n", currentVersion, latestDbSchema) + timestamp := time.Now().UTC().Format("2006-01-02-15:04:05.000Z") + backupPath := fmt.Sprintf("%s.v%s.%s.bak", path, currentVersion, timestamp) + log.Printf("making a backup of current database at %s\n", backupPath) + err = cpFile(path, backupPath) + if err != nil { + return err + } + + err = UpgradeDB(config) + if err != nil { + // database upgrade is a single transaction, so we don't need to restore the backup; + // we can just delete it + os.Remove(backupPath) + } + return err } // UpgradeDB upgrades the datastore to the latest schema. -func UpgradeDB(config *Config) { +func UpgradeDB(config *Config) (err error) { store, err := buntdb.Open(config.Datastore.Path) if err != nil { - log.Fatal(fmt.Sprintf("Failed to open datastore: %s", err.Error())) + return err } defer store.Close() @@ -108,9 +197,14 @@ func UpgradeDB(config *Config) { version, _ = tx.Get(keySchemaVersion) change, schemaNeedsChange := schemaChanges[version] if !schemaNeedsChange { - break + if version == latestDbSchema { + // success! + break + } + // unable to upgrade to the desired version, roll back + return IncompatibleSchemaError(version) } - log.Println("attempting to update store from version " + version) + log.Println("attempting to update schema from version " + version) err := change.Changer(config, tx) if err != nil { return err @@ -119,16 +213,15 @@ func UpgradeDB(config *Config) { if err != nil { return err } - log.Println("successfully updated store to version " + change.TargetVersion) + log.Println("successfully updated schema to version " + change.TargetVersion) } return nil }) if err != nil { - log.Fatal("Could not update datastore:", err.Error()) + log.Println("database upgrade failed and was rolled back") } - - return + return err } func schemaChangeV1toV2(config *Config, tx *buntdb.Tx) error { diff --git a/irc/server.go b/irc/server.go index e6a5570b..9032c3ff 100644 --- a/irc/server.go +++ b/irc/server.go @@ -127,7 +127,6 @@ type Server struct { signals chan os.Signal snomasks *SnoManager store *buntdb.DB - storeFilename string stsEnabled bool webirc []webircConfig whoWas *WhoWasList @@ -746,7 +745,7 @@ func (server *Server) applyConfig(config *Config, initial bool) error { return fmt.Errorf("Maximum line length (linelen) cannot be changed after launching the server, rehash aborted") } else if server.name != config.Server.Name { return fmt.Errorf("Server name cannot be changed after launching the server, rehash aborted") - } else if server.storeFilename != config.Datastore.Path { + } else if server.config.Datastore.Path != config.Datastore.Path { return fmt.Errorf("Datastore path cannot be changed after launching the server, rehash aborted") } } @@ -966,10 +965,9 @@ func (server *Server) applyConfig(config *Config, initial bool) error { server.config = config server.configurableStateMutex.Unlock() - server.storeFilename = config.Datastore.Path - server.logger.Info("rehash", "Using datastore", server.storeFilename) + server.logger.Info("rehash", "Using datastore", config.Datastore.Path) if initial { - if err := server.loadDatastore(server.storeFilename); err != nil { + if err := server.loadDatastore(config); err != nil { return err } } @@ -1066,11 +1064,11 @@ func (server *Server) loadMOTD(motdPath string, useFormatting bool) error { return nil } -func (server *Server) loadDatastore(datastorePath string) error { +func (server *Server) loadDatastore(config *Config) error { // open the datastore and load server state for which it (rather than config) // is the source of truth - db, err := OpenDatabase(datastorePath) + db, err := OpenDatabase(config) if err == nil { server.store = db } else { diff --git a/oragono.go b/oragono.go index f0f99b40..61547131 100644 --- a/oragono.go +++ b/oragono.go @@ -84,7 +84,10 @@ Options: log.Println("database initialized: ", config.Datastore.Path) } } else if arguments["upgradedb"].(bool) { - irc.UpgradeDB(config) + err = irc.UpgradeDB(config) + if err != nil { + log.Fatal("Error while upgrading db:", err.Error()) + } if !arguments["--quiet"].(bool) { log.Println("database upgraded: ", config.Datastore.Path) } diff --git a/oragono.yaml b/oragono.yaml index 6c5aa757..7fca18a7 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -341,6 +341,10 @@ debug: datastore: # path to the datastore path: ircd.db + # if the database schema requires an upgrade, `autoupgrade` (which defaults to true) + # will attempt to perform it automatically on startup. the database will be backed + # up, and if the upgrade fails, the original database will be restored. + autoupgrade: true # languages config languages: From 1a6c334b3d8ec5d4037db709ad5bbca0d87daff8 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Thu, 19 Apr 2018 03:22:45 -0400 Subject: [PATCH 05/12] update developing documentation --- DEVELOPING.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/DEVELOPING.md b/DEVELOPING.md index 5fb19970..48c9d103 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -79,9 +79,19 @@ As well, there's a decent set of 'tests' here, which I like to run Oragono throu https://github.com/DanielOaks/irctest -## Debugging Hangs +## Debugging -To debug a hang, the best thing to do is to get a stack trace. Go's nice, and you can do so by running this: +It's helpful to enable all loglines while developing. Here's how to configure this: + +```yaml +logging: + - + method: stderr + type: "*" + level: debug +``` + +To debug a hang, the best thing to do is to get a stack trace. The easiest way to get stack traces is with the [pprof listener](https://golang.org/pkg/net/http/pprof/), which can be enabled in the `debug` section of the config. Once it's enabled, you can navigate to `http://localhost:6060/debug/pprof/` in your browser and go from there. If that doesn't work, try: $ kill -ABRT From 0b3abb5bde7577bc9b862db188083162182ef640 Mon Sep 17 00:00:00 2001 From: moortens Date: Fri, 20 Apr 2018 09:11:56 +0200 Subject: [PATCH 06/12] Fixed LUSERS count not subtracting invisible users --- irc/handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/irc/handlers.go b/irc/handlers.go index 1b175f68..b54f4332 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -1304,7 +1304,7 @@ func lusersHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re opercount++ } } - rb.Add(nil, server.name, RPL_LUSERCLIENT, client.nick, fmt.Sprintf(client.t("There are %[1]d users and %[2]d invisible on %[3]d server(s)"), totalcount, invisiblecount, 1)) + rb.Add(nil, server.name, RPL_LUSERCLIENT, client.nick, fmt.Sprintf(client.t("There are %[1]d users and %[2]d invisible on %[3]d server(s)"), totalcount - invisiblecount, invisiblecount, 1)) rb.Add(nil, server.name, RPL_LUSEROP, client.nick, fmt.Sprintf(client.t("%d IRC Operators online"), opercount)) rb.Add(nil, server.name, RPL_LUSERCHANNELS, client.nick, fmt.Sprintf(client.t("%d channels formed"), server.channels.Len())) rb.Add(nil, server.name, RPL_LUSERME, client.nick, fmt.Sprintf(client.t("I have %[1]d clients and %[2]d servers"), totalcount, 1)) From 3db71415c95c73d11c4971588e043d85310d9317 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Fri, 20 Apr 2018 03:57:48 -0400 Subject: [PATCH 07/12] review fixes --- irc/config.go | 2 +- irc/database.go | 33 +++------------------------------ irc/utils/os.go | 31 +++++++++++++++++++++++++++++++ oragono.yaml | 5 +++-- 4 files changed, 38 insertions(+), 33 deletions(-) create mode 100644 irc/utils/os.go diff --git a/irc/config.go b/irc/config.go index 3a1282ac..3d1b6504 100644 --- a/irc/config.go +++ b/irc/config.go @@ -230,7 +230,7 @@ type Config struct { Datastore struct { Path string - AutoUpgrade *bool + AutoUpgrade bool } Accounts AccountConfig diff --git a/irc/database.go b/irc/database.go index d92c2027..54f9f469 100644 --- a/irc/database.go +++ b/irc/database.go @@ -8,7 +8,6 @@ import ( "encoding/base64" "encoding/json" "fmt" - "io" "log" "os" "strings" @@ -16,6 +15,7 @@ import ( "github.com/oragono/oragono/irc/modes" "github.com/oragono/oragono/irc/passwd" + "github.com/oragono/oragono/irc/utils" "github.com/tidwall/buntdb" ) @@ -88,11 +88,7 @@ func InitDB(path string) { // OpenDatabase returns an existing database, performing a schema version check. func OpenDatabase(config *Config) (*buntdb.DB, error) { - allowAutoupgrade := true - if config.Datastore.AutoUpgrade != nil { - allowAutoupgrade = *config.Datastore.AutoUpgrade - } - return openDatabaseInternal(config, allowAutoupgrade) + return openDatabaseInternal(config, config.Datastore.AutoUpgrade) } // open the database, giving it at most one chance to auto-upgrade the schema @@ -140,36 +136,13 @@ func openDatabaseInternal(config *Config, allowAutoupgrade bool) (db *buntdb.DB, } } -// implementation of `cp` (go should really provide this...) -func cpFile(src string, dst string) (err error) { - in, err := os.Open(src) - if err != nil { - return - } - defer in.Close() - out, err := os.Create(dst) - if err != nil { - return - } - defer func() { - closeError := out.Close() - if err == nil { - err = closeError - } - }() - if _, err = io.Copy(out, in); err != nil { - return - } - return -} - func performAutoUpgrade(currentVersion string, config *Config) (err error) { path := config.Datastore.Path log.Printf("attempting to auto-upgrade schema from version %s to %s\n", currentVersion, latestDbSchema) timestamp := time.Now().UTC().Format("2006-01-02-15:04:05.000Z") backupPath := fmt.Sprintf("%s.v%s.%s.bak", path, currentVersion, timestamp) log.Printf("making a backup of current database at %s\n", backupPath) - err = cpFile(path, backupPath) + err = utils.CopyFile(path, backupPath) if err != nil { return err } diff --git a/irc/utils/os.go b/irc/utils/os.go new file mode 100644 index 00000000..ef3eb5d1 --- /dev/null +++ b/irc/utils/os.go @@ -0,0 +1,31 @@ +// Copyright (c) 2018 Shivaram Lingamneni + +package utils + +import ( + "io" + "os" +) + +// implementation of `cp` (go should really provide this...) +func CopyFile(src string, dst string) (err error) { + in, err := os.Open(src) + if err != nil { + return + } + defer in.Close() + out, err := os.Create(dst) + if err != nil { + return + } + defer func() { + closeError := out.Close() + if err == nil { + err = closeError + } + }() + if _, err = io.Copy(out, in); err != nil { + return + } + return +} diff --git a/oragono.yaml b/oragono.yaml index 7fca18a7..9bb17b45 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -341,8 +341,9 @@ debug: datastore: # path to the datastore path: ircd.db - # if the database schema requires an upgrade, `autoupgrade` (which defaults to true) - # will attempt to perform it automatically on startup. the database will be backed + + # if the database schema requires an upgrade, `autoupgrade` will attempt to + # perform it automatically on startup. the database will be backed # up, and if the upgrade fails, the original database will be restored. autoupgrade: true From 393070b7d9832f584ae2fd6f0261e951bdf434fd Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Fri, 20 Apr 2018 14:13:25 -0400 Subject: [PATCH 08/12] fix #249 --- irc/client.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/irc/client.go b/irc/client.go index 0cafce83..0ac9bddc 100644 --- a/irc/client.go +++ b/irc/client.go @@ -627,10 +627,14 @@ func (client *Client) LoggedIntoAccount() bool { // RplISupport outputs our ISUPPORT lines to the client. This is used on connection and in VERSION responses. func (client *Client) RplISupport(rb *ResponseBuffer) { translatedISupport := client.t("are supported by this server") - for _, tokenline := range client.server.ISupport().CachedReply { - // ugly trickery ahead - tokenline = append(tokenline, translatedISupport) - rb.Add(nil, client.server.name, RPL_ISUPPORT, append([]string{client.nick}, tokenline...)...) + nick := client.Nick() + for _, cachedTokenLine := range client.server.ISupport().CachedReply { + length := len(cachedTokenLine) + 2 + tokenline := make([]string, length) + tokenline[0] = nick + copy(tokenline[1:], cachedTokenLine) + tokenline[length-1] = translatedISupport + rb.Add(nil, client.server.name, RPL_ISUPPORT, tokenline...) } } From 744ad2ce0bda36ce2b0d18b54b64212fcfde0c98 Mon Sep 17 00:00:00 2001 From: moocow Date: Fri, 20 Apr 2018 22:48:15 +0200 Subject: [PATCH 09/12] Stats for LUSERS logic now seperated, fixed params in LUSERS --- irc/client.go | 9 ++++++++ irc/handlers.go | 23 ++++++++------------ irc/modes.go | 14 ++++++++++++ irc/server.go | 5 +++++ irc/stats.go | 57 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 irc/stats.go diff --git a/irc/client.go b/irc/client.go index 0cafce83..3be6a75b 100644 --- a/irc/client.go +++ b/irc/client.go @@ -724,6 +724,15 @@ func (client *Client) destroy(beingResumed bool) { // send quit messages to friends if !beingResumed { + client.server.stats.ChangeTotal(-1) + if client.HasMode(modes.Invisible) { + client.server.stats.ChangeInvisible(-1) + } + + if client.HasMode(modes.Operator) || client.HasMode(modes.LocalOperator) { + client.server.stats.ChangeOperators(-1) + } + for friend := range friends { if client.quitMessage == "" { client.quitMessage = "Exited" diff --git a/irc/handlers.go b/irc/handlers.go index b54f4332..ac753e80 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -1293,21 +1293,13 @@ func listHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp // LUSERS [ []] func lusersHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *ResponseBuffer) bool { //TODO(vegax87) Fix network statistics and additional parameters - var totalcount, invisiblecount, opercount int + totalCount, invisibleCount, operCount := server.stats.GetStats() + + rb.Add(nil, server.name, RPL_LUSERCLIENT, client.nick, fmt.Sprintf(client.t("There are %[1]d users and %[2]d invisible on %[3]d server(s)"), totalCount-invisibleCount, invisibleCount, 1)) + rb.Add(nil, server.name, RPL_LUSEROP, client.nick, strconv.Itoa(operCount), client.t("IRC Operators online")) + rb.Add(nil, server.name, RPL_LUSERCHANNELS, client.nick, strconv.Itoa(server.channels.Len()), client.t("channels formed")) + rb.Add(nil, server.name, RPL_LUSERME, client.nick, fmt.Sprintf(client.t("I have %[1]d clients and %[2]d servers"), totalCount, 1)) - for _, onlineusers := range server.clients.AllClients() { - totalcount++ - if onlineusers.flags[modes.Invisible] { - invisiblecount++ - } - if onlineusers.flags[modes.Operator] { - opercount++ - } - } - rb.Add(nil, server.name, RPL_LUSERCLIENT, client.nick, fmt.Sprintf(client.t("There are %[1]d users and %[2]d invisible on %[3]d server(s)"), totalcount - invisiblecount, invisiblecount, 1)) - rb.Add(nil, server.name, RPL_LUSEROP, client.nick, fmt.Sprintf(client.t("%d IRC Operators online"), opercount)) - rb.Add(nil, server.name, RPL_LUSERCHANNELS, client.nick, fmt.Sprintf(client.t("%d channels formed"), server.channels.Len())) - rb.Add(nil, server.name, RPL_LUSERME, client.nick, fmt.Sprintf(client.t("I have %[1]d clients and %[2]d servers"), totalcount, 1)) return false } @@ -1792,6 +1784,9 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp server.snomasks.Send(sno.LocalOpers, fmt.Sprintf(ircfmt.Unescape("Client opered up $c[grey][$r%s$c[grey], $r%s$c[grey]]"), client.nickMaskString, client.operName)) + // increase oper count + server.stats.ChangeOperators(1) + // client may now be unthrottled by the fakelag system client.resetFakelag() diff --git a/irc/modes.go b/irc/modes.go index 375831eb..d8cc95b8 100644 --- a/irc/modes.go +++ b/irc/modes.go @@ -37,6 +37,11 @@ func (client *Client) applyUserModeChanges(force bool, changes modes.ModeChanges if client.flags[change.Mode] { continue } + + if change.Mode == modes.Invisible { + client.server.stats.ChangeInvisible(1) + } + client.flags[change.Mode] = true applied = append(applied, change) @@ -44,6 +49,15 @@ func (client *Client) applyUserModeChanges(force bool, changes modes.ModeChanges if !client.flags[change.Mode] { continue } + + if change.Mode == modes.Invisible { + client.server.stats.ChangeInvisible(-1) + } + + if change.Mode == modes.Operator || change.Mode == modes.LocalOperator { + client.server.stats.ChangeOperators(-1) + } + delete(client.flags, change.Mode) applied = append(applied, change) } diff --git a/irc/server.go b/irc/server.go index fa796b71..1fa9dfb4 100644 --- a/irc/server.go +++ b/irc/server.go @@ -131,6 +131,7 @@ type Server struct { stsEnabled bool webirc []webircConfig whoWas *WhoWasList + stats *Stats } var ( @@ -164,6 +165,7 @@ func NewServer(config *Config, logger *logger.Manager) (*Server, error) { signals: make(chan os.Signal, len(ServerExitSignals)), snomasks: NewSnoManager(), whoWas: NewWhoWasList(config.Limits.WhowasEntries), + stats: NewStats(), } if err := server.applyConfig(config, true); err != nil { @@ -472,6 +474,9 @@ func (server *Server) tryRegister(c *Client) { return } + // count new user in statistics + server.stats.ChangeTotal(1) + // continue registration server.logger.Debug("localconnect", fmt.Sprintf("Client registered [%s] [u:%s] [r:%s]", c.nick, c.username, c.realname)) server.snomasks.Send(sno.LocalConnects, fmt.Sprintf(ircfmt.Unescape("Client registered $c[grey][$r%s$c[grey]] [u:$r%s$c[grey]] [h:$r%s$c[grey]] [r:$r%s$c[grey]]"), c.nick, c.username, c.rawHostname, c.realname)) diff --git a/irc/stats.go b/irc/stats.go new file mode 100644 index 00000000..65e4a67f --- /dev/null +++ b/irc/stats.go @@ -0,0 +1,57 @@ +package irc + +import ( + "sync" +) + +// Stats contains the numbers of total, invisible and operators on the server +type Stats struct { + sync.RWMutex + + Total int + Invisible int + Operators int +} + +// NewStats creates a new instance of Stats +func NewStats() *Stats { + serverStats := &Stats{ + Total: 0, + Invisible: 0, + Operators: 0, + } + + return serverStats +} + +// ChangeTotal increments the total user count on server +func (s *Stats) ChangeTotal(i int) { + s.Lock() + defer s.Unlock() + + s.Total += i +} + +// ChangeInvisible increments the invisible count +func (s *Stats) ChangeInvisible(i int) { + s.Lock() + defer s.Unlock() + + s.Invisible += i +} + +// ChangeOperators increases the operator count +func (s *Stats) ChangeOperators(i int) { + s.Lock() + defer s.Unlock() + + s.Operators += i +} + +// GetStats retrives total, invisible and oper count +func (s *Stats) GetStats() (int, int, int) { + s.Lock() + defer s.Unlock() + + return s.Total, s.Invisible, s.Operators +} From fad2475c3f27b099672f42f4cb762bb73c01d517 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Apr 2018 18:47:10 -0400 Subject: [PATCH 10/12] modes refactor, #255 --- Makefile | 6 +- irc/channel.go | 82 +++++++++------------- irc/chanserv.go | 4 +- irc/client.go | 14 ++-- irc/config.go | 10 ++- irc/gateways.go | 6 +- irc/getters.go | 30 ++------ irc/handlers.go | 75 ++++++++------------ irc/modes.go | 144 +++++++++----------------------------- irc/modes/modes.go | 149 ++++++++++++++++++++++++++++++++++++---- irc/modes/modes_test.go | 37 ++++++++++ irc/roleplay.go | 6 +- irc/server.go | 16 ++--- irc/types.go | 15 +--- 14 files changed, 308 insertions(+), 286 deletions(-) create mode 100644 irc/modes/modes_test.go diff --git a/Makefile b/Makefile index c057855e..5fe1c917 100644 --- a/Makefile +++ b/Makefile @@ -12,5 +12,7 @@ deps: git submodule update --init test: - cd irc && go test . - cd irc && go vet . + cd irc && go test . && go vet . + cd irc/isupport && go test . && go vet . + cd irc/modes && go test . && go vet . + cd irc/utils && go test . && go vet . diff --git a/irc/channel.go b/irc/channel.go index 98f645d4..74cf4be2 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -20,7 +20,7 @@ import ( // Channel represents a channel that clients can join. type Channel struct { - flags modes.ModeSet + flags *modes.ModeSet lists map[modes.Mode]*UserMaskSet key string members MemberSet @@ -51,7 +51,7 @@ func NewChannel(s *Server, name string, regInfo *RegisteredChannel) *Channel { channel := &Channel{ createdTime: time.Now(), // may be overwritten by applyRegInfo - flags: make(modes.ModeSet), + flags: modes.NewModeSet(), lists: map[modes.Mode]*UserMaskSet{ modes.BanMask: NewUserMaskSet(), modes.ExceptMask: NewUserMaskSet(), @@ -68,7 +68,7 @@ func NewChannel(s *Server, name string, regInfo *RegisteredChannel) *Channel { channel.applyRegInfo(regInfo) } else { for _, mode := range s.DefaultChannelModes() { - channel.flags[mode] = true + channel.flags.SetMode(mode, true) } } @@ -87,7 +87,7 @@ func (channel *Channel) applyRegInfo(chanReg *RegisteredChannel) { channel.key = chanReg.Key for _, mode := range chanReg.Modes { - channel.flags[mode] = true + channel.flags.SetMode(mode, true) } for _, mask := range chanReg.Banlist { channel.lists[modes.BanMask].Add(mask) @@ -120,9 +120,7 @@ func (channel *Channel) ExportRegistration(includeFlags uint) (info RegisteredCh if includeFlags&IncludeModes != 0 { info.Key = channel.key - for mode := range channel.flags { - info.Modes = append(info.Modes, mode) - } + info.Modes = channel.flags.AllModes() } if includeFlags&IncludeLists != 0 { @@ -225,14 +223,16 @@ func (channel *Channel) ClientIsAtLeast(client *Client, permission modes.Mode) b channel.stateMutex.RLock() defer channel.stateMutex.RUnlock() + clientModes := channel.members[client] + // get voice, since it's not a part of ChannelPrivModes - if channel.members.HasMode(client, permission) { + if clientModes.HasMode(permission) { return true } // check regular modes for _, mode := range modes.ChannelPrivModes { - if channel.members.HasMode(client, mode) { + if clientModes.HasMode(mode) { return true } @@ -263,14 +263,14 @@ func (channel *Channel) ClientHasPrivsOver(client *Client, target *Client) bool targetModes := channel.members[target] result := false for _, mode := range modes.ChannelPrivModes { - if clientModes[mode] { + if clientModes.HasMode(mode) { result = true // admins cannot kick other admins - if mode == modes.ChannelAdmin && targetModes[modes.ChannelAdmin] { + if mode == modes.ChannelAdmin && targetModes.HasMode(modes.ChannelAdmin) { result = false } break - } else if channel.members[target][mode] { + } else if targetModes.HasMode(mode) { break } } @@ -331,14 +331,11 @@ func (channel *Channel) modeStrings(client *Client) (result []string) { mods += modes.UserLimit.String() } + mods += channel.flags.String() + channel.stateMutex.RLock() defer channel.stateMutex.RUnlock() - // flags - for mode := range channel.flags { - mods += mode.String() - } - result = []string{mods} // args for flags with args: The order must match above to keep @@ -395,7 +392,7 @@ func (channel *Channel) Join(client *Client, key string, rb *ResponseBuffer) { } isInvited := channel.lists[modes.InviteMask].Match(client.nickMaskCasefolded) - if channel.flags[modes.InviteOnly] && !isInvited { + if channel.flags.HasMode(modes.InviteOnly) && !isInvited { rb.Add(nil, client.server.name, ERR_INVITEONLYCHAN, channel.name, fmt.Sprintf(client.t("Cannot join channel (+%s)"), "i")) return } @@ -446,7 +443,7 @@ func (channel *Channel) Join(client *Client, key string, rb *ResponseBuffer) { givenMode = &modes.ChannelOperator } if givenMode != nil { - channel.members[client][*givenMode] = true + channel.members[client].SetMode(*givenMode, true) } channel.stateMutex.Unlock() @@ -515,12 +512,12 @@ func (channel *Channel) SendTopic(client *Client, rb *ResponseBuffer) { // SetTopic sets the topic of this channel, if the client is allowed to do so. func (channel *Channel) SetTopic(client *Client, topic string, rb *ResponseBuffer) { - if !(client.flags[modes.Operator] || channel.hasClient(client)) { + if !(client.HasMode(modes.Operator) || channel.hasClient(client)) { rb.Add(nil, client.server.name, ERR_NOTONCHANNEL, channel.name, client.t("You're not on that channel")) return } - if channel.HasMode(modes.OpOnlyTopic) && !channel.ClientIsAtLeast(client, modes.ChannelOperator) { + if channel.flags.HasMode(modes.OpOnlyTopic) && !channel.ClientIsAtLeast(client, modes.ChannelOperator) { rb.Add(nil, client.server.name, ERR_CHANOPRIVSNEEDED, channel.name, client.t("You're not a channel operator")) return } @@ -552,13 +549,13 @@ func (channel *Channel) CanSpeak(client *Client) bool { defer channel.stateMutex.RUnlock() _, hasClient := channel.members[client] - if channel.flags[modes.NoOutside] && !hasClient { + if channel.flags.HasMode(modes.NoOutside) && !hasClient { return false } - if channel.flags[modes.Moderated] && !channel.ClientIsAtLeast(client, modes.Voice) { + if channel.flags.HasMode(modes.Moderated) && !channel.ClientIsAtLeast(client, modes.Voice) { return false } - if channel.flags[modes.RegisteredOnly] && client.Account() == "" { + if channel.flags.HasMode(modes.RegisteredOnly) && client.Account() == "" { return false } return true @@ -682,13 +679,7 @@ func (channel *Channel) sendSplitMessage(msgid, cmd string, minPrefix *modes.Mod } } -func (channel *Channel) applyModeMemberNoMutex(client *Client, mode modes.Mode, op modes.ModeOp, nick string, rb *ResponseBuffer) *modes.ModeChange { - if nick == "" { - //TODO(dan): shouldn't this be handled before it reaches this function? - rb.Add(nil, client.server.name, ERR_NEEDMOREPARAMS, "MODE", client.t("Not enough parameters")) - return nil - } - +func (channel *Channel) applyModeToMember(client *Client, mode modes.Mode, op modes.ModeOp, nick string, rb *ResponseBuffer) (result *modes.ModeChange) { casefoldedName, err := CasefoldName(nick) target := channel.server.clients.Get(casefoldedName) if err != nil || target == nil { @@ -698,26 +689,21 @@ func (channel *Channel) applyModeMemberNoMutex(client *Client, mode modes.Mode, channel.stateMutex.Lock() modeset, exists := channel.members[target] - var already bool if exists { - enable := op == modes.Add - already = modeset[mode] == enable - modeset[mode] = enable + if applied := modeset.SetMode(mode, op == modes.Add); applied { + result = &modes.ModeChange{ + Op: op, + Mode: mode, + Arg: nick, + } + } } channel.stateMutex.Unlock() if !exists { rb.Add(nil, client.server.name, ERR_USERNOTINCHANNEL, client.nick, channel.name, client.t("They aren't on that channel")) - return nil - } else if already { - return nil - } else { - return &modes.ModeChange{ - Op: op, - Mode: mode, - Arg: nick, - } } + return } // ShowMaskList shows the given list to the client. @@ -790,7 +776,7 @@ func (channel *Channel) Quit(client *Client) { } func (channel *Channel) Kick(client *Client, target *Client, comment string, rb *ResponseBuffer) { - if !(client.flags[modes.Operator] || channel.hasClient(client)) { + if !(client.HasMode(modes.Operator) || channel.hasClient(client)) { rb.Add(nil, client.server.name, ERR_NOTONCHANNEL, channel.name, client.t("You're not on that channel")) return } @@ -823,7 +809,7 @@ func (channel *Channel) Kick(client *Client, target *Client, comment string, rb // Invite invites the given client to the channel, if the inviter can do so. func (channel *Channel) Invite(invitee *Client, inviter *Client, rb *ResponseBuffer) { - if channel.flags[modes.InviteOnly] && !channel.ClientIsAtLeast(inviter, modes.ChannelOperator) { + if channel.flags.HasMode(modes.InviteOnly) && !channel.ClientIsAtLeast(inviter, modes.ChannelOperator) { rb.Add(nil, inviter.server.name, ERR_CHANOPRIVSNEEDED, channel.name, inviter.t("You're not a channel operator")) return } @@ -834,7 +820,7 @@ func (channel *Channel) Invite(invitee *Client, inviter *Client, rb *ResponseBuf } //TODO(dan): handle this more nicely, keep a list of last X invited channels on invitee rather than explicitly modifying the invite list? - if channel.flags[modes.InviteOnly] { + if channel.flags.HasMode(modes.InviteOnly) { nmc := invitee.NickCasefolded() channel.stateMutex.Lock() channel.lists[modes.InviteMask].Add(nmc) @@ -850,7 +836,7 @@ func (channel *Channel) Invite(invitee *Client, inviter *Client, rb *ResponseBuf //TODO(dan): should inviter.server.name here be inviter.nickMaskString ? rb.Add(nil, inviter.server.name, RPL_INVITING, invitee.nick, channel.name) invitee.Send(nil, inviter.nickMaskString, "INVITE", invitee.nick, channel.name) - if invitee.flags[modes.Away] { + if invitee.HasMode(modes.Away) { rb.Add(nil, inviter.server.name, RPL_AWAY, invitee.nick, invitee.awayMessage) } } diff --git a/irc/chanserv.go b/irc/chanserv.go index ad9225b4..e767cfa8 100644 --- a/irc/chanserv.go +++ b/irc/chanserv.go @@ -199,7 +199,7 @@ func csOpHandler(server *Server, client *Client, command, params string, rb *Res if client == target { givenMode = modes.ChannelFounder } - change := channelInfo.applyModeMemberNoMutex(target, givenMode, modes.Add, client.NickCasefolded(), rb) + change := channelInfo.applyModeToMember(target, givenMode, modes.Add, client.NickCasefolded(), rb) if change != nil { //TODO(dan): we should change the name of String and make it return a slice here //TODO(dan): unify this code with code in modes.go @@ -260,7 +260,7 @@ func csRegisterHandler(server *Server, client *Client, command, params string, r server.snomasks.Send(sno.LocalChannels, fmt.Sprintf(ircfmt.Unescape("Channel registered $c[grey][$r%s$c[grey]] by $c[grey][$r%s$c[grey]]"), channelName, client.nickMaskString)) // give them founder privs - change := channelInfo.applyModeMemberNoMutex(client, modes.ChannelFounder, modes.Add, client.NickCasefolded(), rb) + change := channelInfo.applyModeToMember(client, modes.ChannelFounder, modes.Add, client.NickCasefolded(), rb) if change != nil { //TODO(dan): we should change the name of String and make it return a slice here //TODO(dan): unify this code with code in modes.go diff --git a/irc/client.go b/irc/client.go index 3eabceb2..9d808ff2 100644 --- a/irc/client.go +++ b/irc/client.go @@ -50,7 +50,7 @@ type Client struct { ctime time.Time exitedSnomaskSent bool fakelag *Fakelag - flags map[modes.Mode]bool + flags *modes.ModeSet hasQuit bool hops int hostname string @@ -98,7 +98,7 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { capVersion: caps.Cap301, channels: make(ChannelSet), ctime: now, - flags: make(map[modes.Mode]bool), + flags: modes.NewModeSet(), server: server, socket: socket, nick: "*", // * is used until actual nick is given @@ -109,7 +109,7 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { client.recomputeMaxlens() if isTLS { - client.flags[modes.TLS] = true + client.SetMode(modes.TLS, true) // error is not useful to us here anyways so we can ignore it client.certfp, _ = client.socket.CertFP() @@ -504,13 +504,7 @@ func (client *Client) HasRoleCapabs(capabs ...string) bool { // ModeString returns the mode string for this client. func (client *Client) ModeString() (str string) { - str = "+" - - for flag := range client.flags { - str += flag.String() - } - - return + return "+" + client.flags.String() } // Friends refers to clients that share a channel with this client. diff --git a/irc/config.go b/irc/config.go index 3d1b6504..47b397b2 100644 --- a/irc/config.go +++ b/irc/config.go @@ -21,6 +21,7 @@ import ( "github.com/oragono/oragono/irc/custime" "github.com/oragono/oragono/irc/languages" "github.com/oragono/oragono/irc/logger" + "github.com/oragono/oragono/irc/modes" "github.com/oragono/oragono/irc/passwd" "github.com/oragono/oragono/irc/utils" "gopkg.in/yaml.v2" @@ -352,7 +353,7 @@ type Oper struct { WhoisLine string Vhost string Pass []byte - Modes string + Modes []modes.ModeChange } // Operators returns a map of operator configs from the given OperClass and config. @@ -379,7 +380,12 @@ func (conf *Config) Operators(oc *map[string]OperClass) (map[string]Oper, error) } else { oper.WhoisLine = class.WhoisLine } - oper.Modes = strings.TrimSpace(opConf.Modes) + modeStr := strings.TrimSpace(opConf.Modes) + modeChanges, unknownChanges := modes.ParseUserModeChanges(strings.Split(modeStr, " ")...) + if len(unknownChanges) > 0 { + return nil, fmt.Errorf("Could not load operator [%s] due to unknown modes %v", name, unknownChanges) + } + oper.Modes = modeChanges // successful, attach to list of opers operators[name] = oper diff --git a/irc/gateways.go b/irc/gateways.go index 83c2dc74..c162583a 100644 --- a/irc/gateways.go +++ b/irc/gateways.go @@ -81,11 +81,7 @@ func (client *Client) ApplyProxiedIP(proxiedIP string, tls bool) (exiting bool) // set tls info client.certfp = "" - if tls { - client.flags[modes.TLS] = true - } else { - delete(client.flags, modes.TLS) - } + client.SetMode(modes.TLS, tls) return false } diff --git a/irc/getters.go b/irc/getters.go index 9d1c6498..24a0455a 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -188,9 +188,12 @@ func (client *Client) SetPreregNick(preregNick string) { } func (client *Client) HasMode(mode modes.Mode) bool { - client.stateMutex.RLock() - defer client.stateMutex.RUnlock() - return client.flags[mode] + // client.flags has its own synch + return client.flags.HasMode(mode) +} + +func (client *Client) SetMode(mode modes.Mode, on bool) bool { + return client.flags.SetMode(mode, on) } func (client *Client) Channels() (result []*Channel) { @@ -260,29 +263,8 @@ func (channel *Channel) setKey(key string) { channel.key = key } -func (channel *Channel) HasMode(mode modes.Mode) bool { - channel.stateMutex.RLock() - defer channel.stateMutex.RUnlock() - return channel.flags[mode] -} - func (channel *Channel) Founder() string { channel.stateMutex.RLock() defer channel.stateMutex.RUnlock() return channel.registeredFounder } - -// set a channel mode, return whether it was already set -func (channel *Channel) setMode(mode modes.Mode, enable bool) (already bool) { - channel.stateMutex.Lock() - already = (channel.flags[mode] == enable) - if !already { - if enable { - channel.flags[mode] = true - } else { - delete(channel.flags, mode) - } - } - channel.stateMutex.Unlock() - return -} diff --git a/irc/handlers.go b/irc/handlers.go index ac753e80..bcde94eb 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -405,15 +405,11 @@ func awayHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp } } - if isAway { - client.flags[modes.Away] = true - } else { - delete(client.flags, modes.Away) - } + client.SetMode(modes.Away, isAway) client.awayMessage = text var op modes.ModeOp - if client.flags[modes.Away] { + if isAway { op = modes.Add rb.Add(nil, server.name, RPL_NOWAWAY, client.nick, client.t("You have been marked as being away")) } else { @@ -429,7 +425,7 @@ func awayHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp // dispatch away-notify for friend := range client.Friends(caps.AwayNotify) { - if client.flags[modes.Away] { + if isAway { friend.SendFromClient("", client, nil, "AWAY", client.awayMessage) } else { friend.SendFromClient("", client, nil, "AWAY") @@ -777,7 +773,7 @@ Get an explanation of , or "index" for a list of help topics.`), rb) // handle index if argument == "index" { - if client.flags[modes.Operator] { + if client.HasMode(modes.Operator) { client.sendHelp("HELP", GetHelpIndex(client.languages, HelpIndexOpers), rb) } else { client.sendHelp("HELP", GetHelpIndex(client.languages, HelpIndex), rb) @@ -787,7 +783,7 @@ Get an explanation of , or "index" for a list of help topics.`), rb) helpHandler, exists := Help[argument] - if exists && (!helpHandler.oper || (helpHandler.oper && client.flags[modes.Operator])) { + if exists && (!helpHandler.oper || (helpHandler.oper && client.HasMode(modes.Operator))) { if helpHandler.textGenerator != nil { client.sendHelp(strings.ToUpper(argument), client.t(helpHandler.textGenerator(client)), rb) } else { @@ -1257,9 +1253,10 @@ func listHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp } } + clientIsOp := client.HasMode(modes.Operator) if len(channels) == 0 { for _, channel := range server.channels.Channels() { - if !client.flags[modes.Operator] && channel.flags[modes.Secret] { + if !clientIsOp && channel.flags.HasMode(modes.Secret) { continue } if matcher.Matches(channel) { @@ -1268,14 +1265,14 @@ func listHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp } } else { // limit regular users to only listing one channel - if !client.flags[modes.Operator] { + if !clientIsOp { channels = channels[:1] } for _, chname := range channels { casefoldedChname, err := CasefoldChannel(chname) channel := server.channels.Get(casefoldedChname) - if err != nil || channel == nil || (!client.flags[modes.Operator] && channel.flags[modes.Secret]) { + if err != nil || channel == nil || (!clientIsOp && channel.flags.HasMode(modes.Secret)) { if len(chname) > 0 { rb.Add(nil, server.name, ERR_NOSUCHCHANNEL, client.nick, chname, client.t("No such channel")) } @@ -1329,7 +1326,7 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res if 1 < len(msg.Params) { // parse out real mode changes params := msg.Params[1:] - changes, unknown := ParseChannelModeChanges(params...) + changes, unknown := modes.ParseChannelModeChanges(params...) // alert for unknown mode changes for char := range unknown { @@ -1415,14 +1412,14 @@ func umodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res } // apply mode changes - applied = target.applyUserModeChanges(msg.Command == "SAMODE", changes) + applied = ApplyUserModeChanges(client, changes, msg.Command == "SAMODE") } if len(applied) > 0 { rb.Add(nil, client.nickMaskString, "MODE", targetNick, applied.String()) } else if hasPrivs { rb.Add(nil, target.nickMaskString, RPL_UMODEIS, targetNick, target.ModeString()) - if client.flags[modes.LocalOperator] || client.flags[modes.Operator] { + if client.HasMode(modes.LocalOperator) || client.HasMode(modes.Operator) { masks := server.snomasks.String(client) if 0 < len(masks) { rb.Add(nil, target.nickMaskString, RPL_SNOMASKIS, targetNick, masks, client.t("Server notice masks")) @@ -1670,7 +1667,7 @@ func noticeHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re msgid := server.generateMessageID() // restrict messages appropriately when +R is set // intentionally make the sending user think the message went through fine - if !user.flags[modes.RegisteredOnly] || client.registered { + if !user.HasMode(modes.RegisteredOnly) || client.LoggedIntoAccount() { user.SendSplitMsgFromClient(msgid, client, clientOnlyTags, "NOTICE", user.nick, splitMsg) } if client.capabilities.Has(caps.EchoMessage) { @@ -1731,7 +1728,7 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp rb.Add(nil, server.name, ERR_PASSWDMISMATCH, client.nick, client.t("Password incorrect")) return true } - if client.flags[modes.Operator] == true { + if client.HasMode(modes.Operator) == true { rb.Add(nil, server.name, ERR_UNKNOWNERROR, "OPER", client.t("You're already opered-up!")) return false } @@ -1760,37 +1757,23 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp client.updateNickMask("") } - // set new modes - var applied modes.ModeChanges - if 0 < len(oper.Modes) { - modeChanges, unknownChanges := modes.ParseUserModeChanges(strings.Split(oper.Modes, " ")...) - applied = client.applyUserModeChanges(true, modeChanges) - if 0 < len(unknownChanges) { - var runes string - for r := range unknownChanges { - runes += string(r) - } - rb.Notice(fmt.Sprintf(client.t("Could not apply mode changes: +%s"), runes)) - } - } - - rb.Add(nil, server.name, RPL_YOUREOPER, client.nick, client.t("You are now an IRC operator")) - - applied = append(applied, modes.ModeChange{ + // set new modes: modes.Operator, plus anything specified in the config + modeChanges := make([]modes.ModeChange, len(oper.Modes)+1) + modeChanges[0] = modes.ModeChange{ Mode: modes.Operator, Op: modes.Add, - }) + } + copy(modeChanges[1:], oper.Modes) + applied := ApplyUserModeChanges(client, modeChanges, true) + + rb.Add(nil, server.name, RPL_YOUREOPER, client.nick, client.t("You are now an IRC operator")) rb.Add(nil, server.name, "MODE", client.nick, applied.String()) server.snomasks.Send(sno.LocalOpers, fmt.Sprintf(ircfmt.Unescape("Client opered up $c[grey][$r%s$c[grey], $r%s$c[grey]]"), client.nickMaskString, client.operName)) - // increase oper count - server.stats.ChangeOperators(1) - // client may now be unthrottled by the fakelag system client.resetFakelag() - client.flags[modes.Operator] = true return false } @@ -1905,13 +1888,13 @@ func privmsgHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *R msgid := server.generateMessageID() // restrict messages appropriately when +R is set // intentionally make the sending user think the message went through fine - if !user.flags[modes.RegisteredOnly] || client.registered { + if !user.HasMode(modes.RegisteredOnly) || client.LoggedIntoAccount() { user.SendSplitMsgFromClient(msgid, client, clientOnlyTags, "PRIVMSG", user.nick, splitMsg) } if client.capabilities.Has(caps.EchoMessage) { rb.AddSplitMessageFromClient(msgid, client, clientOnlyTags, "PRIVMSG", user.nick, splitMsg) } - if user.flags[modes.Away] { + if user.HasMode(modes.Away) { //TODO(dan): possibly implement cooldown of away notifications to users rb.Add(nil, server.name, RPL_AWAY, user.nick, user.awayMessage) } @@ -2159,7 +2142,7 @@ func tagmsgHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re if client.capabilities.Has(caps.EchoMessage) { rb.AddFromClient(msgid, client, clientOnlyTags, "TAGMSG", user.nick) } - if user.flags[modes.Away] { + if user.HasMode(modes.Away) { //TODO(dan): possibly implement cooldown of away notifications to users rb.Add(nil, server.name, RPL_AWAY, user.nick, user.awayMessage) } @@ -2355,10 +2338,10 @@ func userhostHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb * var isOper, isAway string - if target.flags[modes.Operator] { + if target.HasMode(modes.Operator) { isOper = "*" } - if target.flags[modes.Away] { + if target.HasMode(modes.Away) { isAway = "-" } else { isAway = "+" @@ -2399,7 +2382,7 @@ func webircHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re lkey := strings.ToLower(key) if lkey == "tls" || lkey == "secure" { // only accept "tls" flag if the gateway's connection to us is secure as well - if client.flags[modes.TLS] || utils.AddrIsLocal(client.socket.conn.RemoteAddr()) { + if client.HasMode(modes.TLS) || utils.AddrIsLocal(client.socket.conn.RemoteAddr()) { secure = true } } @@ -2488,7 +2471,7 @@ func whoisHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Res return false } - if client.flags[modes.Operator] { + if client.HasMode(modes.Operator) { masks := strings.Split(masksString, ",") for _, mask := range masks { casefoldedMask, err := Casefold(mask) diff --git a/irc/modes.go b/irc/modes.go index d8cc95b8..1149bb2b 100644 --- a/irc/modes.go +++ b/irc/modes.go @@ -21,8 +21,8 @@ var ( } ) -// applyUserModeChanges applies the given changes, and returns the applied changes. -func (client *Client) applyUserModeChanges(force bool, changes modes.ModeChanges) modes.ModeChanges { +// ApplyUserModeChanges applies the given changes, and returns the applied changes. +func ApplyUserModeChanges(client *Client, changes modes.ModeChanges, force bool) modes.ModeChanges { applied := make(modes.ModeChanges, 0) for _, change := range changes { @@ -34,36 +34,28 @@ func (client *Client) applyUserModeChanges(force bool, changes modes.ModeChanges continue } - if client.flags[change.Mode] { - continue + if changed := client.SetMode(change.Mode, true); changed { + if change.Mode == modes.Invisible { + client.server.stats.ChangeInvisible(1) + } else if change.Mode == modes.Operator || change.Mode == modes.LocalOperator { + client.server.stats.ChangeOperators(1) + } + applied = append(applied, change) } - if change.Mode == modes.Invisible { - client.server.stats.ChangeInvisible(1) - } - - client.flags[change.Mode] = true - applied = append(applied, change) - case modes.Remove: - if !client.flags[change.Mode] { - continue + if changed := client.SetMode(change.Mode, false); changed { + if change.Mode == modes.Invisible { + client.server.stats.ChangeInvisible(-1) + } else if change.Mode == modes.Operator || change.Mode == modes.LocalOperator { + client.server.stats.ChangeOperators(-1) + } + applied = append(applied, change) } - - if change.Mode == modes.Invisible { - client.server.stats.ChangeInvisible(-1) - } - - if change.Mode == modes.Operator || change.Mode == modes.LocalOperator { - client.server.stats.ChangeOperators(-1) - } - - delete(client.flags, change.Mode) - applied = append(applied, change) } case modes.ServerNotice: - if !client.flags[modes.Operator] { + if !client.HasMode(modes.Operator) { continue } var masks []sno.Mask @@ -101,7 +93,7 @@ func ParseDefaultChannelModes(config *Config) modes.Modes { return DefaultChannelModes } modeChangeStrings := strings.Split(strings.TrimSpace(*config.Channels.DefaultModes), " ") - modeChanges, _ := ParseChannelModeChanges(modeChangeStrings...) + modeChanges, _ := modes.ParseChannelModeChanges(modeChangeStrings...) defaultChannelModes := make(modes.Modes, 0) for _, modeChange := range modeChanges { if modeChange.Op == modes.Add { @@ -111,83 +103,6 @@ func ParseDefaultChannelModes(config *Config) modes.Modes { return defaultChannelModes } -// ParseChannelModeChanges returns the valid changes, and the list of unknown chars. -func ParseChannelModeChanges(params ...string) (modes.ModeChanges, map[rune]bool) { - changes := make(modes.ModeChanges, 0) - unknown := make(map[rune]bool) - - op := modes.List - - if 0 < len(params) { - modeArg := params[0] - skipArgs := 1 - - for _, mode := range modeArg { - if mode == '-' || mode == '+' { - op = modes.ModeOp(mode) - continue - } - change := modes.ModeChange{ - Mode: modes.Mode(mode), - Op: op, - } - - // put arg into modechange if needed - switch modes.Mode(mode) { - case modes.BanMask, modes.ExceptMask, modes.InviteMask: - if len(params) > skipArgs { - change.Arg = params[skipArgs] - skipArgs++ - } else { - change.Op = modes.List - } - case modes.ChannelFounder, modes.ChannelAdmin, modes.ChannelOperator, modes.Halfop, modes.Voice: - if len(params) > skipArgs { - change.Arg = params[skipArgs] - skipArgs++ - } else { - continue - } - case modes.Key, modes.UserLimit: - // don't require value when removing - if change.Op == modes.Add { - if len(params) > skipArgs { - change.Arg = params[skipArgs] - skipArgs++ - } else { - continue - } - } - } - - var isKnown bool - for _, supportedMode := range modes.SupportedChannelModes { - if rune(supportedMode) == mode { - isKnown = true - break - } - } - for _, supportedMode := range modes.ChannelPrivModes { - if rune(supportedMode) == mode { - isKnown = true - break - } - } - if mode == rune(modes.Voice) { - isKnown = true - } - if !isKnown { - unknown[mode] = true - continue - } - - changes = append(changes, change) - } - } - - return changes, unknown -} - // ApplyChannelModeChanges applies a given set of mode changes. func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, changes modes.ModeChanges, rb *ResponseBuffer) modes.ModeChanges { // so we only output one warning for each list type when full @@ -208,15 +123,17 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c } switch change.Mode { case modes.ChannelFounder, modes.ChannelAdmin, modes.ChannelOperator, modes.Halfop, modes.Voice: - // Admins can't give other people Admin or remove it from others - if change.Mode == modes.ChannelAdmin { - return false - } + // List on these modes is a no-op anyway if change.Op == modes.List { return true } cfarg, _ := CasefoldName(change.Arg) - if change.Op == modes.Remove && cfarg == client.nickCasefolded { + isSelfChange := cfarg == client.NickCasefolded() + // Admins can't give other people Admin or remove it from others + if change.Mode == modes.ChannelAdmin && !isSelfChange { + return false + } + if change.Op == modes.Remove && isSelfChange { // "There is no restriction, however, on anyone `deopping' themselves" // return true @@ -299,8 +216,7 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c continue } - already := channel.setMode(change.Mode, change.Op == modes.Add) - if !already { + if changed := channel.flags.SetMode(change.Mode, change.Op == modes.Add); changed { applied = append(applied, change) } @@ -309,7 +225,13 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c continue } - change := channel.applyModeMemberNoMutex(client, change.Mode, change.Op, change.Arg, rb) + nick := change.Arg + if nick == "" { + rb.Add(nil, client.server.name, ERR_NEEDMOREPARAMS, "MODE", client.t("Not enough parameters")) + return nil + } + + change := channel.applyModeToMember(client, change.Mode, change.Op, nick, rb) if change != nil { applied = append(applied, *change) } diff --git a/irc/modes/modes.go b/irc/modes/modes.go index 47c0aa91..287dbd44 100644 --- a/irc/modes/modes.go +++ b/irc/modes/modes.go @@ -7,6 +7,7 @@ package modes import ( "strings" + "sync" ) var ( @@ -247,34 +248,156 @@ func ParseUserModeChanges(params ...string) (ModeChanges, map[rune]bool) { return changes, unknown } +// ParseChannelModeChanges returns the valid changes, and the list of unknown chars. +func ParseChannelModeChanges(params ...string) (ModeChanges, map[rune]bool) { + changes := make(ModeChanges, 0) + unknown := make(map[rune]bool) + + op := List + + if 0 < len(params) { + modeArg := params[0] + skipArgs := 1 + + for _, mode := range modeArg { + if mode == '-' || mode == '+' { + op = ModeOp(mode) + continue + } + change := ModeChange{ + Mode: Mode(mode), + Op: op, + } + + // put arg into modechange if needed + switch Mode(mode) { + case BanMask, ExceptMask, InviteMask: + if len(params) > skipArgs { + change.Arg = params[skipArgs] + skipArgs++ + } else { + change.Op = List + } + case ChannelFounder, ChannelAdmin, ChannelOperator, Halfop, Voice: + if len(params) > skipArgs { + change.Arg = params[skipArgs] + skipArgs++ + } else { + continue + } + case Key, UserLimit: + // don't require value when removing + if change.Op == Add { + if len(params) > skipArgs { + change.Arg = params[skipArgs] + skipArgs++ + } else { + continue + } + } + } + + var isKnown bool + for _, supportedMode := range SupportedChannelModes { + if rune(supportedMode) == mode { + isKnown = true + break + } + } + for _, supportedMode := range ChannelPrivModes { + if rune(supportedMode) == mode { + isKnown = true + break + } + } + if mode == rune(Voice) { + isKnown = true + } + if !isKnown { + unknown[mode] = true + continue + } + + changes = append(changes, change) + } + } + + return changes, unknown +} + // ModeSet holds a set of modes. -type ModeSet map[Mode]bool +type ModeSet struct { + sync.RWMutex // tier 0 + modes map[Mode]bool +} + +// returns a pointer to a new ModeSet +func NewModeSet() *ModeSet { + return &ModeSet{ + modes: make(map[Mode]bool), + } +} + +// test whether `mode` is set +func (set *ModeSet) HasMode(mode Mode) bool { + set.RLock() + defer set.RUnlock() + return set.modes[mode] +} + +// set `mode` to be on or off, return whether the value actually changed +func (set *ModeSet) SetMode(mode Mode, on bool) (applied bool) { + set.Lock() + defer set.Unlock() + + previouslyOn := set.modes[mode] + needsApply := (on != previouslyOn) + if on && needsApply { + set.modes[mode] = true + } else if !on && needsApply { + delete(set.modes, mode) + } + return needsApply +} + +// return the modes in the set as a slice +func (set *ModeSet) AllModes() (result []Mode) { + set.RLock() + defer set.RUnlock() + + for mode := range set.modes { + result = append(result, mode) + } + return +} // String returns the modes in this set. -func (set ModeSet) String() string { - if len(set) == 0 { +func (set *ModeSet) String() string { + set.RLock() + defer set.RUnlock() + + if len(set.modes) == 0 { return "" } - strs := make([]string, len(set)) - index := 0 - for mode := range set { - strs[index] = mode.String() - index++ + var result []byte + for mode := range set.modes { + result = append(result, mode.String()...) } - return strings.Join(strs, "") + return string(result) } // Prefixes returns a list of prefixes for the given set of channel modes. -func (set ModeSet) Prefixes(isMultiPrefix bool) string { - var prefixes string +func (set *ModeSet) Prefixes(isMultiPrefix bool) (prefixes string) { + set.RLock() + defer set.RUnlock() // add prefixes in order from highest to lowest privs for _, mode := range ChannelPrivModes { - if set[mode] { + if set.modes[mode] { prefixes += ChannelModePrefixes[mode] } } - if set[Voice] { + if set.modes[Voice] { prefixes += ChannelModePrefixes[Voice] } diff --git a/irc/modes/modes_test.go b/irc/modes/modes_test.go new file mode 100644 index 00000000..8f20bdfc --- /dev/null +++ b/irc/modes/modes_test.go @@ -0,0 +1,37 @@ +// Copyright (c) 2018 Shivaram Lingamneni +// released under the MIT license + +package modes + +import ( + "reflect" + "testing" +) + +func TestSetMode(t *testing.T) { + set := NewModeSet() + + if applied := set.SetMode(Invisible, false); applied != false { + t.Errorf("all modes should be false by default") + } + + if applied := set.SetMode(Invisible, true); applied != true { + t.Errorf("initial SetMode call should return true") + } + + set.SetMode(Operator, true) + + if applied := set.SetMode(Invisible, true); applied != false { + t.Errorf("redundant SetMode call should return false") + } + + expected1 := []Mode{Invisible, Operator} + expected2 := []Mode{Operator, Invisible} + if allModes := set.AllModes(); !(reflect.DeepEqual(allModes, expected1) || reflect.DeepEqual(allModes, expected2)) { + t.Errorf("unexpected AllModes value: %v", allModes) + } + + if modeString := set.String(); !(modeString == "io" || modeString == "oi") { + t.Errorf("unexpected modestring: %s", modeString) + } +} diff --git a/irc/roleplay.go b/irc/roleplay.go index 9c3346ed..80d39b19 100644 --- a/irc/roleplay.go +++ b/irc/roleplay.go @@ -35,7 +35,7 @@ func sendRoleplayMessage(server *Server, client *Client, source string, targetSt return } - if !channel.flags[modes.ChanRoleplaying] { + if !channel.flags.HasMode(modes.ChanRoleplaying) { rb.Add(nil, client.server.name, ERR_CANNOTSENDRP, channel.name, client.t("Channel doesn't have roleplaying mode available")) return } @@ -58,7 +58,7 @@ func sendRoleplayMessage(server *Server, client *Client, source string, targetSt return } - if !user.flags[modes.UserRoleplaying] { + if !user.HasMode(modes.UserRoleplaying) { rb.Add(nil, client.server.name, ERR_CANNOTSENDRP, user.nick, client.t("User doesn't have roleplaying mode enabled")) return } @@ -67,7 +67,7 @@ func sendRoleplayMessage(server *Server, client *Client, source string, targetSt if client.capabilities.Has(caps.EchoMessage) { rb.Add(nil, source, "PRIVMSG", user.nick, message) } - if user.flags[modes.Away] { + if user.HasMode(modes.Away) { //TODO(dan): possibly implement cooldown of away notifications to users rb.Add(nil, server.name, RPL_AWAY, user.nick, user.awayMessage) } diff --git a/irc/server.go b/irc/server.go index aa5ba90f..c0342f18 100644 --- a/irc/server.go +++ b/irc/server.go @@ -637,8 +637,8 @@ func (client *Client) WhoisChannelsNames(target *Client) []string { var chstrs []string for _, channel := range target.Channels() { // channel is secret and the target can't see it - if !client.flags[modes.Operator] { - if (target.HasMode(modes.Invisible) || channel.HasMode(modes.Secret)) && !channel.hasClient(client) { + if !client.HasMode(modes.Operator) { + if (target.HasMode(modes.Invisible) || channel.flags.HasMode(modes.Secret)) && !channel.hasClient(client) { continue } } @@ -660,16 +660,16 @@ func (client *Client) getWhoisOf(target *Client, rb *ResponseBuffer) { if target.class != nil { rb.Add(nil, client.server.name, RPL_WHOISOPERATOR, client.nick, target.nick, target.whoisLine) } - if client.flags[modes.Operator] || client == target { + if client.HasMode(modes.Operator) || client == target { rb.Add(nil, client.server.name, RPL_WHOISACTUALLY, client.nick, target.nick, fmt.Sprintf("%s@%s", target.username, utils.LookupHostname(target.IPString())), target.IPString(), client.t("Actual user@host, Actual IP")) } - if target.flags[modes.TLS] { + if target.HasMode(modes.TLS) { rb.Add(nil, client.server.name, RPL_WHOISSECURE, client.nick, target.nick, client.t("is using a secure connection")) } if target.LoggedIntoAccount() { rb.Add(nil, client.server.name, RPL_WHOISACCOUNT, client.nick, target.AccountName(), client.t("is logged in as")) } - if target.flags[modes.Bot] { + if target.HasMode(modes.Bot) { rb.Add(nil, client.server.name, RPL_WHOISBOT, client.nick, target.nick, ircfmt.Unescape(fmt.Sprintf(client.t("is a $bBot$b on %s"), client.server.networkName))) } @@ -682,7 +682,7 @@ func (client *Client) getWhoisOf(target *Client, rb *ResponseBuffer) { rb.Add(nil, client.server.name, RPL_WHOISLANGUAGE, params...) } - if target.certfp != "" && (client.flags[modes.Operator] || client == target) { + if target.certfp != "" && (client.HasMode(modes.Operator) || client == target) { rb.Add(nil, client.server.name, RPL_WHOISCERTFP, client.nick, target.nick, fmt.Sprintf(client.t("has client certificate fingerprint %s"), target.certfp)) } rb.Add(nil, client.server.name, RPL_WHOISIDLE, client.nick, target.nick, strconv.FormatUint(target.IdleSeconds(), 10), strconv.FormatInt(target.SignonTime(), 10), client.t("seconds idle, signon time")) @@ -713,7 +713,7 @@ func (target *Client) rplWhoReply(channel *Channel, client *Client, rb *Response func whoChannel(client *Client, channel *Channel, friends ClientSet, rb *ResponseBuffer) { for _, member := range channel.Members() { - if !client.flags[modes.Invisible] || friends[client] { + if !client.HasMode(modes.Invisible) || friends[client] { client.rplWhoReply(channel, member, rb) } } @@ -1209,7 +1209,7 @@ func (matcher *elistMatcher) Matches(channel *Channel) bool { func (target *Client) RplList(channel *Channel, rb *ResponseBuffer) { // get the correct number of channel members var memberCount int - if target.flags[modes.Operator] || channel.hasClient(target) { + if target.HasMode(modes.Operator) || channel.hasClient(target) { memberCount = len(channel.Members()) } else { for _, member := range channel.Members() { diff --git a/irc/types.go b/irc/types.go index d4e9e018..9ef6b51a 100644 --- a/irc/types.go +++ b/irc/types.go @@ -26,11 +26,11 @@ func (clients ClientSet) Has(client *Client) bool { } // MemberSet is a set of members with modes. -type MemberSet map[*Client]modes.ModeSet +type MemberSet map[*Client]*modes.ModeSet // Add adds the given client to this set. func (members MemberSet) Add(member *Client) { - members[member] = make(modes.ModeSet) + members[member] = modes.NewModeSet() } // Remove removes the given client from this set. @@ -44,19 +44,10 @@ func (members MemberSet) Has(member *Client) bool { return ok } -// HasMode returns true if the given client is in this set with the given mode. -func (members MemberSet) HasMode(member *Client, mode modes.Mode) bool { - modes, ok := members[member] - if !ok { - return false - } - return modes[mode] -} - // AnyHasMode returns true if any of our clients has the given mode. func (members MemberSet) AnyHasMode(mode modes.Mode) bool { for _, modes := range members { - if modes[mode] { + if modes.HasMode(mode) { return true } } From 43b90f2a853804656322d914ccb3584532c762e4 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Apr 2018 20:36:50 -0400 Subject: [PATCH 11/12] have travis enforce gofmt, #253 --- .travis.gofmt.sh | 10 ++++++++++ .travis.yml | 1 + irc/database.go | 4 ++-- 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 .travis.gofmt.sh diff --git a/.travis.gofmt.sh b/.travis.gofmt.sh new file mode 100644 index 00000000..708e86d7 --- /dev/null +++ b/.travis.gofmt.sh @@ -0,0 +1,10 @@ +#!/bin/bash + +# exclude vendor/ +SOURCES="./oragono.go ./irc" + +if [ -n "$(gofmt -s -l $SOURCES)" ]; then + echo "Go code is not formatted correctly with \`gofmt -s\`:" + gofmt -s -d $SOURCES + exit 1 +fi diff --git a/.travis.yml b/.travis.yml index 4c7b9780..9a913bb0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,3 +7,4 @@ script: - tar -xzf goreleaser_Linux_x86_64.tar.gz -C $GOPATH/bin - make - make test +- bash ./.travis.gofmt.sh diff --git a/irc/database.go b/irc/database.go index 54f9f469..2e54a8d1 100644 --- a/irc/database.go +++ b/irc/database.go @@ -282,12 +282,12 @@ func schemaChangeV2ToV3(config *Config, tx *buntdb.Tx) error { func init() { allChanges := []SchemaChange{ - SchemaChange{ + { InitialVersion: "1", TargetVersion: "2", Changer: schemaChangeV1toV2, }, - SchemaChange{ + { InitialVersion: "2", TargetVersion: "3", Changer: schemaChangeV2ToV3, From abbbd2c8995142291370f6dae4aa80d1de4747d4 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 23 Apr 2018 20:03:26 -0400 Subject: [PATCH 12/12] review fix --- irc/channel.go | 2 +- irc/modes.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/irc/channel.go b/irc/channel.go index 74cf4be2..4c626fb5 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -690,7 +690,7 @@ func (channel *Channel) applyModeToMember(client *Client, mode modes.Mode, op mo channel.stateMutex.Lock() modeset, exists := channel.members[target] if exists { - if applied := modeset.SetMode(mode, op == modes.Add); applied { + if modeset.SetMode(mode, op == modes.Add) { result = &modes.ModeChange{ Op: op, Mode: mode, diff --git a/irc/modes.go b/irc/modes.go index 1149bb2b..576d0375 100644 --- a/irc/modes.go +++ b/irc/modes.go @@ -34,7 +34,7 @@ func ApplyUserModeChanges(client *Client, changes modes.ModeChanges, force bool) continue } - if changed := client.SetMode(change.Mode, true); changed { + if client.SetMode(change.Mode, true) { if change.Mode == modes.Invisible { client.server.stats.ChangeInvisible(1) } else if change.Mode == modes.Operator || change.Mode == modes.LocalOperator { @@ -44,7 +44,7 @@ func ApplyUserModeChanges(client *Client, changes modes.ModeChanges, force bool) } case modes.Remove: - if changed := client.SetMode(change.Mode, false); changed { + if client.SetMode(change.Mode, false) { if change.Mode == modes.Invisible { client.server.stats.ChangeInvisible(-1) } else if change.Mode == modes.Operator || change.Mode == modes.LocalOperator { @@ -216,7 +216,7 @@ func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, c continue } - if changed := channel.flags.SetMode(change.Mode, change.Op == modes.Add); changed { + if channel.flags.SetMode(change.Mode, change.Op == modes.Add) { applied = append(applied, change) }