Repository: ergochat/ergo (go)

Issue:
spurious success message when deleting ephemeral history message
Reported by mv33:

```
histserv delete ei36qp4vm49qfpbbifq2quwjdw
@time=2021-12-28T04:10:12.523Z :HistServ!HistServ@localhost NOTICE asdf :Successfully deleted message
history #asdf  
@time=2021-12-28T04:10:24.082Z :ergo.test BATCH +2 chathistory #asdf
@msgid=_s3xeh5j4gan3esbjkyuqz472ga;time=2021-12-28T04:09:11.075Z;batch=2 :HistServ!HistServ@localhost PRIVMSG #asdf :asdf joined the channel
@time=2021-12-28T04:09:23.675Z;batch=2;msgid=je88i9m5t8mknevupudcvpwkzn :asdf!~u@m5gy8jpgzbefg.irc PRIVMSG #asdf :hi1
@msgid=ei36qp4vm49qfpbbifq2quwjdw;time=2021-12-28T04:09:26.731Z;batch=2 :asdf!~u@m5gy8jpgzbefg.irc PRIVMSG #asdf :hi2
@batch=2;msgid=a2rnmfpckm4d4sh2z3fje89mew;time=2021-12-28T04:09:29.451Z :asdf!~u@m5gy8jpgzbefg.irc PRIVMSG #asdf :hi3
@time=2021-12-28T04:10:24.082Z :ergo.test BATCH -2
histserv delete #asdf ei36qp4vm49qfpbbifq2quwjdw
@time=2021-12-28T04:10:28.786Z :HistServ!HistServ@localhost NOTICE asdf :Successfully deleted message
history #asdf
@time=2021-12-28T04:10:32.626Z :ergo.test BATCH +3 chathistory #asdf
@msgid=_s3xeh5j4gan3esbjkyuqz472ga;time=2021-12-28T04:09:11.075Z;batch=3 :HistServ!HistServ@localhost PRIVMSG #asdf :asdf joined the channel
@batch=3;msgid=je88i9m5t8mknevupudcvpwkzn;time=2021-12-28T04:09:23.675Z :asdf!~u@m5gy8jpgzbefg.irc PRIVMSG #asdf :hi1
@msgid=a2rnmfpckm4d4sh2z3fje89mew;time=2021-12-28T04:09:29.451Z;batch=3 :asdf!~u@m5gy8jpgzbefg.irc PRIVMSG #asdf :hi3
@time=2021-12-28T04:10:32.626Z :ergo.test BATCH -3
```

Deleting an ephemeral history message requires the channel name as an argument, but sends a spurious success message if that isn't provided. This should possibly be made consistent by always having it take two arguments (the history backend can ignore the first if it's not necessary).

Pull request:
histserv delete now requires two params
Fixes #1881 . As per discussion with @slingamn on irc, this is fixed by requiring two parameters for all cases, instead of allowing one parameter for cases when persistent database storage is used. Also deleted the extra logic for number of params at the beginning of histservDeleteHandler which is now not necessary.

Also modified the bits of help to represent the target param as necessary ([] -> <>). Let me know if I missed anything else, thank you.

Patch:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index d6675222..6200972d 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -19,7 +19,7 @@ jobs:
       - name: "setup go"
         uses: "actions/setup-go@v2"
         with:
-          go-version: "1.18"
+          go-version: "1.17"
       - name: "install python3-pytest"
         run: "sudo apt install -y python3-pytest"
       - name: "make install"
diff --git a/Dockerfile b/Dockerfile
index 0e41b0ce..dd1afd19 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,5 +1,5 @@
 ## build ergo binary
-FROM golang:1.18-alpine AS build-env
+FROM golang:1.17-alpine3.13 AS build-env
 
 RUN apk add -U --force-refresh --no-cache --purge --clean-protected -l -u make git
 
diff --git a/docs/MANUAL.md b/docs/MANUAL.md
index fd5f9624..f7d6b7bd 100644
--- a/docs/MANUAL.md
+++ b/docs/MANUAL.md
@@ -506,8 +506,6 @@ If you try to oper unsuccessfully, Ergo will disconnect you from the network. If
 
 The config file accepts hashed passwords, not plaintext passwords. You must run `ergo genpasswd`, type your actual password in, and then receive a hashed blob back (it will look like `$2a$04$GvCFlShLZQjId3dARzwOWu9Nvq6lndXINw2Sdm6mUcwxhtx1U/hIm`). Enter that into the relevant `opers` block in your config file, then save the file.
 
-Although it's theoretically possible to use an operator password that contains spaces, your client may not support it correctly, so it's advisable to choose a password without spaces. (The period character `.` is an acceptable alternative separator if your password is based on randomly chosen words.)
-
 After that, you must rehash or restart Ergo to apply the config change. If a rehash didn't accomplish the desired effects, you might want to try a restart instead.
 
 
diff --git a/go.mod b/go.mod
index a91e2a4b..1c85648b 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module github.com/ergochat/ergo
 
-go 1.18
+go 1.17
 
 require (
 	code.cloudfoundry.org/bytefmt v0.0.0-20200131002437-cf55d5288a48
diff --git a/irc/channel.go b/irc/channel.go
index f44de1e7..eaddd55b 100644
--- a/irc/channel.go
+++ b/irc/channel.go
@@ -177,7 +177,10 @@ func (channel *Channel) ExportRegistration(includeFlags uint) (info RegisteredCh
 		info.Bans = channel.lists[modes.BanMask].Masks()
 		info.Invites = channel.lists[modes.InviteMask].Masks()
 		info.Excepts = channel.lists[modes.ExceptMask].Masks()
-		info.AccountToUMode = utils.CopyMap(channel.accountToUMode)
+		info.AccountToUMode = make(map[string]modes.Mode)
+		for account, mode := range channel.accountToUMode {
+			info.AccountToUMode[account] = mode
+		}
 	}
 
 	if includeFlags&IncludeSettings != 0 {
diff --git a/irc/channelmanager.go b/irc/channelmanager.go
index a1921c4b..5d4f2cd7 100644
--- a/irc/channelmanager.go
+++ b/irc/channelmanager.go
@@ -26,17 +26,17 @@ type ChannelManager struct {
 	sync.RWMutex // tier 2
 	// chans is the main data structure, mapping casefolded name -> *Channel
 	chans               map[string]*channelManagerEntry
-	chansSkeletons      utils.HashSet[string] // skeletons of *unregistered* chans
-	registeredChannels  utils.HashSet[string] // casefolds of registered chans
-	registeredSkeletons utils.HashSet[string] // skeletons of registered chans
-	purgedChannels      utils.HashSet[string] // 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(utils.HashSet[string])
+	cm.chansSkeletons = make(utils.StringSet)
 	cm.server = server
 
 	// purging should work even if registration is disabled
@@ -66,8 +66,8 @@ func (cm *ChannelManager) loadRegisteredChannels(config *Config) {
 	cm.Lock()
 	defer cm.Unlock()
 
-	cm.registeredChannels = make(utils.HashSet[string], len(rawNames))
-	cm.registeredSkeletons = make(utils.HashSet[string], len(rawNames))
+	cm.registeredChannels = make(utils.StringSet, len(rawNames))
+	cm.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 bb0a851b..1a3a366e 100644
--- a/irc/channelreg.go
+++ b/irc/channelreg.go
@@ -145,8 +145,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 utils.HashSet[string]) {
-	result = make(utils.HashSet[string])
+func (reg *ChannelRegistry) PurgedChannels() (result utils.StringSet) {
+	result = make(utils.StringSet)
 
 	prefix := fmt.Sprintf(keyChannelPurged, "")
 	reg.server.store.View(func(tx *buntdb.Tx) error {
diff --git a/irc/client.go b/irc/client.go
index 2e7fbc93..6912483c 100644
--- a/irc/client.go
+++ b/irc/client.go
@@ -994,8 +994,8 @@ func (client *Client) ModeString() (str string) {
 }
 
 // Friends refers to clients that share a channel with this client.
-func (client *Client) Friends(capabs ...caps.Capability) (result utils.HashSet[*Session]) {
-	result = make(utils.HashSet[*Session])
+func (client *Client) Friends(capabs ...caps.Capability) (result map[*Session]empty) {
+	result = make(map[*Session]empty)
 
 	// look at the client's own sessions
 	addFriendsToSet(result, client, capabs...)
@@ -1010,19 +1010,19 @@ func (client *Client) Friends(capabs ...caps.Capability) (result utils.HashSet[*
 }
 
 // Friends refers to clients that share a channel or extended-monitor this client.
-func (client *Client) FriendsMonitors(capabs ...caps.Capability) (result utils.HashSet[*Session]) {
+func (client *Client) FriendsMonitors(capabs ...caps.Capability) (result map[*Session]empty) {
 	result = client.Friends(capabs...)
 	client.server.monitorManager.AddMonitors(result, client.nickCasefolded, capabs...)
 	return
 }
 
 // helper for Friends
-func addFriendsToSet(set utils.HashSet[*Session], client *Client, capabs ...caps.Capability) {
+func addFriendsToSet(set map[*Session]empty, client *Client, capabs ...caps.Capability) {
 	client.stateMutex.RLock()
 	defer client.stateMutex.RUnlock()
 	for _, session := range client.sessions {
 		if session.capabilities.HasAll(capabs...) {
-			set.Add(session)
+			set[session] = empty{}
 		}
 	}
 }
@@ -1575,7 +1575,7 @@ func (client *Client) addChannel(channel *Channel, simulated bool) (err error) {
 	} else if client.oper == nil && len(client.channels) >= config.Channels.MaxChannelsPerClient {
 		err = errTooManyChannels
 	} else {
-		client.channels.Add(channel) // success
+		client.channels[channel] = empty{} // success
 	}
 	client.stateMutex.Unlock()
 
diff --git a/irc/config.go b/irc/config.go
index 75fd1f27..3c26577a 100644
--- a/irc/config.go
+++ b/irc/config.go
@@ -704,8 +704,8 @@ type Config struct {
 // OperClass defines an assembled operator class.
 type OperClass struct {
 	Title        string
-	WhoisLine    string                `yaml:"whois-line"`
-	Capabilities utils.HashSet[string] // 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.
@@ -743,7 +743,7 @@ func (conf *Config) OperatorClasses() (map[string]*OperClass, error) {
 
 			// create new operclass
 			var oc OperClass
-			oc.Capabilities = make(utils.HashSet[string])
+			oc.Capabilities = make(utils.StringSet)
 
 			// get inhereted info from other operclasses
 			if len(info.Extends) > 0 {
diff --git a/irc/handlers.go b/irc/handlers.go
index e777acce..65e459d5 100644
--- a/irc/handlers.go
+++ b/irc/handlers.go
@@ -3424,7 +3424,7 @@ func whoHandler(server *Server, client *Client, msg ircmsg.Message, rb *Response
 		// Construct set of channels the client is in.
 		userChannels := make(ChannelSet)
 		for _, channel := range client.Channels() {
-			userChannels.Add(channel)
+			userChannels[channel] = empty{}
 		}
 
 		// Another client is a friend if they share at least one channel, or they are the same client.
@@ -3437,7 +3437,7 @@ func whoHandler(server *Server, client *Client, msg ircmsg.Message, rb *Response
 				if channel.flags.HasMode(modes.Auditorium) {
 					return false // TODO this should respect +v etc.
 				}
-				if userChannels.Has(channel) {
+				if _, present := userChannels[channel]; present {
 					return true
 				}
 			}
diff --git a/irc/history/history.go b/irc/history/history.go
index 28b2d891..db3c0d22 100644
--- a/irc/history/history.go
+++ b/irc/history/history.go
@@ -203,7 +203,7 @@ func (list *Buffer) betweenHelper(start, end Selector, cutoff time.Time, pred Pr
 
 // returns all correspondents, in reverse time order
 func (list *Buffer) allCorrespondents() (results []TargetListing) {
-	seen := make(utils.HashSet[string])
+	seen := make(utils.StringSet)
 
 	list.RLock()
 	defer list.RUnlock()
diff --git a/irc/histserv.go b/irc/histserv.go
index 59149fa4..3416cd6d 100644
--- a/irc/histserv.go
+++ b/irc/histserv.go
@@ -42,14 +42,13 @@ FORGET deletes all history messages sent by an account.`,
 		},
 		"delete": {
 			handler: histservDeleteHandler,
-			help: `Syntax: $bDELETE [target] <msgid>$b
+			help: `Syntax: $bDELETE <target> <msgid>$b
 
-DELETE deletes an individual message by its msgid. The target is a channel
-name or nickname; depending on the history implementation, this may or may not
-be necessary to locate the message.`,
-			helpShort: `$bDELETE$b deletes an individual message by its msgid.`,
+DELETE deletes an individual message by its msgid. The target is the channel
+name. The msgid is the ID as can be found in the tags of that message.`,
+			helpShort: `$bDELETE$b deletes an individual message by its target and msgid.`,
 			enabled:   histservEnabled,
-			minParams: 1,
+			minParams: 2,
 			maxParams: 2,
 		},
 		"export": {
@@ -94,12 +93,7 @@ func histservForgetHandler(service *ircService, server *Server, client *Client,
 }
 
 func histservDeleteHandler(service *ircService, server *Server, client *Client, command string, params []string, rb *ResponseBuffer) {
-	var target, msgid string
-	if len(params) == 1 {
-		msgid = params[0]
-	} else {
-		target, msgid = params[0], params[1]
-	}
+	target, msgid := params[0], params[1] // Fix #1881 2 params are required
 
 	// operators can delete; if individual delete is allowed, a chanop or
 	// the message author can delete
diff --git a/irc/import.go b/irc/import.go
index 2ebf88dd..27212e96 100644
--- a/irc/import.go
+++ b/irc/import.go
@@ -54,7 +54,7 @@ type databaseImport struct {
 	Channels map[string]channelImport
 }
 
-func serializeAmodes(raw map[string]string, validCfUsernames utils.HashSet[string]) (result []byte, err error) {
+func serializeAmodes(raw map[string]string, validCfUsernames utils.StringSet) (result []byte, err error) {
 	processed := make(map[string]int, len(raw))
 	for accountName, mode := range raw {
 		if len(mode) != 1 {
@@ -80,7 +80,7 @@ func doImportDBGeneric(config *Config, dbImport databaseImport, credsType Creden
 	tx.Set(keySchemaVersion, strconv.Itoa(importDBSchemaVersion), nil)
 	tx.Set(keyCloakSecret, utils.GenerateSecretKey(), nil)
 
-	cfUsernames := make(utils.HashSet[string])
+	cfUsernames := make(utils.StringSet)
 	skeletonToUsername := make(map[string]string)
 	warnSkeletons := false
 
diff --git a/irc/monitor.go b/irc/monitor.go
index 9131e81b..65abe9ad 100644
--- a/irc/monitor.go
+++ b/irc/monitor.go
@@ -7,7 +7,6 @@ import (
 	"sync"
 
 	"github.com/ergochat/ergo/irc/caps"
-	"github.com/ergochat/ergo/irc/utils"
 
 	"github.com/ergochat/irc-go/ircmsg"
 )
@@ -18,21 +17,21 @@ type MonitorManager struct {
 	// client -> (casefolded nick it's watching -> uncasefolded nick)
 	watching map[*Session]map[string]string
 	// casefolded nick -> clients watching it
-	watchedby map[string]utils.HashSet[*Session]
+	watchedby map[string]map[*Session]empty
 }
 
 func (mm *MonitorManager) Initialize() {
 	mm.watching = make(map[*Session]map[string]string)
-	mm.watchedby = make(map[string]utils.HashSet[*Session])
+	mm.watchedby = make(map[string]map[*Session]empty)
 }
 
 // AddMonitors adds clients using extended-monitor monitoring `client`'s nick to the passed user set.
-func (manager *MonitorManager) AddMonitors(users utils.HashSet[*Session], cfnick string, capabs ...caps.Capability) {
+func (manager *MonitorManager) AddMonitors(users map[*Session]empty, cfnick string, capabs ...caps.Capability) {
 	manager.RLock()
 	defer manager.RUnlock()
 	for session := range manager.watchedby[cfnick] {
 		if session.capabilities.Has(caps.ExtendedMonitor) && session.capabilities.HasAll(capabs...) {
-			users.Add(session)
+			users[session] = empty{}
 		}
 	}
 }
@@ -71,7 +70,7 @@ func (manager *MonitorManager) Add(session *Session, nick string, limit int) err
 		manager.watching[session] = make(map[string]string)
 	}
 	if manager.watchedby[cfnick] == nil {
-		manager.watchedby[cfnick] = make(utils.HashSet[*Session])
+		manager.watchedby[cfnick] = make(map[*Session]empty)
 	}
 
 	if len(manager.watching[session]) >= limit {
@@ -79,7 +78,7 @@ func (manager *MonitorManager) Add(session *Session, nick string, limit int) err
 	}
 
 	manager.watching[session][cfnick] = nick
-	manager.watchedby[cfnick].Add(session)
+	manager.watchedby[cfnick][session] = empty{}
 	return nil
 }
 
@@ -93,7 +92,7 @@ func (manager *MonitorManager) Remove(session *Session, nick string) (err error)
 	manager.Lock()
 	defer manager.Unlock()
 	delete(manager.watching[session], cfnick)
-	manager.watchedby[cfnick].Remove(session)
+	delete(manager.watchedby[cfnick], session)
 	return nil
 }
 
@@ -103,7 +102,7 @@ func (manager *MonitorManager) RemoveAll(session *Session) {
 	defer manager.Unlock()
 
 	for cfnick := range manager.watching[session] {
-		manager.watchedby[cfnick].Remove(session)
+		delete(manager.watchedby[cfnick], session)
 	}
 	delete(manager.watching, session)
 }
diff --git a/irc/nickname.go b/irc/nickname.go
index 3fb36609..6e31cf4a 100644
--- a/irc/nickname.go
+++ b/irc/nickname.go
@@ -24,8 +24,8 @@ var (
 		"MemoServ", "BotServ", "OperServ",
 	}
 
-	restrictedCasefoldedNicks = make(utils.HashSet[string])
-	restrictedSkeletons       = make(utils.HashSet[string])
+	restrictedCasefoldedNicks = make(utils.StringSet)
+	restrictedSkeletons       = make(utils.StringSet)
 )
 
 func performNickChange(server *Server, client *Client, target *Client, session *Session, nickname string, rb *ResponseBuffer) error {
diff --git a/irc/types.go b/irc/types.go
index 45da2536..9166befc 100644
--- a/irc/types.go
+++ b/irc/types.go
@@ -9,11 +9,28 @@ import (
 	"time"
 
 	"github.com/ergochat/ergo/irc/modes"
-	"github.com/ergochat/ergo/irc/utils"
 )
 
+type empty struct{}
+
 // ClientSet is a set of clients.
-type ClientSet = utils.HashSet[*Client]
+type ClientSet map[*Client]empty
+
+// Add adds the given client to this set.
+func (clients ClientSet) Add(client *Client) {
+	clients[client] = empty{}
+}
+
+// Remove removes the given client from this set.
+func (clients ClientSet) Remove(client *Client) {
+	delete(clients, client)
+}
+
+// Has returns true if the given client is in this set.
+func (clients ClientSet) Has(client *Client) bool {
+	_, ok := clients[client]
+	return ok
+}
 
 type memberData struct {
 	modes    *modes.ModeSet
@@ -43,4 +60,4 @@ func (members MemberSet) Has(member *Client) bool {
 }
 
 // ChannelSet is a set of channels.
-type ChannelSet = utils.HashSet[*Channel]
+type ChannelSet map[*Channel]empty
diff --git a/irc/utils/types.go b/irc/utils/types.go
index 06b56649..249b486e 100644
--- a/irc/utils/types.go
+++ b/irc/utils/types.go
@@ -5,25 +5,13 @@ package utils
 
 type empty struct{}
 
-type HashSet[T comparable] map[T]empty
+type StringSet map[string]empty
 
-func (s HashSet[T]) Has(elem T) bool {
-	_, ok := s[elem]
+func (s StringSet) Has(str string) bool {
+	_, ok := s[str]
 	return ok
 }
 
-func (s HashSet[T]) Add(elem T) {
-	s[elem] = empty{}
-}
-
-func (s HashSet[T]) Remove(elem T) {
-	delete(s, elem)
-}
-
-func CopyMap[K comparable, V any](input map[K]V) (result map[K]V) {
-	result = make(map[K]V, len(input))
-	for key, value := range input {
-		result[key] = value
-	}
-	return
+func (s StringSet) Add(str string) {
+	s[str] = empty{}
 }
diff --git a/irc/znc.go b/irc/znc.go
index a118e839..dcc29155 100644
--- a/irc/znc.go
+++ b/irc/znc.go
@@ -74,7 +74,7 @@ func timeToZncWireTime(t time.Time) (result string) {
 type zncPlaybackTimes struct {
 	start   time.Time
 	end     time.Time
-	targets utils.HashSet[string] // nil for "*" (everything), otherwise the channel names
+	targets utils.StringSet // nil for "*" (everything), otherwise the channel names
 	setAt   time.Time
 }
 
@@ -134,7 +134,7 @@ func zncPlaybackPlayHandler(client *Client, command string, params []string, rb
 		end = zncWireTimeToTime(params[3])
 	}
 
-	var targets utils.HashSet[string]
+	var targets utils.StringSet
 	var nickTargets []string
 
 	// three cases:
@@ -157,7 +157,7 @@ func zncPlaybackPlayHandler(client *Client, command string, params []string, rb
 	if params[1] == "*" {
 		playPrivmsgs = true // XXX nil `targets` means "every channel"
 	} else {
-		targets = make(utils.HashSet[string])
+		targets = make(utils.StringSet)
 		for _, targetName := range strings.Split(targetString, ",") {
 			if strings.HasPrefix(targetName, "#") {
 				if cfTarget, err := CasefoldChannel(targetName); err == nil {


Test patch:
diff --git a/irc/client_test.go b/irc/client_test.go
index 64d1036c..7aeee37b 100644
--- a/irc/client_test.go
+++ b/irc/client_test.go
@@ -11,7 +11,7 @@ import (
 
 func TestGenerateBatchID(t *testing.T) {
 	var session Session
-	s := make(utils.HashSet[string])
+	s := make(utils.StringSet)
 
 	count := 100000
 	for i := 0; i < count; i++ {
diff --git a/irctest b/irctest
index 8356ace0..da005d7d 160000
--- a/irctest
+++ b/irctest
@@ -1,1 +1,1 @@
-Subproject commit 8356ace0144297885f6d51e334763df5223b933f
+Subproject commit da005d7d2492bf31c4bdeb46108240766c69d0ad