From 588dce918aeda0efc4db80b30525d28943c029cd Mon Sep 17 00:00:00 2001
From: Jeff Halter <868228+jhalter@users.noreply.github.com>
Date: Sat, 30 May 2026 16:16:32 -0700
Subject: Surface transaction handler errors instead of silently discarding
 them

Many handlers in internal/mobius/transaction_handlers.go returned an empty
transaction slice on error (return res / return nil), so the client received
no reply and the operation silently no-opped, with no log in most cases.

Convert these silent discards across the file to a consistent convention:
log the underlying cause via cc.Logger.Error and return cc.NewErrReply with a
user-facing message. Covers the file-operation, file-transfer, news,
user/account, and chat handler groups. Genuinely intentional no-reply paths
(target user offline, banned-nickname disconnect) are kept silent but now
carry an explanatory comment.

Also fixes adjacent defects found along the way:
- HandleSetFileInfo: a non-IsNotExist error from os.Rename during a directory
  rename was swallowed; it now returns an error reply.
- HandleUploadFile: when a resume is requested but no .incomplete file exists,
  fall back to a normal upload reply instead of discarding the reply.
- hotline.DecodeNewsPath: previously panicked on a 1-byte client-supplied
  field (slice out of range) and silently produced empty path components on
  truncated input. It now validates length and framing and returns an error,
  which the handlers already surface.

Adds regression tests for the new error replies, the upload resume fallback,
and DecodeNewsPath's malformed-input handling.
---
 hotline/field.go      | 14 ++++++++++++--
 hotline/field_test.go | 16 ++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

(limited to 'hotline')

diff --git a/hotline/field.go b/hotline/field.go
index 554f63d..99d0d35 100644
--- a/hotline/field.go
+++ b/hotline/field.go
@@ -5,6 +5,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"slices"
 )
 
@@ -137,19 +138,28 @@ func (f *Field) DecodeNewsPath() ([]string, error) {
 	if len(f.Data) == 0 {
 		return []string{}, nil
 	}
+	if len(f.Data) < 2 {
+		return nil, fmt.Errorf("news path too short: %d bytes", len(f.Data))
+	}
 
 	pathCount := binary.BigEndian.Uint16(f.Data[0:2])
 
 	scanner := bufio.NewScanner(bytes.NewReader(f.Data[2:]))
 	scanner.Split(newsPathScanner)
 
-	var paths []string
+	paths := make([]string, 0, pathCount)
 
 	for i := uint16(0); i < pathCount; i++ {
-		scanner.Scan()
+		if !scanner.Scan() {
+			return nil, fmt.Errorf("news path truncated: declared %d items, found %d", pathCount, i)
+		}
 		paths = append(paths, scanner.Text())
 	}
 
+	if err := scanner.Err(); err != nil {
+		return nil, fmt.Errorf("scan news path: %w", err)
+	}
+
 	return paths, nil
 }
 
diff --git a/hotline/field_test.go b/hotline/field_test.go
index 6dff102..b8d909b 100644
--- a/hotline/field_test.go
+++ b/hotline/field_test.go
@@ -247,6 +247,22 @@ func TestField_DecodeNewsPath(t *testing.T) {
 			want:    []string{},
 			wantErr: assert.NoError,
 		},
+		{
+			name:    "one byte of data returns an error instead of panicking",
+			fields:  fields{Data: []byte{0x00}},
+			want:    nil,
+			wantErr: assert.Error,
+		},
+		{
+			name: "declared path count exceeding available items returns an error",
+			fields: fields{Data: []byte{
+				0x00, 0x02, // path count = 2
+				0x00, 0x00, 0x05, // 2 bytes unused + 1 byte length (5)
+				0x48, 0x65, 0x6c, 0x6c, 0x6f, // "Hello" (only 1 item present)
+			}},
+			want:    nil,
+			wantErr: assert.Error,
+		},
 		{
 			name: "single path",
 			fields: fields{Data: []byte{
-- 
cgit