review fixes

docs/MANUAL.md

@@ -348,7 +348,7 @@ Otherwise, in the Oragono config file, you'll want to enable raw line logging by
 
 # IRC over TLS
 
-Traditionally, IRC used a plaintext protocol, typically on port 6667. Over time, a convention emerged to use this protocol inside SSL/TLS instead, typically on port 6697. As of now, we recommend that you make your server available *exclusively* via TLS, since allowing plaintext access can result in the disclosure of user data or passwords. While the default config file still exposes a public plaintext port for the benefit of legacy clients, it also contains instructions on how to disable it --- if at all possible, you should follow them!
+IRC has traditionally been available over both plaintext (on port 6667) and SSL/TLS (on port 6697). We recommend that you make your server available exclusively via TLS, since exposing plaintext access allows for unauthorized interception or modification of user data or passwords. While the default config file exposes a plaintext public port, it also contains instructions on how to disable it or replace it with a 'dummy' plaintext listener that simply directs users to reconnect using TLS.
 
 
 ## How do I use Let's Encrypt certificates?

irc/caps/set.go

@@ -93,8 +93,8 @@ func (s *Set) Empty() bool {
 
 const maxPayloadLength = 440
 
-// String returns all of our enabled capabilities as a string.
-func (s *Set) String(version Version, values Values) (result []string) {
+// Strings returns all of our enabled capabilities as a slice of strings.
+func (s *Set) Strings(version Version, values Values) (result []string) {
 	var strs sort.StringSlice
 
 	var capab Capability
@@ -105,7 +105,7 @@ func (s *Set) String(version Version, values Values) (result []string) {
 			continue
 		}
 		capString := capab.Name()
-		if version == Cap302 {
+		if version >= Cap302 {
 			val, exists := values[capab]
 			if exists {
 				capString += "=" + val

irc/caps/set_test.go

@@ -43,17 +43,17 @@ func TestSets(t *testing.T) {
 		t.Error("Add/Remove don't work")
 	}
 
-	// test String()
+	// test Strings()
 	values := make(Values)
 	values[InviteNotify] = "invitemepls"
 
-	actualCap301ValuesString := s1.String(Cap301, values)
+	actualCap301ValuesString := s1.Strings(Cap301, values)
 	expectedCap301ValuesString := []string{"invite-notify userhost-in-names"}
 	if !reflect.DeepEqual(actualCap301ValuesString, expectedCap301ValuesString) {
 		t.Errorf("Generated Cap301 values string [%v] did not match expected values string [%v]", actualCap301ValuesString, expectedCap301ValuesString)
 	}
 
-	actualCap302ValuesString := s1.String(Cap302, values)
+	actualCap302ValuesString := s1.Strings(Cap302, values)
 	expectedCap302ValuesString := []string{"invite-notify=invitemepls userhost-in-names"}
 	if !reflect.DeepEqual(actualCap302ValuesString, expectedCap302ValuesString) {
 		t.Errorf("Generated Cap302 values string [%s] did not match expected values string [%s]", actualCap302ValuesString, expectedCap302ValuesString)

irc/handlers.go

@@ -569,11 +569,11 @@ func capHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo
 
 	sendCapLines := func(cset *caps.Set, values caps.Values) {
 		version := rb.session.capVersion
-		capLines := cset.String(version, values)
+		capLines := cset.Strings(version, values)
 		// weechat 1.4 has a bug here where it won't accept the CAP reply unless it contains
 		// the server.name source:
 		for i, capStr := range capLines {
-			if version == caps.Cap302 && i < len(capLines)-1 {
+			if version >= caps.Cap302 && i < len(capLines)-1 {
 				rb.Add(nil, server.name, "CAP", details.nick, subCommand, "*", capStr)
 			} else {
 				rb.Add(nil, server.name, "CAP", details.nick, subCommand, capStr)

irc/server.go

@@ -673,6 +673,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) {
 	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)
 	}
 
@@ -704,17 +705,17 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) {
 
 	// updated caps get DEL'd and then NEW'd
 	// so, we can just add updated ones to both removed and added lists here and they'll be correctly handled
-	server.logger.Debug("server", "Updated Caps", strings.Join(updatedCaps.String(caps.Cap301, config.Server.capValues), " "))
+	server.logger.Debug("server", "Updated Caps", strings.Join(updatedCaps.Strings(caps.Cap301, config.Server.capValues), " "))
 	addedCaps.Union(updatedCaps)
 	removedCaps.Union(updatedCaps)
 
 	if !addedCaps.Empty() || !removedCaps.Empty() {
 		capBurstSessions = server.clients.AllWithCapsNotify()
 
-		added[caps.Cap301] = addedCaps.String(caps.Cap301, config.Server.capValues)
-		added[caps.Cap302] = addedCaps.String(caps.Cap302, config.Server.capValues)
+		added[caps.Cap301] = addedCaps.Strings(caps.Cap301, config.Server.capValues)
+		added[caps.Cap302] = addedCaps.Strings(caps.Cap302, config.Server.capValues)
 		// removed never has values, so we leave it as Cap301
-		removed = removedCaps.String(caps.Cap301, config.Server.capValues)
+		removed = removedCaps.Strings(caps.Cap301, config.Server.capValues)
 	}
 
 	for _, sSession := range capBurstSessions {