toc: code review fixes

server/toc/cmd_client.go

@@ -137,7 +137,7 @@ func (s OSCARProxy) RecvClientCmd(
 	}
 
 	if s.Logger.Enabled(ctx, slog.LevelDebug) {
-		s.Logger.InfoContext(ctx, "client request", "command", payload)
+		s.Logger.DebugContext(ctx, "client request", "command", payload)
 	} else {
 		s.Logger.InfoContext(ctx, "client request", "command", cmd)
 	}
@@ -265,7 +265,7 @@ func (s OSCARProxy) AddPermit(ctx context.Context, me *state.Session, cmd []byte
 	return ""
 }
 
-// AddDeny handles the toc_chat_join TOC command.
+// AddDeny handles the toc_add_deny TOC command.
 //
 // From the TiK documentation:
 //
@@ -721,12 +721,13 @@ func (s OSCARProxy) GetDirSearchURL(ctx context.Context, me *state.Session, cmd
 
 	// map labels to param values at their corresponding positions
 	p := url.Values{}
-	i := 0
-	for i < len(params) && i < len(labels) {
-		if len(params[i]) > 0 {
-			p.Add(labels[i], strings.Trim(params[i], "\""))
+	for i, param := range params {
+		if i >= len(labels) {
+			break
+		}
+		if param != "" {
+			p.Add(labels[i], strings.Trim(param, "\""))
 		}
-		i++
 	}
 
 	if len(p) == 0 {
@@ -1338,34 +1339,29 @@ func parseArgs(payload []byte, cmd string, args ...*string) (varArgs []string, e
 
 	segs, err := reader.Read()
 	if err != nil {
-		return nil, fmt.Errorf("CSV reader error: %w", err)
+		return []string{}, fmt.Errorf("CSV reader error: %w", err)
 	}
 
 	// sanity check the command name
 	if segs[0] != cmd {
-		return nil, fmt.Errorf("command mismatch. expected %s, got %s", cmd, segs[0])
+		return []string{}, fmt.Errorf("command mismatch. expected %s, got %s", cmd, segs[0])
 	}
 
 	// all elements after the command are arguments
 	segs = segs[1:]
 	if len(segs) < len(args) {
-		return nil, fmt.Errorf("command contains fewer arguments than expected")
+		return []string{}, fmt.Errorf("command contains fewer arguments than expected")
 	}
 
-	i := 0
 	// populate placeholder pointers with their corresponding values
-	for ; i < len(args); i++ {
-		if args[i] == nil {
-			// ignore argument, don't populate its corresponding pointer
-			continue
+	for i, arg := range args {
+		if arg != nil {
+			*arg = strings.TrimSpace(segs[i])
 		}
-		*args[i] = strings.TrimSpace(segs[i])
 	}
 
 	// dump remaining arguments as varargs
-	varArgs = append(varArgs, segs[i:]...)
-
-	return varArgs, err
+	return segs[len(args):], err
 }
 
 // runtimeErr is a convenience function that logs an error and returns a TOC

server/toc/cmd_client_test.go

@@ -3336,7 +3336,7 @@ func Test_parseArgs(t *testing.T) {
 			givenPayload: `toc_chat_invite`,
 			givenCmd:     "toc_chat_invite",
 			givenArgs:    nil,
-			wantVarArgs:  nil,
+			wantVarArgs:  []string{},
 		},
 		{
 			name:         "positional args with varargs",
@@ -3359,7 +3359,7 @@ func Test_parseArgs(t *testing.T) {
 			givenPayload: `toc_chat_invite 1234 "Join me!"`,
 			givenCmd:     "toc_chat_invite",
 			givenArgs:    []*string{new(string), new(string)}, // roomID + msg
-			wantVarArgs:  nil,
+			wantVarArgs:  []string{},
 			wantArgs:     []string{"1234", "Join me!"},
 		},
 		{
@@ -3374,7 +3374,7 @@ func Test_parseArgs(t *testing.T) {
 			givenPayload: `toc_chat_invite user1 user2 user3`,
 			givenCmd:     "toc_chat_accept",
 			givenArgs:    nil,
-			wantVarArgs:  nil,
+			wantVarArgs:  []string{},
 			wantErrMsg:   "mismatch",
 		},
 		{
@@ -3382,7 +3382,7 @@ func Test_parseArgs(t *testing.T) {
 			givenPayload: `toc_chat_invite`,
 			givenCmd:     "toc_chat_invite",
 			givenArgs:    []*string{new(string), new(string)},
-			wantVarArgs:  nil,
+			wantVarArgs:  []string{},
 			wantErrMsg:   "command contains fewer arguments than expected",
 		},
 		{
@@ -3390,7 +3390,7 @@ func Test_parseArgs(t *testing.T) {
 			givenPayload: ``,
 			givenCmd:     "toc_chat_invite",
 			givenArgs:    []*string{nil},
-			wantVarArgs:  nil,
+			wantVarArgs:  []string{},
 			wantErrMsg:   "CSV reader error",
 		},
 	}

server/toc/cmd_server.go

@@ -180,7 +180,7 @@ func (s OSCARProxy) IMIn(ctx context.Context, chatRegistry *ChatRegistry, snac w
 		}
 
 		svcData, ok := frag.Bytes(wire.ICBMRdvTLVTagsSvcData)
-		if !ok {
+		if !ok || svcData == nil {
 			return s.runtimeErr(ctx, errors.New("frag.Bytes: missing room info"))
 		}
 

server/toc/http.go

@@ -48,6 +48,24 @@ const directoryTpl = `
 {{- end -}}
 </BODY></HTML>`
 
+var (
+	profileTemplate   *template.Template
+	directoryTemplate *template.Template
+)
+
+func init() {
+	var err error
+	profileTemplate, err = template.New("profile").Parse(profileTpl)
+	if err != nil {
+		panic(fmt.Errorf("failed to compile profile template: %w", err))
+	}
+
+	directoryTemplate, err = template.New("directory").Parse(directoryTpl)
+	if err != nil {
+		panic(fmt.Errorf("failed to compile directory template: %w", err))
+	}
+}
+
 // NewServeMux creates and returns an HTTP mux that serves all TOC routes.
 func (s OSCARProxy) NewServeMux() http.Handler {
 	mux := http.NewServeMux()
@@ -143,12 +161,6 @@ func (s OSCARProxy) ProfileHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		t, err := template.New("results").Parse(profileTpl)
-		if err != nil {
-			s.logAndReturn500(ctx, w, fmt.Errorf("template.New: %w", err))
-			return
-		}
-
 		pd := struct {
 			ScreenName string
 			Profile    string
@@ -157,7 +169,7 @@ func (s OSCARProxy) ProfileHandler(w http.ResponseWriter, r *http.Request) {
 			Profile:    extractBodyContent(profile),
 		}
 
-		if err := t.Execute(w, pd); err != nil {
+		if err := profileTemplate.Execute(w, pd); err != nil {
 			s.logAndReturn500(ctx, w, fmt.Errorf("t.Execute: %w", err))
 		}
 	default:
@@ -310,13 +322,7 @@ func (s OSCARProxy) outputSearchResults(ctx context.Context, w http.ResponseWrit
 		results = append(results, rec)
 	}
 
-	t, err := template.New("results").Parse(directoryTpl)
-	if err != nil {
-		s.logAndReturn500(ctx, w, fmt.Errorf("template.New: %w", err))
-		return
-	}
-
-	if err := t.Execute(w, PageData{Results: results}); err != nil {
+	if err := directoryTemplate.Execute(w, PageData{Results: results}); err != nil {
 		s.logAndReturn500(ctx, w, fmt.Errorf("t.Execute: %w", err))
 	}
 }

server/toc/server.go

@@ -127,7 +127,7 @@ func (rt Server) Start(ctx context.Context) error {
 		go func() {
 			defer wg.Done()
 			bufCon := newBufferedConn(conn)
-			b, err := bufCon.Peek(6)
+			b, err := bufCon.Peek(6) // todo set a read deadline here
 			if err != nil {
 				rt.Logger.ErrorContext(ctx, "peek failed", "err", err.Error())
 				return