From 25c4eb2996b3d7b44e2d6e67a293e5ea00912d68 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sat, 21 Dec 2019 20:19:19 -0500 Subject: [PATCH 1/4] fix #702 --- irc/accounts.go | 6 +++--- irc/server.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index 530f3dff..6c827928 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -65,12 +65,12 @@ func (am *AccountManager) Initialize(server *Server) { am.accountToMethod = make(map[string]NickEnforcementMethod) am.server = server - am.buildNickToAccountIndex() + am.buildNickToAccountIndex(server.Config()) am.initVHostRequestQueue() } -func (am *AccountManager) buildNickToAccountIndex() { - if !am.server.AccountConfig().NickReservation.Enabled { +func (am *AccountManager) buildNickToAccountIndex(config *Config) { + if !config.Accounts.NickReservation.Enabled { return } diff --git a/irc/server.go b/irc/server.go index bb498139..bff1c482 100644 --- a/irc/server.go +++ b/irc/server.go @@ -658,7 +658,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { nickReservationPreviouslyDisabled := oldConfig != nil && !oldConfig.Accounts.NickReservation.Enabled nickReservationNowEnabled := config.Accounts.NickReservation.Enabled if nickReservationPreviouslyDisabled && nickReservationNowEnabled { - server.accounts.buildNickToAccountIndex() + server.accounts.buildNickToAccountIndex(config) } hsPreviouslyDisabled := oldConfig != nil && !oldConfig.Accounts.VHosts.Enabled From 26ca016c668090c5e634a2d8ad2ddcd99e5bd7f3 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sat, 21 Dec 2019 20:26:40 -0500 Subject: [PATCH 2/4] fix the analogous issue for vhosts --- irc/accounts.go | 9 +++++---- irc/server.go | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index 6c827928..d3b0b9fc 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -65,8 +65,9 @@ func (am *AccountManager) Initialize(server *Server) { am.accountToMethod = make(map[string]NickEnforcementMethod) am.server = server - am.buildNickToAccountIndex(server.Config()) - am.initVHostRequestQueue() + config := server.Config() + am.buildNickToAccountIndex(config) + am.initVHostRequestQueue(config) } func (am *AccountManager) buildNickToAccountIndex(config *Config) { @@ -135,8 +136,8 @@ func (am *AccountManager) buildNickToAccountIndex(config *Config) { } } -func (am *AccountManager) initVHostRequestQueue() { - if !am.server.AccountConfig().VHosts.Enabled { +func (am *AccountManager) initVHostRequestQueue(config *Config) { + if !config.Accounts.VHosts.Enabled { return } diff --git a/irc/server.go b/irc/server.go index bff1c482..412a1e94 100644 --- a/irc/server.go +++ b/irc/server.go @@ -664,7 +664,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { hsPreviouslyDisabled := oldConfig != nil && !oldConfig.Accounts.VHosts.Enabled hsNowEnabled := config.Accounts.VHosts.Enabled if hsPreviouslyDisabled && hsNowEnabled { - server.accounts.initVHostRequestQueue() + server.accounts.initVHostRequestQueue(config) } chanRegPreviouslyDisabled := oldConfig != nil && !oldConfig.Channels.Registration.Enabled From bd6c2117e850e2b259c0c3afb63c92de5f49b271 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Dec 2019 08:11:24 -0500 Subject: [PATCH 3/4] fix analogous issue for history History couldn't be enabled by rehash if autoresize-window was nonzero. --- irc/history/history.go | 34 +++++++++++++++++++++++----------- irc/history/history_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/irc/history/history.go b/irc/history/history.go index 5f604c23..a70eb894 100644 --- a/irc/history/history.go +++ b/irc/history/history.go @@ -101,14 +101,7 @@ func NewHistoryBuffer(size int, window time.Duration) (result *Buffer) { } func (hist *Buffer) Initialize(size int, window time.Duration) { - initialSize := size - if window != 0 { - initialSize = initialAutoSize - if size < initialSize { - initialSize = size // min(initialAutoSize, size) - } - } - hist.buffer = make([]Item, initialSize) + hist.buffer = make([]Item, hist.initialSize(size, window)) hist.start = -1 hist.end = -1 hist.window = window @@ -118,6 +111,18 @@ func (hist *Buffer) Initialize(size int, window time.Duration) { hist.setEnabled(size) } +// compute the initial size for the buffer, taking into account autoresize +func (hist *Buffer) initialSize(size int, window time.Duration) (result int) { + result = size + if window != 0 { + result = initialAutoSize + if size < result { + result = size // min(initialAutoSize, size) + } + } + return +} + func (hist *Buffer) setEnabled(size int) { var enabled uint32 if size != 0 { @@ -339,12 +344,19 @@ func (list *Buffer) Resize(maximumSize int, window time.Duration) { list.maximumSize = maximumSize list.window = window - // if we're not autoresizing, we need to resize now; - // if we are autoresizing, we may need to shrink the buffer down to maximumSize, - // but we don't need to grow it now (we can just grow it on the next Add) + // three cases where we need to preemptively resize: + // (1) we are not autoresizing + // (2) the buffer is currently larger than maximumSize and needs to be shrunk + // (3) the buffer is currently smaller than the recommended initial size + // (including the case where it is currently disabled and needs to be enabled) // TODO make it possible to shrink the buffer so that it only contains `window` if window == 0 || maximumSize < len(list.buffer) { list.resize(maximumSize) + } else { + initialSize := list.initialSize(maximumSize, window) + if len(list.buffer) < initialSize { + list.resize(initialSize) + } } } diff --git a/irc/history/history_test.go b/irc/history/history_test.go index 10364437..e50cfb28 100644 --- a/irc/history/history_test.go +++ b/irc/history/history_test.go @@ -213,6 +213,35 @@ func TestAutoresize(t *testing.T) { assertEqual(atoi(items[len(items)-1].Nick), 271, t) } +// regression test for #702 +func TestEnabledByResize(t *testing.T) { + now := easyParse("2006-01-01 00:00:00Z") + // empty/disabled autoresizing buffer + buf := NewHistoryBuffer(0, time.Hour) + // enable the buffer as during a rehash + buf.Resize(128, time.Hour) + // add an item and test that it is stored and retrievable + buf.Add(autoItem(0, now)) + items := buf.Latest(0) + assertEqual(len(items), 1, t) + assertEqual(atoi(items[0].Nick), 0, t) +} + +func TestDisabledByResize(t *testing.T) { + now := easyParse("2006-01-01 00:00:00Z") + // enabled autoresizing buffer + buf := NewHistoryBuffer(128, time.Hour) + buf.Add(autoItem(0, now)) + items := buf.Latest(0) + assertEqual(len(items), 1, t) + assertEqual(atoi(items[0].Nick), 0, t) + + // disable as during a rehash, confirm that nothing can be retrieved + buf.Resize(0, time.Hour) + items = buf.Latest(0) + assertEqual(len(items), 0, t) +} + func TestRoundUp(t *testing.T) { assertEqual(roundUpToPowerOfTwo(2), 2, t) assertEqual(roundUpToPowerOfTwo(3), 4, t) From 76a8768d05e0a0a946ef71f3e5f40e08b04e5e29 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Dec 2019 08:17:56 -0500 Subject: [PATCH 4/4] make rehash-enable logic a little more uniform --- irc/channelmanager.go | 10 ++-- irc/server.go | 104 +++++++++++++++++------------------------- 2 files changed, 48 insertions(+), 66 deletions(-) diff --git a/irc/channelmanager.go b/irc/channelmanager.go index 8ceb390b..a502204b 100644 --- a/irc/channelmanager.go +++ b/irc/channelmanager.go @@ -30,12 +30,14 @@ func (cm *ChannelManager) Initialize(server *Server) { cm.chans = make(map[string]*channelManagerEntry) cm.server = server - if server.Config().Channels.Registration.Enabled { - cm.loadRegisteredChannels() - } + cm.loadRegisteredChannels(server.Config()) } -func (cm *ChannelManager) loadRegisteredChannels() { +func (cm *ChannelManager) loadRegisteredChannels(config *Config) { + if !config.Channels.Registration.Enabled { + return + } + registeredChannels := cm.server.channelRegistry.AllChannels() cm.Lock() defer cm.Unlock() diff --git a/irc/server.go b/irc/server.go index 412a1e94..1fecb005 100644 --- a/irc/server.go +++ b/irc/server.go @@ -611,18 +611,10 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { } } - // sanity checks complete, start modifying server state server.logger.Info("server", "Using config file", server.configFilename) oldConfig := server.Config() // first, reload config sections for functionality implemented in subpackages: - - server.connectionLimiter.ApplyConfig(&config.Server.IPLimits) - - tlConf := &config.Server.TorListeners - server.torLimiter.Configure(tlConf.MaxConnections, tlConf.ThrottleDuration, tlConf.MaxConnectionsPerDuration) - - // reload logging config wasLoggingRawIO := !initial && server.logger.IsLoggingRawIO() err = server.logger.ApplyConfig(config.Logging) if err != nil { @@ -632,6 +624,11 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { // notify existing clients if raw i/o logging was enabled by a rehash sendRawOutputNotice := !wasLoggingRawIO && nowLoggingRawIO + server.connectionLimiter.ApplyConfig(&config.Server.IPLimits) + + tlConf := &config.Server.TorListeners + server.torLimiter.Configure(tlConf.MaxConnections, tlConf.ThrottleDuration, tlConf.MaxConnectionsPerDuration) + // setup new and removed caps addedCaps := caps.NewSet() removedCaps := caps.NewSet() @@ -641,67 +638,50 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { server.logger.Debug("server", "Regenerating HELP indexes for new languages") server.helpIndexManager.GenerateIndices(config.languageManager) - if oldConfig != nil && config.Server.capValues[caps.Languages] != oldConfig.Server.capValues[caps.Languages] { - updatedCaps.Add(caps.Languages) - } - - // SASL - authPreviouslyEnabled := oldConfig != nil && oldConfig.Accounts.AuthenticationEnabled - if config.Accounts.AuthenticationEnabled && (oldConfig == nil || !authPreviouslyEnabled) { - // enabling SASL - addedCaps.Add(caps.SASL) - } else if !config.Accounts.AuthenticationEnabled && (oldConfig == nil || authPreviouslyEnabled) { - // disabling SASL - removedCaps.Add(caps.SASL) - } - - nickReservationPreviouslyDisabled := oldConfig != nil && !oldConfig.Accounts.NickReservation.Enabled - nickReservationNowEnabled := config.Accounts.NickReservation.Enabled - if nickReservationPreviouslyDisabled && nickReservationNowEnabled { - server.accounts.buildNickToAccountIndex(config) - } - - hsPreviouslyDisabled := oldConfig != nil && !oldConfig.Accounts.VHosts.Enabled - hsNowEnabled := config.Accounts.VHosts.Enabled - if hsPreviouslyDisabled && hsNowEnabled { - server.accounts.initVHostRequestQueue(config) - } - - chanRegPreviouslyDisabled := oldConfig != nil && !oldConfig.Channels.Registration.Enabled - chanRegNowEnabled := config.Channels.Registration.Enabled - if chanRegPreviouslyDisabled && chanRegNowEnabled { - server.channels.loadRegisteredChannels() - } - - // STS - stsPreviouslyEnabled := oldConfig != nil && oldConfig.Server.STS.Enabled - stsValue := config.Server.capValues[caps.STS] - stsCurrentCapValue := "" if oldConfig != nil { - stsCurrentCapValue = oldConfig.Server.capValues[caps.STS] - } - server.logger.Debug("server", "STS Vals", stsCurrentCapValue, stsValue, fmt.Sprintf("server[%v] config[%v]", stsPreviouslyEnabled, config.Server.STS.Enabled)) - if (config.Server.STS.Enabled != stsPreviouslyEnabled) || (stsValue != stsCurrentCapValue) { - // XXX: STS is always removed by CAP NEW sts=duration=0, not CAP DEL - // so the appropriate notify is always a CAP NEW; put it in addedCaps for any change - addedCaps.Add(caps.STS) - } + // cap changes + if oldConfig.Server.capValues[caps.Languages] != config.Server.capValues[caps.Languages] { + updatedCaps.Add(caps.Languages) + } - if oldConfig != nil && config.Accounts.Bouncer.Enabled != oldConfig.Accounts.Bouncer.Enabled { - if config.Accounts.Bouncer.Enabled { + if !oldConfig.Accounts.AuthenticationEnabled && config.Accounts.AuthenticationEnabled { + addedCaps.Add(caps.SASL) + } else if oldConfig.Accounts.AuthenticationEnabled && !config.Accounts.AuthenticationEnabled { + removedCaps.Add(caps.SASL) + } + + if !oldConfig.Accounts.Bouncer.Enabled && config.Accounts.Bouncer.Enabled { addedCaps.Add(caps.Bouncer) - } else { + } else if oldConfig.Accounts.Bouncer.Enabled && !config.Accounts.Bouncer.Enabled { removedCaps.Add(caps.Bouncer) } - } - // resize history buffers as needed - if oldConfig != nil && oldConfig.History != config.History { - for _, channel := range server.channels.Channels() { - channel.history.Resize(config.History.ChannelLength, config.History.AutoresizeWindow) + if oldConfig.Server.STS.Enabled != config.Server.STS.Enabled || oldConfig.Server.capValues[caps.STS] != config.Server.capValues[caps.STS] { + // XXX: STS is always removed by CAP NEW sts=duration=0, not CAP DEL + // so the appropriate notify is always a CAP NEW; put it in addedCaps for any change + addedCaps.Add(caps.STS) } - for _, client := range server.clients.AllClients() { - client.history.Resize(config.History.ClientLength, config.History.AutoresizeWindow) + + // if certain features were enabled by rehash, we need to load the corresponding data + // from the store + if !oldConfig.Accounts.NickReservation.Enabled { + server.accounts.buildNickToAccountIndex(config) + } + if !oldConfig.Accounts.VHosts.Enabled { + server.accounts.initVHostRequestQueue(config) + } + if !oldConfig.Channels.Registration.Enabled { + server.channels.loadRegisteredChannels(config) + } + + // resize history buffers as needed + if oldConfig.History != config.History { + for _, channel := range server.channels.Channels() { + channel.history.Resize(config.History.ChannelLength, config.History.AutoresizeWindow) + } + for _, client := range server.clients.AllClients() { + client.history.Resize(config.History.ClientLength, config.History.AutoresizeWindow) + } } }