From 449ef4cea168aead570ebe38eacfda08db4e964f Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Tue, 25 Jul 2017 23:02:35 -0700 Subject: [PATCH 1/2] strings: disallow ':' in nicks This matches the behavior of inspircd at the very least. Previously, the comment above that section claimed ':' should be disallowed, but the code didn't do so. I also simplified the code a little bit and added tests. --- irc/strings.go | 9 +--- irc/strings_test.go | 105 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 irc/strings_test.go diff --git a/irc/strings.go b/irc/strings.go index af0fdecf..d8f09502 100644 --- a/irc/strings.go +++ b/irc/strings.go @@ -44,8 +44,7 @@ func CasefoldChannel(name string) (string, error) { // , is used as a separator // * is used in mask matching // ? is used in mask matching - if strings.Contains(lowered, " ") || strings.Contains(lowered, ",") || - strings.Contains(lowered, "*") || strings.Contains(lowered, "?") { + if strings.ContainsAny(lowered, " ,*?") { return "", errInvalidCharacter } @@ -73,11 +72,7 @@ func CasefoldName(name string) (string, error) { // # is a channel prefix // ~&@%+ are channel membership prefixes // - I feel like disallowing - if strings.Contains(lowered, " ") || strings.Contains(lowered, ",") || - strings.Contains(lowered, "*") || strings.Contains(lowered, "?") || - strings.Contains(lowered, ".") || strings.Contains(lowered, "!") || - strings.Contains(lowered, "@") || - strings.Contains("#~&@%+-", string(lowered[0])) { + if strings.ContainsAny(lowered, " ,*?.!@:") || strings.ContainsAny(string(lowered[0]), "#~&@%+-") { return "", errInvalidCharacter } diff --git a/irc/strings_test.go b/irc/strings_test.go new file mode 100644 index 00000000..808f6ca3 --- /dev/null +++ b/irc/strings_test.go @@ -0,0 +1,105 @@ +// Copyright (c) 2017 Euan Kemp +// released under the MIT license + +package irc + +import ( + "fmt" + "testing" +) + +func TestCasefoldChannel(t *testing.T) { + type channelTest struct { + channel string + folded string + err bool + } + testCases := []channelTest{ + { + channel: "#foo", + folded: "#foo", + }, + { + channel: "#rfc1459[noncompliant]", + folded: "#rfc1459[noncompliant]", + }, + { + channel: "#{[]}", + folded: "#{[]}", + }, + { + channel: "#FOO", + folded: "#foo", + }, + { + channel: "#bang!", + folded: "#bang!", + }, + { + channel: "#", + folded: "#", + }, + } + + for _, errCase := range []string{ + "", "#*starpower", "# NASA", "#interro?", "OOF#", "foo", + } { + testCases = append(testCases, channelTest{channel: errCase, err: true}) + } + + for i, tt := range testCases { + t.Run(fmt.Sprintf("case %d: %s", i, tt.channel), func(t *testing.T) { + res, err := CasefoldChannel(tt.channel) + if tt.err { + if err == nil { + t.Errorf("expected error") + } + return + } + if tt.folded != res { + t.Errorf("expected %v to be %v", tt.folded, res) + } + }) + } +} + +func TestCasefoldName(t *testing.T) { + type nameTest struct { + name string + folded string + err bool + } + testCases := []nameTest{ + { + name: "foo", + folded: "foo", + }, + { + name: "FOO", + folded: "foo", + }, + } + + for _, errCase := range []string{ + "", "#", "foo,bar", "star*man*junior", "lo7t?", + "f.l", "excited!nick", "foo@bar", ":trail", + "~o", "&o", "@o", "%h", "+v", "-m", + } { + testCases = append(testCases, nameTest{name: errCase, err: true}) + } + + for i, tt := range testCases { + t.Run(fmt.Sprintf("case %d: %s", i, tt.name), func(t *testing.T) { + res, err := CasefoldName(tt.name) + if tt.err { + if err == nil { + t.Errorf("expected error") + } + return + } + if tt.folded != res { + t.Errorf("expected %v to be %v", tt.folded, res) + } + }) + } +} From 3b47f3d4705825f45c67c07baa1a8982fb1f4491 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Tue, 25 Jul 2017 23:27:11 -0700 Subject: [PATCH 2/2] config: don't casefold tls names I don't think casefolding things like `:6697` ever made sense. Since these are configured by the ircd operator, it makes sense to assume they'll already be in a canonical form regardless. --- irc/config.go | 7 +------ web/config.go | 9 +-------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/irc/config.go b/irc/config.go index 8a5dae7c..ac840f7e 100644 --- a/irc/config.go +++ b/irc/config.go @@ -369,12 +369,7 @@ func (conf *Config) TLSListeners() map[string]*tls.Config { if err != nil { log.Fatal(err) } - name, err := CasefoldName(s) - if err == nil { - tlsListeners[name] = config - } else { - log.Println("Could not casefold TLS listener:", err.Error()) - } + tlsListeners[s] = config } return tlsListeners } diff --git a/web/config.go b/web/config.go index f8dbad3f..fa68e03e 100644 --- a/web/config.go +++ b/web/config.go @@ -11,8 +11,6 @@ import ( "io/ioutil" "log" - "github.com/oragono/oragono/irc" - "gopkg.in/yaml.v2" ) @@ -48,12 +46,7 @@ func (conf *Config) TLSListeners() map[string]*tls.Config { if err != nil { log.Fatal(err) } - name, err := irc.CasefoldName(s) - if err == nil { - tlsListeners[name] = config - } else { - log.Println("Could not casefold TLS listener:", err.Error()) - } + tlsListeners[name] = config } return tlsListeners }