From 3f9221420a57b0b7534a0877925e363855274b1a Mon Sep 17 00:00:00 2001 From: Ruben Beltran del Rio Date: Wed, 5 Feb 2025 15:06:22 +0100 Subject: Account for 16 vs 32 bit integers in folder upload --- hotline/server.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'hotline') diff --git a/hotline/server.go b/hotline/server.go index ed3c041..a521a6b 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -603,10 +603,20 @@ func (s *Server) handleFileTransfer(ctx context.Context, rwc io.ReadWriter) erro s.Stats.Decrement(StatUploadsInProgress) }() + var transferSizeValue uint32 + switch len(fileTransfer.TransferSize) { + case 2: // 16-bit + transferSizeValue = uint32(binary.BigEndian.Uint16(fileTransfer.TransferSize)) + case 4: // 32-bit + transferSizeValue = binary.BigEndian.Uint32(fileTransfer.TransferSize) + default: + rLogger.Warn("Unexpected TransferSize length: %d bytes", len(fileTransfer.TransferSize)) + } + rLogger.Info( "Folder upload started", "dstPath", fullPath, - "TransferSize", binary.BigEndian.Uint32(fileTransfer.TransferSize), + "TransferSize", transferSizeValue, "FolderItemCount", fileTransfer.FolderItemCount, ) -- cgit From 24d753e4ecb06c467784732d7159b0db1b48aa9a Mon Sep 17 00:00:00 2001 From: Theo Knez <27211475+Knezzen@users.noreply.github.com> Date: Sat, 10 May 2025 20:36:09 +0200 Subject: Update server.go --- hotline/server.go | 173 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 120 insertions(+), 53 deletions(-) (limited to 'hotline') diff --git a/hotline/server.go b/hotline/server.go index a521a6b..fb75039 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -8,8 +8,6 @@ import ( "encoding/binary" "errors" "fmt" - "golang.org/x/text/encoding/charmap" - "golang.org/x/time/rate" "io" "log" "log/slog" @@ -18,6 +16,10 @@ import ( "strings" "sync" "time" + + "github.com/redis/go-redis/v9" + "golang.org/x/text/encoding/charmap" + "golang.org/x/time/rate" ) type contextKey string @@ -64,6 +66,8 @@ type Server struct { BanList BanMgr MessageBoard io.ReadWriteSeeker + + Redis *redis.Client } type Option = func(s *Server) @@ -269,33 +273,57 @@ func (s *Server) registerWithTrackers(ctx context.Context) { s.Logger.Info("Tracker registration enabled", "trackers", s.Config.Trackers) } + // Do the first registration immediately + if s.Config.EnableTrackerRegistration { + for _, t := range s.Config.Trackers { + tr := &TrackerRegistration{ + UserCount: len(s.ClientMgr.List()), + PassID: s.TrackerPassID, + Name: s.Config.Name, + Description: s.Config.Description, + } + binary.BigEndian.PutUint16(tr.Port[:], uint16(s.Port)) + + splitAddr := strings.Split(":", t) + if len(splitAddr) == 3 { + tr.Password = splitAddr[2] + } + + if err := register(&RealDialer{}, t, tr); err != nil { + s.Logger.Error(fmt.Sprintf("Unable to register with tracker %v", t), "error", err) + } + } + } + + ticker := time.NewTicker(trackerUpdateFrequency * time.Second) + defer ticker.Stop() + for { - if s.Config.EnableTrackerRegistration { - for _, t := range s.Config.Trackers { - tr := &TrackerRegistration{ - UserCount: len(s.ClientMgr.List()), - PassID: s.TrackerPassID, - Name: s.Config.Name, - Description: s.Config.Description, - } - binary.BigEndian.PutUint16(tr.Port[:], uint16(s.Port)) + select { + case <-ctx.Done(): + return + case <-ticker.C: + if s.Config.EnableTrackerRegistration { + for _, t := range s.Config.Trackers { + tr := &TrackerRegistration{ + UserCount: len(s.ClientMgr.List()), + PassID: s.TrackerPassID, + Name: s.Config.Name, + Description: s.Config.Description, + } + binary.BigEndian.PutUint16(tr.Port[:], uint16(s.Port)) - // Check the tracker string for a password. This is janky but avoids a breaking change to the Config - // Trackers field. - splitAddr := strings.Split(":", t) - if len(splitAddr) == 3 { - tr.Password = splitAddr[2] - } + splitAddr := strings.Split(":", t) + if len(splitAddr) == 3 { + tr.Password = splitAddr[2] + } - if err := register(&RealDialer{}, t, tr); err != nil { - s.Logger.Error(fmt.Sprintf("Unable to register with tracker %v", t), "error", err) + if err := register(&RealDialer{}, t, tr); err != nil { + s.Logger.Error(fmt.Sprintf("Unable to register with tracker %v", t), "error", err) + } } } } - // Using time.Ticker with for/select would be more idiomatic, but it's super annoying that it doesn't tick on - // first pass. Revist, maybe. - // https://github.com/golang/go/issues/17601 - time.Sleep(trackerUpdateFrequency * time.Second) } } @@ -372,24 +400,6 @@ func (s *Server) handleNewConnection(ctx context.Context, rwc io.ReadWriteCloser return fmt.Errorf("perform handshake: %w", err) } - // Check if remoteAddr is present in the ban list - ipAddr := strings.Split(remoteAddr, ":")[0] - if isBanned, banUntil := s.BanList.IsBanned(ipAddr); isBanned { - // permaban - if banUntil == nil { - sendBanMessage(rwc, "You are permanently banned on this server") - s.Logger.Debug("Disconnecting permanently banned IP", "remoteAddr", ipAddr) - return nil - } - - // temporary ban - if time.Now().Before(*banUntil) { - sendBanMessage(rwc, "You are temporarily banned on this server") - s.Logger.Debug("Disconnecting temporarily banned IP", "remoteAddr", ipAddr) - return nil - } - } - // Create a new scanner for parsing incoming bytes into transaction tokens scanner := bufio.NewScanner(rwc) scanner.Split(transactionScanner) @@ -406,17 +416,66 @@ func (s *Server) handleNewConnection(ctx context.Context, rwc io.ReadWriteCloser return fmt.Errorf("error writing login transaction: %w", err) } - c := s.NewClientConn(rwc, remoteAddr) - defer c.Disconnect() - - encodedPassword := clientLogin.GetField(FieldUserPassword).Data - c.Version = clientLogin.GetField(FieldVersion).Data - login := clientLogin.GetField(FieldUserLogin).DecodeObfuscatedString() if login == "" { login = GuestAccount } + // Check if remoteAddr is present in the ban list, we do this after we have the login name + ipAddr := strings.Split(remoteAddr, ":")[0] + if s.Redis != nil { + // Redis-based ban check + bannedUser, _ := s.Redis.SIsMember(ctx, "mobius:banned:users", login).Result() + bannedIP, _ := s.Redis.SIsMember(ctx, "mobius:banned:ips", ipAddr).Result() + if bannedUser { + s.Redis.SAdd(ctx, "mobius:banned:ips", ipAddr) + sendBanMessage(rwc, "You are banned on this server") + s.Logger.Debug("Disconnecting banned user", "login", login, "ip", ipAddr) + return nil + } + if bannedIP { + sendBanMessage(rwc, "You are banned on this server") + s.Logger.Debug("Disconnecting banned IP", "ip", ipAddr) + return nil + } + } else { + // Fallback to in-memory ban list + if isBanned, banUntil := s.BanList.IsBanned(ipAddr); isBanned { + // permaban + if banUntil == nil { + sendBanMessage(rwc, "You are permanently banned on this server") + s.Logger.Debug("Disconnecting permanently banned IP", "remoteAddr", ipAddr) + return nil + } + // temporary ban + if time.Now().Before(*banUntil) { + sendBanMessage(rwc, "You are temporarily banned on this server") + s.Logger.Debug("Disconnecting temporarily banned IP", "remoteAddr", ipAddr) + return nil + } + } + } + + c := s.NewClientConn(rwc, remoteAddr) + // Add the client to the list of connected clients + if s.Redis != nil { + s.Redis.SAdd(context.Background(), "mobius:online", login+"::"+ipAddr) + } + + // Remove the client from the list of connected clients when they disconnect + defer func() { + if s.Redis != nil { + s.Redis.SRem(context.Background(), "mobius:online", login+"::"+ipAddr) + if len(c.UserName) != 0 { + s.Redis.SRem(context.Background(), "mobius:online", login+":"+string(c.UserName)+":"+ipAddr) + } + } + c.Disconnect() + }() + + encodedPassword := clientLogin.GetField(FieldUserPassword).Data + c.Version = clientLogin.GetField(FieldVersion).Data + c.Logger = s.Logger.With("ip", ipAddr, "login", login) // If authentication fails, send error reply and close connection @@ -486,6 +545,14 @@ func (s *Server) handleNewConnection(ctx context.Context, rwc io.ReadWriteCloser c.Logger = c.Logger.With("name", string(c.UserName)) c.Logger.Info("Login successful") + // Update the Redis set with the new information + if s.Redis != nil && len(c.UserName) != 0 { + // Remove old entry (login::ip) + s.Redis.SRem(context.Background(), "mobius:online", login+"::"+ipAddr) + // Add new entry with login, nickname, ip + s.Redis.SAdd(context.Background(), "mobius:online", login+":"+string(c.UserName)+":"+ipAddr) + } + // Notify other clients on the server that the new user has logged in. For 1.5+ clients we don't have this // information yet, so we do it in TranAgreed instead for _, t := range c.NotifyOthers( @@ -605,12 +672,12 @@ func (s *Server) handleFileTransfer(ctx context.Context, rwc io.ReadWriter) erro var transferSizeValue uint32 switch len(fileTransfer.TransferSize) { - case 2: // 16-bit - transferSizeValue = uint32(binary.BigEndian.Uint16(fileTransfer.TransferSize)) - case 4: // 32-bit - transferSizeValue = binary.BigEndian.Uint32(fileTransfer.TransferSize) - default: - rLogger.Warn("Unexpected TransferSize length: %d bytes", len(fileTransfer.TransferSize)) + case 2: // 16-bit + transferSizeValue = uint32(binary.BigEndian.Uint16(fileTransfer.TransferSize)) + case 4: // 32-bit + transferSizeValue = binary.BigEndian.Uint32(fileTransfer.TransferSize) + default: + rLogger.Warn("Unexpected TransferSize length", "bytes", len(fileTransfer.TransferSize)) } rLogger.Info( -- cgit From 5cc6ed27177304f743ebefc79fa3a17481bf8c98 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Thu, 19 Jun 2025 15:33:04 -0700 Subject: Ensure temporary upload files are closed before rename This seems to be important on Windows! See #161 --- hotline/file_transfer.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) (limited to 'hotline') diff --git a/hotline/file_transfer.go b/hotline/file_transfer.go index 626cfff..f09e995 100644 --- a/hotline/file_transfer.go +++ b/hotline/file_transfer.go @@ -318,15 +318,16 @@ func UploadHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileTransfe if err == nil { return fmt.Errorf("existing file found: %s", fullPath) } - if errors.Is(err, fs.ErrNotExist) { - // If not found, open or create a new .incomplete file - file, err = os.OpenFile(fullPath+IncompleteFileSuffix, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) - if err != nil { - return err - } + + if !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("check file existence: %w", err) } - defer file.Close() + // If not found, open or create a new .incomplete file + file, err = os.OpenFile(fullPath+IncompleteFileSuffix, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) + if err != nil { + return fmt.Errorf("open temp file for uploade: %w", err) + } f, err := NewFileWrapper(fileStore, fullPath, 0) if err != nil { @@ -350,9 +351,16 @@ func UploadHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileTransfe } if err := receiveFile(rwc, file, rForkWriter, iForkWriter, fileTransfer.bytesSentCounter); err != nil { + _ = file.Close() // Close on error return fmt.Errorf("receive file: %v", err) } + // Close the file before attempting to rename it. + if err := file.Close(); err != nil { + return fmt.Errorf("close file: %v", err) + } + + // Rename the temporary upload file to the final file name. if err := fileStore.Rename(fullPath+".incomplete", fullPath); err != nil { return fmt.Errorf("rename incomplete file: %v", err) } @@ -665,6 +673,12 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT return err } + // Close the file before attempting to rename it. + if err := incWriter.Close(); err != nil { + return fmt.Errorf("close file: %v", err) + } + + // Rename the temporary upload file to the final file name. if err := os.Rename(filePath+".incomplete", filePath); err != nil { return err } -- cgit From 55b8e77c409761639e95168c77dc22c13e858b6b Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Thu, 26 Jun 2025 14:01:22 -0700 Subject: Replace filepath.Join with path.Join for Windows compatibility Replace all instances of filepath.Join with path.Join across the codebase to improve Windows compatibility following the guidance from https://github.com/golang/go/issues/44305. Key changes: - Replaced filepath.Join with path.Join in 14 files - Updated import statements appropriately - Resolved variable shadowing issues where function parameters named 'path' were conflicting with the path package - Maintained filepath imports where needed for OS-specific functions like filepath.Walk, filepath.IsAbs, filepath.Dir, and filepath.Base All tests pass, confirming the changes maintain functionality while improving cross-platform compatibility. --- cmd/mobius-hotline-server/main.go | 7 +++---- cmd/mobius-hotline-server/main_test.go | 16 ++++++++-------- hotline/file_path.go | 8 ++++---- hotline/file_transfer.go | 13 +++++++------ internal/mobius/account_manager.go | 12 ++++++------ internal/mobius/ban.go | 4 ++-- internal/mobius/ban_test.go | 8 ++++---- internal/mobius/threaded_news_test.go | 4 ++-- internal/mobius/transaction_handlers.go | 3 +-- 9 files changed, 37 insertions(+), 38 deletions(-) (limited to 'hotline') diff --git a/cmd/mobius-hotline-server/main.go b/cmd/mobius-hotline-server/main.go index e230771..afad4d1 100644 --- a/cmd/mobius-hotline-server/main.go +++ b/cmd/mobius-hotline-server/main.go @@ -10,7 +10,6 @@ import ( "os" "os/signal" "path" - "path/filepath" "syscall" "github.com/jhalter/mobius/hotline" @@ -108,7 +107,7 @@ func main() { os.Exit(1) } - srv.AccountManager, err = mobius.NewYAMLAccountManager(filepath.Join(*configDir, "Users/")) + srv.AccountManager, err = mobius.NewYAMLAccountManager(path.Join(*configDir, "Users/")) if err != nil { slogger.Error(fmt.Sprintf("Error loading accounts: %v", err)) os.Exit(1) @@ -120,7 +119,7 @@ func main() { os.Exit(1) } - bannerPath := filepath.Join(*configDir, config.BannerFile) + bannerPath := path.Join(*configDir, config.BannerFile) srv.Banner, err = os.ReadFile(bannerPath) if err != nil { slogger.Error(fmt.Sprintf("Error loading accounts: %v", err)) @@ -146,7 +145,7 @@ func main() { } // Let's try to reload the banner - bannerPath := filepath.Join(*configDir, config.BannerFile) + bannerPath := path.Join(*configDir, config.BannerFile) srv.Banner, err = os.ReadFile(bannerPath) if err != nil { slogger.Error(fmt.Sprintf("Error reloading banner: %v", err)) diff --git a/cmd/mobius-hotline-server/main_test.go b/cmd/mobius-hotline-server/main_test.go index 17a2cb4..ed63065 100644 --- a/cmd/mobius-hotline-server/main_test.go +++ b/cmd/mobius-hotline-server/main_test.go @@ -2,7 +2,7 @@ package main import ( "os" - "path/filepath" + "path" "testing" "github.com/jhalter/mobius/internal/mobius" @@ -30,7 +30,7 @@ func TestCopyDir(t *testing.T) { } for _, expectedFile := range expectedFiles { - fullPath := filepath.Join(dstDir, expectedFile) + fullPath := path.Join(dstDir, expectedFile) assert.FileExists(t, fullPath, "Expected file %s to exist", expectedFile) // Verify file is not empty @@ -46,7 +46,7 @@ func TestCopyDir(t *testing.T) { } for _, expectedDir := range expectedDirs { - fullPath := filepath.Join(dstDir, expectedDir) + fullPath := path.Join(dstDir, expectedDir) info, err := os.Stat(fullPath) require.NoError(t, err) assert.True(t, info.IsDir(), "Expected %s to be a directory", expectedDir) @@ -69,11 +69,11 @@ func TestCopyDirRecursive(t *testing.T) { require.NoError(t, err) // Verify nested structure is copied correctly - nestedPath := filepath.Join(dstDir, "Users", "admin.yaml") + nestedPath := path.Join(dstDir, "Users", "admin.yaml") assert.FileExists(t, nestedPath) // Verify nested Files directory - filesDir := filepath.Join(dstDir, "Files") + filesDir := path.Join(dstDir, "Files") info, err := os.Stat(filesDir) require.NoError(t, err) assert.True(t, info.IsDir()) @@ -81,7 +81,7 @@ func TestCopyDirRecursive(t *testing.T) { func TestCopyFile(t *testing.T) { dstDir := t.TempDir() - dstFile := filepath.Join(dstDir, "copied.yaml") + dstFile := path.Join(dstDir, "copied.yaml") // Copy a single file from embedded config err := copyFile("mobius/config/config.yaml", dstFile) @@ -99,7 +99,7 @@ func TestCopyFile(t *testing.T) { func TestCopyFileErrors(t *testing.T) { t.Run("source file does not exist", func(t *testing.T) { dstDir := t.TempDir() - err := copyFile("nonexistent.txt", filepath.Join(dstDir, "dest.txt")) + err := copyFile("nonexistent.txt", path.Join(dstDir, "dest.txt")) assert.Error(t, err) assert.Contains(t, err.Error(), "failed to open source file") }) @@ -118,7 +118,7 @@ func TestCopyDirPermissions(t *testing.T) { require.NoError(t, err) // Check directory permissions - info, err := os.Stat(filepath.Join(dstDir, "Users")) + info, err := os.Stat(path.Join(dstDir, "Users")) require.NoError(t, err) assert.True(t, info.IsDir()) diff --git a/hotline/file_path.go b/hotline/file_path.go index f4a27cc..a3a13f1 100644 --- a/hotline/file_path.go +++ b/hotline/file_path.go @@ -7,7 +7,7 @@ import ( "errors" "fmt" "io" - "path/filepath" + "path" "strings" ) @@ -112,13 +112,13 @@ func ReadPath(fileRoot string, filePath, fileName []byte) (fullPath string, err var subPath string for _, pathItem := range fp.Items { - subPath = filepath.Join("/", subPath, string(pathItem.Name)) + subPath = path.Join("/", subPath, string(pathItem.Name)) } - fullPath = filepath.Join( + fullPath = path.Join( fileRoot, subPath, - filepath.Join("/", string(fileName)), + path.Join("/", string(fileName)), ) fullPath, err = txtDecoder.String(fullPath) if err != nil { diff --git a/hotline/file_transfer.go b/hotline/file_transfer.go index f09e995..0ddac0b 100644 --- a/hotline/file_transfer.go +++ b/hotline/file_transfer.go @@ -11,6 +11,7 @@ import ( "log/slog" "math" "os" + "path" "path/filepath" "slices" "strings" @@ -200,7 +201,7 @@ func (fu *folderUpload) FormattedPath() string { pathData = pathData[3+segLen:] } - return filepath.Join(pathSegments...) + return path.Join(pathSegments...) } type FileHeader struct { @@ -566,8 +567,8 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT } if fu.IsFolder == [2]byte{0, 1} { - if _, err := os.Stat(filepath.Join(fullPath, fu.FormattedPath())); os.IsNotExist(err) { - if err := os.Mkdir(filepath.Join(fullPath, fu.FormattedPath()), 0777); err != nil { + if _, err := os.Stat(path.Join(fullPath, fu.FormattedPath())); os.IsNotExist(err) { + if err := os.Mkdir(path.Join(fullPath, fu.FormattedPath()), 0777); err != nil { return err } } @@ -580,7 +581,7 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT nextAction := DlFldrActionSendFile // Check if we have the full file already. If so, send dlFldrAction_NextFile to client to skip. - _, err := os.Stat(filepath.Join(fullPath, fu.FormattedPath())) + _, err := os.Stat(path.Join(fullPath, fu.FormattedPath())) if err != nil && !errors.Is(err, fs.ErrNotExist) { return err } @@ -589,7 +590,7 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT } // Check if we have a partial file already. If so, send dlFldrAction_ResumeFile to client to resume upload. - incompleteFile, err := os.Stat(filepath.Join(fullPath, fu.FormattedPath()+IncompleteFileSuffix)) + incompleteFile, err := os.Stat(path.Join(fullPath, fu.FormattedPath()+IncompleteFileSuffix)) if err != nil && !errors.Is(err, fs.ErrNotExist) { return err } @@ -642,7 +643,7 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT return err } - filePath := filepath.Join(fullPath, fu.FormattedPath()) + filePath := path.Join(fullPath, fu.FormattedPath()) hlFile, err := NewFileWrapper(fileStore, filePath, 0) if err != nil { diff --git a/internal/mobius/account_manager.go b/internal/mobius/account_manager.go index 72984b7..2558d3a 100644 --- a/internal/mobius/account_manager.go +++ b/internal/mobius/account_manager.go @@ -37,7 +37,7 @@ func NewYAMLAccountManager(accountDir string) (*YAMLAccountManager, error) { accounts: make(map[string]hotline.Account), } - matches, err := filepath.Glob(filepath.Join(accountDir, "*.yaml")) + matches, err := filepath.Glob(path.Join(accountDir, "*.yaml")) if err != nil { return nil, fmt.Errorf("list account files: %w", err) } @@ -77,7 +77,7 @@ func (am *YAMLAccountManager) Create(account hotline.Account) error { // Create account file, returning an error if one already exists. file, err := os.OpenFile( - filepath.Join(am.accountDir, path.Join("/", account.Login+".yaml")), + path.Join(am.accountDir, path.Join("/", account.Login+".yaml")), os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0644, ) if err != nil { @@ -107,8 +107,8 @@ func (am *YAMLAccountManager) Update(account hotline.Account, newLogin string) e // If the login has changed, rename the account file. if account.Login != newLogin { err := os.Rename( - filepath.Join(am.accountDir, path.Join("/", account.Login)+".yaml"), - filepath.Join(am.accountDir, path.Join("/", newLogin)+".yaml"), + path.Join(am.accountDir, path.Join("/", account.Login)+".yaml"), + path.Join(am.accountDir, path.Join("/", newLogin)+".yaml"), ) if err != nil { return fmt.Errorf("error renaming account file: %w", err) @@ -124,7 +124,7 @@ func (am *YAMLAccountManager) Update(account hotline.Account, newLogin string) e return err } - if err := os.WriteFile(filepath.Join(am.accountDir, newLogin+".yaml"), out, 0644); err != nil { + if err := os.WriteFile(path.Join(am.accountDir, newLogin+".yaml"), out, 0644); err != nil { return fmt.Errorf("error writing account file: %w", err) } @@ -161,7 +161,7 @@ func (am *YAMLAccountManager) Delete(login string) error { am.mu.Lock() defer am.mu.Unlock() - err := os.Remove(filepath.Join(am.accountDir, path.Join("/", login+".yaml"))) + err := os.Remove(path.Join(am.accountDir, path.Join("/", login+".yaml"))) if err != nil { return fmt.Errorf("delete account file: %v", err) } diff --git a/internal/mobius/ban.go b/internal/mobius/ban.go index e73b14a..781052b 100644 --- a/internal/mobius/ban.go +++ b/internal/mobius/ban.go @@ -4,7 +4,7 @@ import ( "fmt" "gopkg.in/yaml.v3" "os" - "path/filepath" + "path" "sync" "time" ) @@ -64,7 +64,7 @@ func (bf *BanFile) Add(ip string, until *time.Time) error { return fmt.Errorf("marshal yaml: %v", err) } - err = os.WriteFile(filepath.Join(bf.filePath), out, 0644) + err = os.WriteFile(path.Join(bf.filePath), out, 0644) if err != nil { return fmt.Errorf("write file: %v", err) } diff --git a/internal/mobius/ban_test.go b/internal/mobius/ban_test.go index f03f214..1bf68a4 100644 --- a/internal/mobius/ban_test.go +++ b/internal/mobius/ban_test.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/stretchr/testify/assert" "os" - "path/filepath" + "path" "sync" "testing" "time" @@ -26,9 +26,9 @@ func TestNewBanFile(t *testing.T) { }{ { name: "Valid path with valid content", - args: args{path: filepath.Join(cwd, "test", "config", "Banlist.yaml")}, + args: args{path: path.Join(cwd, "test", "config", "Banlist.yaml")}, want: &BanFile{ - filePath: filepath.Join(cwd, "test", "config", "Banlist.yaml"), + filePath: path.Join(cwd, "test", "config", "Banlist.yaml"), banList: map[string]*time.Time{"192.168.86.29": &testTime}, }, wantErr: assert.NoError, @@ -55,7 +55,7 @@ func TestAdd(t *testing.T) { defer os.RemoveAll(tmpDir) // Clean up the temporary directory. // Path to the temporary ban file. - tmpFilePath := filepath.Join(tmpDir, "banfile.yaml") + tmpFilePath := path.Join(tmpDir, "banfile.yaml") // Initialize BanFile. bf := &BanFile{ diff --git a/internal/mobius/threaded_news_test.go b/internal/mobius/threaded_news_test.go index dd06a28..7f5cdaa 100644 --- a/internal/mobius/threaded_news_test.go +++ b/internal/mobius/threaded_news_test.go @@ -5,7 +5,7 @@ import ( "github.com/jhalter/mobius/hotline" "github.com/stretchr/testify/assert" "os" - "path/filepath" + "path" "sync" "testing" ) @@ -164,7 +164,7 @@ func TestThreadedNewsYAML_CreateGrouping(t *testing.T) { defer os.RemoveAll(tmpDir) // Clean up the temporary directory. // Path to the temporary ban file. - tmpFilePath := filepath.Join(tmpDir, "ThreadedNews.yaml") + tmpFilePath := path.Join(tmpDir, "ThreadedNews.yaml") type fields struct { ThreadedNews hotline.ThreadedNews diff --git a/internal/mobius/transaction_handlers.go b/internal/mobius/transaction_handlers.go index c03704a..6553887 100644 --- a/internal/mobius/transaction_handlers.go +++ b/internal/mobius/transaction_handlers.go @@ -10,7 +10,6 @@ import ( "math/big" "os" "path" - "path/filepath" "strings" "time" @@ -453,7 +452,7 @@ func HandleNewFolder(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotl } for _, pathItem := range newFp.Items { - subPath = filepath.Join("/", subPath, string(pathItem.Name)) + subPath = path.Join("/", subPath, string(pathItem.Name)) } } newFolderPath := path.Join(cc.FileRoot(), subPath, folderName) -- cgit From 83430dba76359f3b84a50051dd3fffcbbef90c18 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Thu, 26 Jun 2025 17:31:07 -0700 Subject: Refactor FormattedPath to use scanner interface and add comprehensive tests - Replace manual byte slicing with bufio.Scanner for safer parsing - Add pathSegmentScanner implementing bufio.SplitFunc pattern - Add comprehensive table tests covering edge cases and special characters - Improve code safety with proper bounds checking - Follow established codebase patterns for binary data parsing --- hotline/file_transfer.go | 39 +++++++++++++++---- hotline/file_transfer_test.go | 91 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 7 deletions(-) (limited to 'hotline') diff --git a/hotline/file_transfer.go b/hotline/file_transfer.go index 0ddac0b..701259a 100644 --- a/hotline/file_transfer.go +++ b/hotline/file_transfer.go @@ -2,6 +2,7 @@ package hotline import ( "bufio" + "bytes" "crypto/rand" "encoding/binary" "errors" @@ -188,17 +189,41 @@ type folderUpload struct { // return n + 6, nil //} +// pathSegmentScanner implements bufio.SplitFunc for parsing path segments +func pathSegmentScanner(data []byte, _ bool) (advance int, token []byte, err error) { + if len(data) < 3 { + return 0, nil, nil + } + + segLen := int(data[2]) + totalLen := 3 + segLen + + if len(data) < totalLen { + return 0, nil, nil + } + + return totalLen, data[0:totalLen], nil +} + func (fu *folderUpload) FormattedPath() string { pathItemLen := binary.BigEndian.Uint16(fu.PathItemCount[:]) - var pathSegments []string - pathData := fu.FileNamePath + if pathItemLen == 0 { + return "" + } - // TODO: implement scanner interface instead? - for i := uint16(0); i < pathItemLen; i++ { - segLen := pathData[2] - pathSegments = append(pathSegments, string(pathData[3:3+segLen])) - pathData = pathData[3+segLen:] + var pathSegments []string + scanner := bufio.NewScanner(bytes.NewReader(fu.FileNamePath)) + scanner.Split(pathSegmentScanner) + + for scanner.Scan() && len(pathSegments) < int(pathItemLen) { + segmentData := scanner.Bytes() + if len(segmentData) >= 3 { + segLen := int(segmentData[2]) + if len(segmentData) >= 3+segLen { + pathSegments = append(pathSegments, string(segmentData[3:3+segLen])) + } + } } return path.Join(pathSegments...) diff --git a/hotline/file_transfer_test.go b/hotline/file_transfer_test.go index 0b549f8..ba29910 100644 --- a/hotline/file_transfer_test.go +++ b/hotline/file_transfer_test.go @@ -175,3 +175,94 @@ func TestFileHeader_Payload(t *testing.T) { }) } } + +func Test_folderUpload_FormattedPath(t *testing.T) { + tests := []struct { + name string + pathItemCount [2]byte + fileNamePath []byte + want string + }{ + { + name: "empty path", + pathItemCount: [2]byte{0x00, 0x00}, + fileNamePath: []byte{}, + want: "", + }, + { + name: "single path segment", + pathItemCount: [2]byte{0x00, 0x01}, + fileNamePath: []byte{ + 0x00, 0x00, // path separator + 0x03, // segment length + 0x66, 0x6f, 0x6f, // "foo" + }, + want: "foo", + }, + { + name: "multiple path segments", + pathItemCount: [2]byte{0x00, 0x03}, + fileNamePath: []byte{ + 0x00, 0x00, // path separator + 0x04, // segment length + 0x68, 0x6f, 0x6d, 0x65, // "home" + 0x00, 0x00, // path separator + 0x04, // segment length + 0x75, 0x73, 0x65, 0x72, // "user" + 0x00, 0x00, // path separator + 0x09, // segment length + 0x64, 0x6f, 0x63, 0x75, 0x6d, 0x65, 0x6e, 0x74, 0x73, // "documents" + }, + want: "home/user/documents", + }, + { + name: "path with spaces", + pathItemCount: [2]byte{0x00, 0x02}, + fileNamePath: []byte{ + 0x00, 0x00, // path separator + 0x07, // segment length + 0x4d, 0x79, 0x20, 0x46, 0x69, 0x6c, 0x65, // "My File" + 0x00, 0x00, // path separator + 0x0d, // segment length (13 bytes) + 0x49, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x61, 0x6e, 0x74, 0x2e, 0x74, 0x78, 0x74, // "Important.txt" + }, + want: "My File/Important.txt", + }, + { + name: "single character segments", + pathItemCount: [2]byte{0x00, 0x03}, + fileNamePath: []byte{ + 0x00, 0x00, // path separator + 0x01, // segment length + 0x61, // "a" + 0x00, 0x00, // path separator + 0x01, // segment length + 0x62, // "b" + 0x00, 0x00, // path separator + 0x01, // segment length + 0x63, // "c" + }, + want: "a/b/c", + }, + { + name: "path with special characters", + pathItemCount: [2]byte{0x00, 0x01}, + fileNamePath: []byte{ + 0x00, 0x00, // path separator + 0x08, // segment length + 0x74, 0x65, 0x73, 0x74, 0x40, 0x24, 0x25, 0x26, // "test@$%&" + }, + want: "test@$%&", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fu := &folderUpload{ + PathItemCount: tt.pathItemCount, + FileNamePath: tt.fileNamePath, + } + got := fu.FormattedPath() + assert.Equal(t, tt.want, got) + }) + } +} -- cgit From d2e125bd255e807aa4d374b23eb9c2bb02fe63a3 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Sun, 29 Jun 2025 17:49:35 -0700 Subject: Fix IPv6 address parsing in IP extraction Replace string splitting with net.SplitHostPort to properly handle both IPv4 and IPv6 addresses. Fixes issue where IPv6 addresses like [::1]:8080 were incorrectly parsed as "[" instead of "::1". --- hotline/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hotline') diff --git a/hotline/server.go b/hotline/server.go index fb75039..98b6132 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -228,7 +228,7 @@ func (s *Server) Serve(ctx context.Context, ln net.Listener) error { } go func() { - ipAddr := strings.Split(conn.RemoteAddr().(*net.TCPAddr).String(), ":")[0] + ipAddr, _, _ := net.SplitHostPort(conn.RemoteAddr().String()) connCtx := context.WithValue(ctx, contextKeyReq, requestCtx{ remoteAddr: conn.RemoteAddr().String(), @@ -422,7 +422,7 @@ func (s *Server) handleNewConnection(ctx context.Context, rwc io.ReadWriteCloser } // Check if remoteAddr is present in the ban list, we do this after we have the login name - ipAddr := strings.Split(remoteAddr, ":")[0] + ipAddr, _, _ := net.SplitHostPort(remoteAddr) if s.Redis != nil { // Redis-based ban check bannedUser, _ := s.Redis.SIsMember(ctx, "mobius:banned:users", login).Result() -- cgit From 2bef32a36300e64106a27851fe1eab349d2dbeef Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Mon, 30 Jun 2025 10:38:27 -0700 Subject: Refactor access bitmap unmarshal logic to use map-driven approach Replace 120+ lines of repetitive if statements with a compact map lookup. This improves maintainability by reducing code duplication and making it easier to add new access permissions. --- hotline/access.go | 164 +++++++++++++++--------------------------------------- 1 file changed, 46 insertions(+), 118 deletions(-) (limited to 'hotline') diff --git a/hotline/access.go b/hotline/access.go index 42e96c4..370f8c2 100644 --- a/hotline/access.go +++ b/hotline/access.go @@ -73,125 +73,53 @@ func (bits *AccessBitmap) UnmarshalYAML(unmarshal func(interface{}) error) error case map[string]interface{}: // Mobius versions >= v0.17.0 store the user access bitmap as map[string]bool to provide a human-readable view of // the account permissions. - if f, ok := v["DeleteFile"].(bool); ok && f { - bits.Set(AccessDeleteFile) + accessMap := map[string]int{ + "DeleteFile": AccessDeleteFile, + "UploadFile": AccessUploadFile, + "DownloadFile": AccessDownloadFile, + "RenameFile": AccessRenameFile, + "MoveFile": AccessMoveFile, + "CreateFolder": AccessCreateFolder, + "DeleteFolder": AccessDeleteFolder, + "RenameFolder": AccessRenameFolder, + "MoveFolder": AccessMoveFolder, + "ReadChat": AccessReadChat, + "SendChat": AccessSendChat, + "OpenChat": AccessOpenChat, + "CloseChat": AccessCloseChat, + "ShowInList": AccessShowInList, + "CreateUser": AccessCreateUser, + "DeleteUser": AccessDeleteUser, + "OpenUser": AccessOpenUser, + "ModifyUser": AccessModifyUser, + "ChangeOwnPass": AccessChangeOwnPass, + "NewsReadArt": AccessNewsReadArt, + "NewsPostArt": AccessNewsPostArt, + "DisconnectUser": AccessDisconUser, + "CannotBeDisconnected": AccessCannotBeDiscon, + "GetClientInfo": AccessGetClientInfo, + "UploadAnywhere": AccessUploadAnywhere, + "AnyName": AccessAnyName, + "NoAgreement": AccessNoAgreement, + "SetFileComment": AccessSetFileComment, + "SetFolderComment": AccessSetFolderComment, + "ViewDropBoxes": AccessViewDropBoxes, + "MakeAlias": AccessMakeAlias, + "Broadcast": AccessBroadcast, + "NewsDeleteArt": AccessNewsDeleteArt, + "NewsCreateCat": AccessNewsCreateCat, + "NewsDeleteCat": AccessNewsDeleteCat, + "NewsCreateFldr": AccessNewsCreateFldr, + "NewsDeleteFldr": AccessNewsDeleteFldr, + "SendPrivMsg": AccessSendPrivMsg, + "UploadFolder": AccessUploadFolder, + "DownloadFolder": AccessDownloadFolder, } - if f, ok := v["UploadFile"].(bool); ok && f { - bits.Set(AccessUploadFile) - } - if f, ok := v["DownloadFile"].(bool); ok && f { - bits.Set(AccessDownloadFile) - } - if f, ok := v["UploadFolder"].(bool); ok && f { - bits.Set(AccessUploadFolder) - } - if f, ok := v["DownloadFolder"].(bool); ok && f { - bits.Set(AccessDownloadFolder) - } - if f, ok := v["RenameFile"].(bool); ok && f { - bits.Set(AccessRenameFile) - } - if f, ok := v["MoveFile"].(bool); ok && f { - bits.Set(AccessMoveFile) - } - if f, ok := v["CreateFolder"].(bool); ok && f { - bits.Set(AccessCreateFolder) - } - if f, ok := v["DeleteFolder"].(bool); ok && f { - bits.Set(AccessDeleteFolder) - } - if f, ok := v["RenameFolder"].(bool); ok && f { - bits.Set(AccessRenameFolder) - } - if f, ok := v["MoveFolder"].(bool); ok && f { - bits.Set(AccessMoveFolder) - } - if f, ok := v["ReadChat"].(bool); ok && f { - bits.Set(AccessReadChat) - } - if f, ok := v["SendChat"].(bool); ok && f { - bits.Set(AccessSendChat) - } - if f, ok := v["OpenChat"].(bool); ok && f { - bits.Set(AccessOpenChat) - } - if f, ok := v["CloseChat"].(bool); ok && f { - bits.Set(AccessCloseChat) - } - if f, ok := v["ShowInList"].(bool); ok && f { - bits.Set(AccessShowInList) - } - if f, ok := v["CreateUser"].(bool); ok && f { - bits.Set(AccessCreateUser) - } - if f, ok := v["DeleteUser"].(bool); ok && f { - bits.Set(AccessDeleteUser) - } - if f, ok := v["OpenUser"].(bool); ok && f { - bits.Set(AccessOpenUser) - } - if f, ok := v["ModifyUser"].(bool); ok && f { - bits.Set(AccessModifyUser) - } - if f, ok := v["ChangeOwnPass"].(bool); ok && f { - bits.Set(AccessChangeOwnPass) - } - if f, ok := v["NewsReadArt"].(bool); ok && f { - bits.Set(AccessNewsReadArt) - } - if f, ok := v["NewsPostArt"].(bool); ok && f { - bits.Set(AccessNewsPostArt) - } - if f, ok := v["DisconnectUser"].(bool); ok && f { - bits.Set(AccessDisconUser) - } - if f, ok := v["CannotBeDisconnected"].(bool); ok && f { - bits.Set(AccessCannotBeDiscon) - } - if f, ok := v["GetClientInfo"].(bool); ok && f { - bits.Set(AccessGetClientInfo) - } - if f, ok := v["UploadAnywhere"].(bool); ok && f { - bits.Set(AccessUploadAnywhere) - } - if f, ok := v["AnyName"].(bool); ok && f { - bits.Set(AccessAnyName) - } - if f, ok := v["NoAgreement"].(bool); ok && f { - bits.Set(AccessNoAgreement) - } - if f, ok := v["SetFileComment"].(bool); ok && f { - bits.Set(AccessSetFileComment) - } - if f, ok := v["SetFolderComment"].(bool); ok && f { - bits.Set(AccessSetFolderComment) - } - if f, ok := v["ViewDropBoxes"].(bool); ok && f { - bits.Set(AccessViewDropBoxes) - } - if f, ok := v["MakeAlias"].(bool); ok && f { - bits.Set(AccessMakeAlias) - } - if f, ok := v["Broadcast"].(bool); ok && f { - bits.Set(AccessBroadcast) - } - if f, ok := v["NewsDeleteArt"].(bool); ok && f { - bits.Set(AccessNewsDeleteArt) - } - if f, ok := v["NewsCreateCat"].(bool); ok && f { - bits.Set(AccessNewsCreateCat) - } - if f, ok := v["NewsDeleteCat"].(bool); ok && f { - bits.Set(AccessNewsDeleteCat) - } - if f, ok := v["NewsCreateFldr"].(bool); ok && f { - bits.Set(AccessNewsCreateFldr) - } - if f, ok := v["NewsDeleteFldr"].(bool); ok && f { - bits.Set(AccessNewsDeleteFldr) - } - if f, ok := v["SendPrivMsg"].(bool); ok && f { - bits.Set(AccessSendPrivMsg) + + for key, accessBit := range accessMap { + if flag, ok := v[key].(bool); ok && flag { + bits.Set(accessBit) + } } } -- cgit From 437415ca92a98783d03e71c689bdbb58fe0a8d51 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Mon, 30 Jun 2025 13:31:56 -0700 Subject: Add type safety to field system with FieldType alias - Define FieldType as typed alias for [2]byte to improve type safety - Update all 47 field constants to use FieldType instead of raw [2]byte - Update Field struct to use FieldType for Type field - Update function signatures: NewField, GetField, Transaction.GetField - Fix field_test.go to use new FieldType in test cases - Maintains backward compatibility with zero runtime overhead - Enhances API clarity and prevents accidental field type misuse --- hotline/field.go | 125 +++++++++++++++++++++++++------------------------ hotline/field_test.go | 10 ++-- hotline/transaction.go | 2 +- 3 files changed, 70 insertions(+), 67 deletions(-) (limited to 'hotline') diff --git a/hotline/field.go b/hotline/field.go index 65ff394..1e6fc16 100644 --- a/hotline/field.go +++ b/hotline/field.go @@ -9,77 +9,80 @@ import ( "slices" ) +// FieldType represents a Hotline protocol field type identifier +type FieldType [2]byte + // List of Hotline protocol field types taken from the official 1.9 protocol document var ( - FieldError = [2]byte{0x00, 0x64} // 100 - FieldData = [2]byte{0x00, 0x65} // 101 - FieldUserName = [2]byte{0x00, 0x66} // 102 - FieldUserID = [2]byte{0x00, 0x67} // 103 - FieldUserIconID = [2]byte{0x00, 0x68} // 104 - FieldUserLogin = [2]byte{0x00, 0x69} // 105 - FieldUserPassword = [2]byte{0x00, 0x6A} // 106 - FieldRefNum = [2]byte{0x00, 0x6B} // 107 - FieldTransferSize = [2]byte{0x00, 0x6C} // 108 - FieldChatOptions = [2]byte{0x00, 0x6D} // 109 - FieldUserAccess = [2]byte{0x00, 0x6E} // 110 - FieldUserFlags = [2]byte{0x00, 0x70} // 112 - FieldOptions = [2]byte{0x00, 0x71} // 113 - FieldChatID = [2]byte{0x00, 0x72} // 114 - FieldChatSubject = [2]byte{0x00, 0x73} // 115 - FieldWaitingCount = [2]byte{0x00, 0x74} // 116 - FieldBannerType = [2]byte{0x00, 0x98} // 152 - FieldNoServerAgreement = [2]byte{0x00, 0x98} // 152 - FieldVersion = [2]byte{0x00, 0xA0} // 160 - FieldCommunityBannerID = [2]byte{0x00, 0xA1} // 161 - FieldServerName = [2]byte{0x00, 0xA2} // 162 - FieldFileNameWithInfo = [2]byte{0x00, 0xC8} // 200 - FieldFileName = [2]byte{0x00, 0xC9} // 201 - FieldFilePath = [2]byte{0x00, 0xCA} // 202 - FieldFileResumeData = [2]byte{0x00, 0xCB} // 203 - FieldFileTransferOptions = [2]byte{0x00, 0xCC} // 204 - FieldFileTypeString = [2]byte{0x00, 0xCD} // 205 - FieldFileCreatorString = [2]byte{0x00, 0xCE} // 206 - FieldFileSize = [2]byte{0x00, 0xCF} // 207 - FieldFileCreateDate = [2]byte{0x00, 0xD0} // 208 - FieldFileModifyDate = [2]byte{0x00, 0xD1} // 209 - FieldFileComment = [2]byte{0x00, 0xD2} // 210 - FieldFileNewName = [2]byte{0x00, 0xD3} // 211 - FieldFileNewPath = [2]byte{0x00, 0xD4} // 212 - FieldFileType = [2]byte{0x00, 0xD5} // 213 - FieldQuotingMsg = [2]byte{0x00, 0xD6} // 214 - FieldAutomaticResponse = [2]byte{0x00, 0xD7} // 215 - FieldFolderItemCount = [2]byte{0x00, 0xDC} // 220 - FieldUsernameWithInfo = [2]byte{0x01, 0x2C} // 300 - FieldNewsArtListData = [2]byte{0x01, 0x41} // 321 - FieldNewsCatName = [2]byte{0x01, 0x42} // 322 - FieldNewsCatListData15 = [2]byte{0x01, 0x43} // 323 - FieldNewsPath = [2]byte{0x01, 0x45} // 325 - FieldNewsArtID = [2]byte{0x01, 0x46} // 326 - FieldNewsArtDataFlav = [2]byte{0x01, 0x47} // 327 - FieldNewsArtTitle = [2]byte{0x01, 0x48} // 328 - FieldNewsArtPoster = [2]byte{0x01, 0x49} // 329 - FieldNewsArtDate = [2]byte{0x01, 0x4A} // 330 - FieldNewsArtPrevArt = [2]byte{0x01, 0x4B} // 331 - FieldNewsArtNextArt = [2]byte{0x01, 0x4C} // 332 - FieldNewsArtData = [2]byte{0x01, 0x4D} // 333 - FieldNewsArtParentArt = [2]byte{0x01, 0x4F} // 335 - FieldNewsArt1stChildArt = [2]byte{0x01, 0x50} // 336 - FieldNewsArtRecurseDel = [2]byte{0x01, 0x51} // 337 + FieldError = FieldType{0x00, 0x64} // 100 + FieldData = FieldType{0x00, 0x65} // 101 + FieldUserName = FieldType{0x00, 0x66} // 102 + FieldUserID = FieldType{0x00, 0x67} // 103 + FieldUserIconID = FieldType{0x00, 0x68} // 104 + FieldUserLogin = FieldType{0x00, 0x69} // 105 + FieldUserPassword = FieldType{0x00, 0x6A} // 106 + FieldRefNum = FieldType{0x00, 0x6B} // 107 + FieldTransferSize = FieldType{0x00, 0x6C} // 108 + FieldChatOptions = FieldType{0x00, 0x6D} // 109 + FieldUserAccess = FieldType{0x00, 0x6E} // 110 + FieldUserFlags = FieldType{0x00, 0x70} // 112 + FieldOptions = FieldType{0x00, 0x71} // 113 + FieldChatID = FieldType{0x00, 0x72} // 114 + FieldChatSubject = FieldType{0x00, 0x73} // 115 + FieldWaitingCount = FieldType{0x00, 0x74} // 116 + FieldBannerType = FieldType{0x00, 0x98} // 152 + FieldNoServerAgreement = FieldType{0x00, 0x98} // 152 + FieldVersion = FieldType{0x00, 0xA0} // 160 + FieldCommunityBannerID = FieldType{0x00, 0xA1} // 161 + FieldServerName = FieldType{0x00, 0xA2} // 162 + FieldFileNameWithInfo = FieldType{0x00, 0xC8} // 200 + FieldFileName = FieldType{0x00, 0xC9} // 201 + FieldFilePath = FieldType{0x00, 0xCA} // 202 + FieldFileResumeData = FieldType{0x00, 0xCB} // 203 + FieldFileTransferOptions = FieldType{0x00, 0xCC} // 204 + FieldFileTypeString = FieldType{0x00, 0xCD} // 205 + FieldFileCreatorString = FieldType{0x00, 0xCE} // 206 + FieldFileSize = FieldType{0x00, 0xCF} // 207 + FieldFileCreateDate = FieldType{0x00, 0xD0} // 208 + FieldFileModifyDate = FieldType{0x00, 0xD1} // 209 + FieldFileComment = FieldType{0x00, 0xD2} // 210 + FieldFileNewName = FieldType{0x00, 0xD3} // 211 + FieldFileNewPath = FieldType{0x00, 0xD4} // 212 + FieldFileType = FieldType{0x00, 0xD5} // 213 + FieldQuotingMsg = FieldType{0x00, 0xD6} // 214 + FieldAutomaticResponse = FieldType{0x00, 0xD7} // 215 + FieldFolderItemCount = FieldType{0x00, 0xDC} // 220 + FieldUsernameWithInfo = FieldType{0x01, 0x2C} // 300 + FieldNewsArtListData = FieldType{0x01, 0x41} // 321 + FieldNewsCatName = FieldType{0x01, 0x42} // 322 + FieldNewsCatListData15 = FieldType{0x01, 0x43} // 323 + FieldNewsPath = FieldType{0x01, 0x45} // 325 + FieldNewsArtID = FieldType{0x01, 0x46} // 326 + FieldNewsArtDataFlav = FieldType{0x01, 0x47} // 327 + FieldNewsArtTitle = FieldType{0x01, 0x48} // 328 + FieldNewsArtPoster = FieldType{0x01, 0x49} // 329 + FieldNewsArtDate = FieldType{0x01, 0x4A} // 330 + FieldNewsArtPrevArt = FieldType{0x01, 0x4B} // 331 + FieldNewsArtNextArt = FieldType{0x01, 0x4C} // 332 + FieldNewsArtData = FieldType{0x01, 0x4D} // 333 + FieldNewsArtParentArt = FieldType{0x01, 0x4F} // 335 + FieldNewsArt1stChildArt = FieldType{0x01, 0x50} // 336 + FieldNewsArtRecurseDel = FieldType{0x01, 0x51} // 337 // These fields are documented, but seemingly unused. - // FieldUserAlias = [2]byte{0x00, 0x6F} // 111 - // FieldNewsArtFlags = [2]byte{0x01, 0x4E} // 334 + // FieldUserAlias = FieldType{0x00, 0x6F} // 111 + // FieldNewsArtFlags = FieldType{0x01, 0x4E} // 334 ) type Field struct { - Type [2]byte // Type of field - FieldSize [2]byte // Size of the data field - Data []byte // Field data + Type FieldType // Type of field + FieldSize [2]byte // Size of the data field + Data []byte // Field data readOffset int // Internal offset to track read progress } -func NewField(fieldType [2]byte, data []byte) Field { +func NewField(fieldType FieldType, data []byte) Field { f := Field{ Type: fieldType, Data: make([]byte, len(data)), @@ -185,7 +188,7 @@ func (f *Field) Write(p []byte) (int, error) { return minFieldLen + dataSize, nil } -func GetField(id [2]byte, fields *[]Field) *Field { +func GetField(id FieldType, fields *[]Field) *Field { for _, field := range *fields { if id == field.Type { return &field diff --git a/hotline/field_test.go b/hotline/field_test.go index 213d9c3..676098c 100644 --- a/hotline/field_test.go +++ b/hotline/field_test.go @@ -102,7 +102,7 @@ func Test_fieldScanner(t *testing.T) { func TestField_Read(t *testing.T) { type fields struct { - ID [2]byte + Type FieldType FieldSize [2]byte Data []byte readOffset int @@ -121,7 +121,7 @@ func TestField_Read(t *testing.T) { { name: "returns field bytes", fields: fields{ - ID: [2]byte{0x00, 0x62}, + Type: FieldType{0x00, 0x62}, FieldSize: [2]byte{0x00, 0x03}, Data: []byte("hai!"), }, @@ -139,7 +139,7 @@ func TestField_Read(t *testing.T) { { name: "returns field bytes from readOffset", fields: fields{ - ID: [2]byte{0x00, 0x62}, + Type: FieldType{0x00, 0x62}, FieldSize: [2]byte{0x00, 0x03}, Data: []byte("hai!"), readOffset: 4, @@ -156,7 +156,7 @@ func TestField_Read(t *testing.T) { { name: "returns io.EOF when all bytes read", fields: fields{ - ID: [2]byte{0x00, 0x62}, + Type: FieldType{0x00, 0x62}, FieldSize: [2]byte{0x00, 0x03}, Data: []byte("hai!"), readOffset: 8, @@ -172,7 +172,7 @@ func TestField_Read(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { f := &Field{ - Type: tt.fields.ID, + Type: tt.fields.Type, FieldSize: tt.fields.FieldSize, Data: tt.fields.Data, readOffset: tt.fields.readOffset, diff --git a/hotline/transaction.go b/hotline/transaction.go index fa1e96d..a03030d 100644 --- a/hotline/transaction.go +++ b/hotline/transaction.go @@ -275,7 +275,7 @@ func (t *Transaction) Size() []byte { return bs } -func (t *Transaction) GetField(id [2]byte) *Field { +func (t *Transaction) GetField(id FieldType) *Field { for _, field := range t.Fields { if id == field.Type { return &field -- cgit From a9c6485fb00dbf2e4b55351c1ae74d4eaca3eec9 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Mon, 30 Jun 2025 15:24:19 -0700 Subject: Replace inappropriate panic calls with proper error handling - Convert panic in news article processing to return error instead - Replace panic in API stats endpoint with HTTP error response - Update ThreadedNewsMgr interface to return errors from ListArticles - Ensure server stability by handling errors gracefully --- hotline/news.go | 13 ++++++------- internal/mobius/api.go | 3 ++- internal/mobius/threaded_news.go | 2 +- internal/mobius/transaction_handlers.go | 5 ++++- 4 files changed, 13 insertions(+), 10 deletions(-) (limited to 'hotline') diff --git a/hotline/news.go b/hotline/news.go index a490a89..c61a76f 100644 --- a/hotline/news.go +++ b/hotline/news.go @@ -14,7 +14,7 @@ var ( ) type ThreadedNewsMgr interface { - ListArticles(newsPath []string) NewsArtListData + ListArticles(newsPath []string) (NewsArtListData, error) GetArticle(newsPath []string, articleID uint32) *NewsArtData DeleteArticle(newsPath []string, articleID uint32, recursive bool) error PostArticle(newsPath []string, parentArticleID uint32, article NewsArtData) error @@ -41,7 +41,7 @@ type NewsCategoryListData15 struct { readOffset int // Internal offset to track read progress } -func (newscat *NewsCategoryListData15) GetNewsArtListData() NewsArtListData { +func (newscat *NewsCategoryListData15) GetNewsArtListData() (NewsArtListData, error) { var newsArts []NewsArtList var newsArtsPayload []byte @@ -70,8 +70,7 @@ func (newscat *NewsCategoryListData15) GetNewsArtListData() NewsArtListData { for _, v := range newsArts { b, err := io.ReadAll(&v) if err != nil { - // TODO - panic(err) + return NewsArtListData{}, err } newsArtsPayload = append(newsArtsPayload, b...) } @@ -81,7 +80,7 @@ func (newscat *NewsCategoryListData15) GetNewsArtListData() NewsArtListData { Name: []byte{}, Description: []byte{}, NewsArtList: newsArtsPayload, - } + }, nil } // NewsArtData represents an individual news article. @@ -242,10 +241,10 @@ type MockThreadNewsMgr struct { mock.Mock } -func (m *MockThreadNewsMgr) ListArticles(newsPath []string) NewsArtListData { +func (m *MockThreadNewsMgr) ListArticles(newsPath []string) (NewsArtListData, error) { args := m.Called(newsPath) - return args.Get(0).(NewsArtListData) + return args.Get(0).(NewsArtListData), args.Error(1) } func (m *MockThreadNewsMgr) GetArticle(newsPath []string, articleID uint32) *NewsArtData { diff --git a/internal/mobius/api.go b/internal/mobius/api.go index 4dfc575..0a20d01 100644 --- a/internal/mobius/api.go +++ b/internal/mobius/api.go @@ -263,7 +263,8 @@ func (srv *APIServer) ReloadHandler(reloadFunc func()) func(w http.ResponseWrite func (srv *APIServer) RenderStats(w http.ResponseWriter, _ *http.Request) { u, err := json.Marshal(srv.hlServer.CurrentStats()) if err != nil { - panic(err) + http.Error(w, "failed to marshal stats", http.StatusInternalServerError) + return } _, _ = w.Write(u) diff --git a/internal/mobius/threaded_news.go b/internal/mobius/threaded_news.go index e9d9c22..67e8282 100644 --- a/internal/mobius/threaded_news.go +++ b/internal/mobius/threaded_news.go @@ -195,7 +195,7 @@ func (n *ThreadedNewsYAML) DeleteArticle(newsPath []string, articleID uint32, _ return n.writeFile() } -func (n *ThreadedNewsYAML) ListArticles(newsPath []string) hotline.NewsArtListData { +func (n *ThreadedNewsYAML) ListArticles(newsPath []string) (hotline.NewsArtListData, error) { n.mu.Lock() defer n.mu.Unlock() diff --git a/internal/mobius/transaction_handlers.go b/internal/mobius/transaction_handlers.go index d8c4cb5..5181e15 100644 --- a/internal/mobius/transaction_handlers.go +++ b/internal/mobius/transaction_handlers.go @@ -1319,7 +1319,10 @@ func HandleGetNewsArtNameList(cc *hotline.ClientConn, t *hotline.Transaction) (r return res } - nald := cc.Server.ThreadedNewsMgr.ListArticles(pathStrs) + nald, err := cc.Server.ThreadedNewsMgr.ListArticles(pathStrs) + if err != nil { + return res + } b, err := io.ReadAll(&nald) if err != nil { -- cgit From 262f66351484369fa65c2e23df81ef682ea06d89 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Tue, 1 Jul 2025 16:03:57 -0700 Subject: Add documentation and comprehensive test coverage for AccountManager - Add GoDoc comments to AccountManager interface and all YAMLAccountManager methods - Add complete table-driven test coverage for Create, Update, Get, and List methods - Tests follow project conventions with proper error handling and temporary directories - All tests pass with comprehensive verification of both memory state and file operations --- hotline/account_manager.go | 2 + internal/mobius/account_manager.go | 19 ++ internal/mobius/account_manager_test.go | 347 +++++++++++++++++++++++++++++++- 3 files changed, 362 insertions(+), 6 deletions(-) (limited to 'hotline') diff --git a/hotline/account_manager.go b/hotline/account_manager.go index 03621df..52c24f8 100644 --- a/hotline/account_manager.go +++ b/hotline/account_manager.go @@ -1,5 +1,7 @@ package hotline +// AccountManager provides an interface for managing user accounts, +// including creation, retrieval, updates, and deletion operations. type AccountManager interface { Create(account Account) error Update(account Account, newLogin string) error diff --git a/internal/mobius/account_manager.go b/internal/mobius/account_manager.go index 2558d3a..8859c58 100644 --- a/internal/mobius/account_manager.go +++ b/internal/mobius/account_manager.go @@ -24,6 +24,8 @@ func loadFromYAMLFile(path string, data interface{}) error { return decoder.Decode(data) } +// YAMLAccountManager implements AccountManager interface using YAML files for persistence. +// It maintains an in-memory cache of accounts and synchronizes with YAML files on disk. type YAMLAccountManager struct { accounts map[string]hotline.Account accountDir string @@ -31,6 +33,9 @@ type YAMLAccountManager struct { mu sync.Mutex } +// NewYAMLAccountManager creates a new YAML-based account manager that loads existing +// accounts from .yaml files in the specified directory. It also performs migration +// from old access flag format to new AccessBitmap format when needed. func NewYAMLAccountManager(accountDir string) (*YAMLAccountManager, error) { accountMgr := YAMLAccountManager{ accountDir: accountDir, @@ -71,6 +76,8 @@ func NewYAMLAccountManager(accountDir string) (*YAMLAccountManager, error) { return &accountMgr, nil } +// Create adds a new account by writing it to a YAML file and updating the in-memory cache. +// Returns an error if an account with the same login already exists. func (am *YAMLAccountManager) Create(account hotline.Account) error { am.mu.Lock() defer am.mu.Unlock() @@ -100,6 +107,8 @@ func (am *YAMLAccountManager) Create(account hotline.Account) error { return nil } +// Update modifies an existing account with new data and optionally renames it. +// If newLogin differs from account.Login, the account file is renamed accordingly. func (am *YAMLAccountManager) Update(account hotline.Account, newLogin string) error { am.mu.Lock() defer am.mu.Unlock() @@ -133,6 +142,8 @@ func (am *YAMLAccountManager) Update(account hotline.Account, newLogin string) e return nil } +// Get retrieves an account by login from the in-memory cache. +// Returns nil if the account is not found. func (am *YAMLAccountManager) Get(login string) *hotline.Account { am.mu.Lock() defer am.mu.Unlock() @@ -145,6 +156,7 @@ func (am *YAMLAccountManager) Get(login string) *hotline.Account { return &account } +// List returns all accounts from the in-memory cache as a slice. func (am *YAMLAccountManager) List() []hotline.Account { am.mu.Lock() defer am.mu.Unlock() @@ -157,6 +169,7 @@ func (am *YAMLAccountManager) List() []hotline.Account { return accounts } +// Delete removes an account by deleting its YAML file and removing it from the cache. func (am *YAMLAccountManager) Delete(login string) error { am.mu.Lock() defer am.mu.Unlock() @@ -171,34 +184,40 @@ func (am *YAMLAccountManager) Delete(login string) error { return nil } +// MockAccountManager provides a test double implementation of AccountManager using testify/mock. type MockAccountManager struct { mock.Mock } +// Create mocks the Create method for testing. func (m *MockAccountManager) Create(account hotline.Account) error { args := m.Called(account) return args.Error(0) } +// Update mocks the Update method for testing. func (m *MockAccountManager) Update(account hotline.Account, newLogin string) error { args := m.Called(account, newLogin) return args.Error(0) } +// Get mocks the Get method for testing. func (m *MockAccountManager) Get(login string) *hotline.Account { args := m.Called(login) return args.Get(0).(*hotline.Account) } +// List mocks the List method for testing. func (m *MockAccountManager) List() []hotline.Account { args := m.Called() return args.Get(0).([]hotline.Account) } +// Delete mocks the Delete method for testing. func (m *MockAccountManager) Delete(login string) error { args := m.Called(login) diff --git a/internal/mobius/account_manager_test.go b/internal/mobius/account_manager_test.go index a6f39ca..b0bfd79 100644 --- a/internal/mobius/account_manager_test.go +++ b/internal/mobius/account_manager_test.go @@ -1,25 +1,25 @@ package mobius import ( - "os" - "path" "github.com/jhalter/mobius/hotline" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "os" + "path" "testing" ) // copyTestFiles copies test config files to a temporary directory func copyTestFiles(t *testing.T, srcDir, dstDir string) { files := []string{"admin.yaml", "guest.yaml", "user-with-old-access-format.yaml"} - + for _, file := range files { srcPath := path.Join(srcDir, file) dstPath := path.Join(dstDir, file) - + data, err := os.ReadFile(srcPath) require.NoError(t, err, "Failed to read %s", srcPath) - + err = os.WriteFile(dstPath, data, 0644) require.NoError(t, err, "Failed to write %s", dstPath) } @@ -30,7 +30,7 @@ func TestNewYAMLAccountManager(t *testing.T) { // Create temporary directory and copy test files tempDir := t.TempDir() copyTestFiles(t, "test/config/Users", tempDir) - + // Use temp directory instead of original test directory got, err := NewYAMLAccountManager(tempDir) require.NoError(t, err) @@ -66,3 +66,338 @@ func TestNewYAMLAccountManager(t *testing.T) { ) }) } + +func TestYAMLAccountManager_Delete(t *testing.T) { + t.Run("successful deletions", func(t *testing.T) { + type args struct { + login string + } + tests := []struct { + name string + args args + }{ + { + name: "deletes existing guest account", + args: args{ + login: "guest", + }, + }, + { + name: "deletes existing admin account", + args: args{ + login: "admin", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temporary directory and copy test files + tempDir := t.TempDir() + copyTestFiles(t, "test/config/Users", tempDir) + + accountMgr, err := NewYAMLAccountManager(tempDir) + require.NoError(t, err) + + // Get initial state + initialAccounts := accountMgr.List() + initialCount := len(initialAccounts) + + // Verify account exists before deletion + account := accountMgr.Get(tt.args.login) + require.NotNilf(t, account, "Account %s should exist before deletion", tt.args.login) + + // Perform deletion + err = accountMgr.Delete(tt.args.login) + assert.NoErrorf(t, err, "Delete(%v)", tt.args.login) + + // Verify account no longer exists in memory + account = accountMgr.Get(tt.args.login) + assert.Nilf(t, account, "Delete(%v)", tt.args.login) + + // Verify file was deleted + filePath := path.Join(tempDir, tt.args.login+".yaml") + _, fileErr := os.Stat(filePath) + assert.Truef(t, os.IsNotExist(fileErr), "Delete(%v) - file should be deleted", tt.args.login) + + // Verify list count decreased + updatedAccounts := accountMgr.List() + expectedCount := initialCount - 1 + assert.Equalf(t, expectedCount, len(updatedAccounts), "Delete(%v)", tt.args.login) + + // Verify deleted account is not in the list + for _, account := range updatedAccounts { + assert.NotEqualf(t, tt.args.login, account.Login, "Delete(%v)", tt.args.login) + } + }) + } + }) + + t.Run("error cases", func(t *testing.T) { + type args struct { + login string + } + tests := []struct { + name string + args args + }{ + { + name: "returns error when deleting non-existent account", + args: args{ + login: "nonexistent", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temporary directory and copy test files + tempDir := t.TempDir() + copyTestFiles(t, "test/config/Users", tempDir) + + accountMgr, err := NewYAMLAccountManager(tempDir) + require.NoError(t, err) + + // Perform deletion + err = accountMgr.Delete(tt.args.login) + assert.Errorf(t, err, "Delete(%v)", tt.args.login) + }) + } + }) +} + +func TestYAMLAccountManager_Create(t *testing.T) { + t.Run("successful creation", func(t *testing.T) { + type args struct { + account hotline.Account + } + tests := []struct { + name string + args args + }{ + { + name: "creates new account with valid data", + args: args{ + account: hotline.Account{ + Login: "newuser", + Name: "New User", + Password: "hashedpassword", + Access: hotline.AccessBitmap{0x60, 0x70, 0x0c, 0x20, 0x03, 0x80, 0x00, 0x00}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + copyTestFiles(t, "test/config/Users", tempDir) + + accountMgr, err := NewYAMLAccountManager(tempDir) + require.NoError(t, err) + + initialCount := len(accountMgr.List()) + + err = accountMgr.Create(tt.args.account) + assert.NoErrorf(t, err, "Create(%v)", tt.args.account) + + // Verify account exists in memory + retrievedAccount := accountMgr.Get(tt.args.account.Login) + assert.NotNilf(t, retrievedAccount, "Create(%v)", tt.args.account) + assert.Equalf(t, &tt.args.account, retrievedAccount, "Create(%v)", tt.args.account) + + // Verify file was created + filePath := path.Join(tempDir, tt.args.account.Login+".yaml") + _, err = os.Stat(filePath) + assert.NoErrorf(t, err, "Create(%v) - file should exist", tt.args.account) + + // Verify list count increased + assert.Equalf(t, initialCount+1, len(accountMgr.List()), "Create(%v)", tt.args.account) + }) + } + }) + + t.Run("error cases", func(t *testing.T) { + type args struct { + account hotline.Account + } + tests := []struct { + name string + args args + }{ + { + name: "returns error when creating account with existing login", + args: args{ + account: hotline.Account{ + Login: "admin", + Name: "Duplicate Admin", + Password: "password", + Access: hotline.AccessBitmap{}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + copyTestFiles(t, "test/config/Users", tempDir) + + accountMgr, err := NewYAMLAccountManager(tempDir) + require.NoError(t, err) + + err = accountMgr.Create(tt.args.account) + assert.Errorf(t, err, "Create(%v)", tt.args.account) + }) + } + }) +} + +func TestYAMLAccountManager_Update(t *testing.T) { + t.Run("successful updates", func(t *testing.T) { + type args struct { + account hotline.Account + newLogin string + } + tests := []struct { + name string + args args + }{ + { + name: "updates account without changing login", + args: args{ + account: hotline.Account{ + Login: "guest", + Name: "Updated Guest", + Password: "newpassword", + Access: hotline.AccessBitmap{0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88}, + }, + newLogin: "guest", + }, + }, + { + name: "updates account with login change", + args: args{ + account: hotline.Account{ + Login: "guest", + Name: "Renamed Guest", + Password: "password", + Access: hotline.AccessBitmap{0x60, 0x70, 0x0c, 0x20, 0x03, 0x80, 0x00, 0x00}, + }, + newLogin: "renamedguest", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + copyTestFiles(t, "test/config/Users", tempDir) + + accountMgr, err := NewYAMLAccountManager(tempDir) + require.NoError(t, err) + + err = accountMgr.Update(tt.args.account, tt.args.newLogin) + assert.NoErrorf(t, err, "Update(%v, %v)", tt.args.account, tt.args.newLogin) + + // Verify updated account exists with new login + retrievedAccount := accountMgr.Get(tt.args.newLogin) + assert.NotNilf(t, retrievedAccount, "Update(%v, %v)", tt.args.account, tt.args.newLogin) + + expectedAccount := tt.args.account + expectedAccount.Login = tt.args.newLogin + assert.Equalf(t, &expectedAccount, retrievedAccount, "Update(%v, %v)", tt.args.account, tt.args.newLogin) + + // If login changed, verify old login no longer exists + if tt.args.account.Login != tt.args.newLogin { + oldAccount := accountMgr.Get(tt.args.account.Login) + assert.Nilf(t, oldAccount, "Update(%v, %v) - old login should not exist", tt.args.account, tt.args.newLogin) + + // Verify old file was renamed + oldFilePath := path.Join(tempDir, tt.args.account.Login+".yaml") + _, err = os.Stat(oldFilePath) + assert.Truef(t, os.IsNotExist(err), "Update(%v, %v) - old file should not exist", tt.args.account, tt.args.newLogin) + } + + // Verify new file exists + newFilePath := path.Join(tempDir, tt.args.newLogin+".yaml") + _, err = os.Stat(newFilePath) + assert.NoErrorf(t, err, "Update(%v, %v) - new file should exist", tt.args.account, tt.args.newLogin) + }) + } + }) +} + +func TestYAMLAccountManager_Get(t *testing.T) { + type args struct { + login string + } + tests := []struct { + name string + args args + want *hotline.Account + }{ + { + name: "returns existing account", + args: args{ + login: "admin", + }, + want: &hotline.Account{ + Name: "admin", + Login: "admin", + Password: "$2a$04$2itGEYx8C1N5bsfRSoC9JuonS3I4YfnyVPZHLSwp7kEInRX0yoB.a", + Access: hotline.AccessBitmap{0xff, 0xff, 0xef, 0xff, 0xff, 0x80, 0x00, 0x00}, + }, + }, + { + name: "returns nil for non-existent account", + args: args{ + login: "nonexistent", + }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + copyTestFiles(t, "test/config/Users", tempDir) + + accountMgr, err := NewYAMLAccountManager(tempDir) + require.NoError(t, err) + + got := accountMgr.Get(tt.args.login) + assert.Equalf(t, tt.want, got, "Get(%v)", tt.args.login) + }) + } +} + +func TestYAMLAccountManager_List(t *testing.T) { + tests := []struct { + name string + wantCount int + wantLogins []string + }{ + { + name: "returns all accounts", + wantCount: 3, + wantLogins: []string{"admin", "guest", "test-user"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + copyTestFiles(t, "test/config/Users", tempDir) + + accountMgr, err := NewYAMLAccountManager(tempDir) + require.NoError(t, err) + + got := accountMgr.List() + assert.Equalf(t, tt.wantCount, len(got), "List() count") + + // Check that all expected logins are present + gotLogins := make([]string, len(got)) + for i, account := range got { + gotLogins[i] = account.Login + } + + for _, expectedLogin := range tt.wantLogins { + assert.Containsf(t, gotLogins, expectedLogin, "List() should contain login %s", expectedLogin) + } + }) + } +} -- cgit From ee6629ad78ac62fa14371ea5ddb7474c1fe9c979 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Fri, 4 Jul 2025 17:24:02 -0700 Subject: Fix file handle close warnings by ignoring return values Updated all file close operations to use anonymous functions that ignore return values to satisfy golangci-lint errcheck warnings. --- cmd/mobius-hotline-server/main.go | 4 ++-- cmd/mobius-hotline-server/main_test.go | 2 +- hotline/files_test.go | 2 +- hotline/server.go | 4 ++-- hotline/tracker.go | 2 +- internal/mobius/account_manager.go | 4 ++-- internal/mobius/api.go | 12 ++++++------ internal/mobius/ban.go | 2 +- internal/mobius/ban_test.go | 2 +- internal/mobius/threaded_news.go | 2 +- internal/mobius/threaded_news_test.go | 4 ++-- 11 files changed, 20 insertions(+), 20 deletions(-) (limited to 'hotline') diff --git a/cmd/mobius-hotline-server/main.go b/cmd/mobius-hotline-server/main.go index 1ba11ae..aa41279 100644 --- a/cmd/mobius-hotline-server/main.go +++ b/cmd/mobius-hotline-server/main.go @@ -246,13 +246,13 @@ func copyFile(src, dst string) error { if err != nil { return fmt.Errorf("failed to open source file: %w", err) } - defer srcFile.Close() + defer func() { _ = srcFile.Close() }() dstFile, err := os.Create(dst) if err != nil { return fmt.Errorf("failed to create destination file: %w", err) } - defer dstFile.Close() + defer func() { _ = dstFile.Close() }() if _, err := io.Copy(dstFile, srcFile); err != nil { return fmt.Errorf("failed to copy file contents: %w", err) diff --git a/cmd/mobius-hotline-server/main_test.go b/cmd/mobius-hotline-server/main_test.go index ed63065..dec2ebf 100644 --- a/cmd/mobius-hotline-server/main_test.go +++ b/cmd/mobius-hotline-server/main_test.go @@ -166,7 +166,7 @@ func TestFindConfigPath(t *testing.T) { tmpDir := t.TempDir() originalDir, err := os.Getwd() require.NoError(t, err) - defer os.Chdir(originalDir) + defer func() { _ = os.Chdir(originalDir) }() err = os.Chdir(tmpDir) require.NoError(t, err) diff --git a/hotline/files_test.go b/hotline/files_test.go index 0a7eb7b..9bed670 100644 --- a/hotline/files_test.go +++ b/hotline/files_test.go @@ -148,7 +148,7 @@ func TestCalcItemCount(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tempDir) + defer func() { _ = os.RemoveAll(tempDir) }() // Create the test directory structure if err := createTestDirStructure(tempDir, tt.structure); err != nil { diff --git a/hotline/server.go b/hotline/server.go index 98b6132..19c7acc 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -235,7 +235,7 @@ func (s *Server) Serve(ctx context.Context, ln net.Listener) error { }) s.Logger.Info("Connection established", "ip", ipAddr) - defer conn.Close() + defer func() { _ = conn.Close() }() // Check if we have an existing rate limit for the IP and create one if we do not. rl, ok := s.rateLimiters[ipAddr] @@ -247,7 +247,7 @@ func (s *Server) Serve(ctx context.Context, ln net.Listener) error { // Check if the rate limit is exceeded and close the connection if so. if !rl.Allow() { s.Logger.Info("Rate limit exceeded", "RemoteAddr", conn.RemoteAddr()) - conn.Close() + _ = conn.Close() return } diff --git a/hotline/tracker.go b/hotline/tracker.go index 52963bf..edd973d 100644 --- a/hotline/tracker.go +++ b/hotline/tracker.go @@ -69,7 +69,7 @@ func register(dialer Dialer, tracker string, tr io.Reader) error { if err != nil { return fmt.Errorf("failed to dial tracker: %v", err) } - defer conn.Close() + defer func() { _ = conn.Close() }() if _, err := io.Copy(conn, tr); err != nil { return fmt.Errorf("failed to write to connection: %w", err) diff --git a/internal/mobius/account_manager.go b/internal/mobius/account_manager.go index 8859c58..d9169c9 100644 --- a/internal/mobius/account_manager.go +++ b/internal/mobius/account_manager.go @@ -18,7 +18,7 @@ func loadFromYAMLFile(path string, data interface{}) error { if err != nil { return err } - defer fh.Close() + defer func() { _ = fh.Close() }() decoder := yaml.NewDecoder(fh) return decoder.Decode(data) @@ -90,7 +90,7 @@ func (am *YAMLAccountManager) Create(account hotline.Account) error { if err != nil { return fmt.Errorf("create account file: %w", err) } - defer file.Close() + defer func() { _ = file.Close() }() b, err := yaml.Marshal(account) if err != nil { diff --git a/internal/mobius/api.go b/internal/mobius/api.go index 0a20d01..f912f60 100644 --- a/internal/mobius/api.go +++ b/internal/mobius/api.go @@ -125,7 +125,7 @@ func (srv *APIServer) OnlineHandler(w http.ResponseWriter, r *http.Request) { } } - json.NewEncoder(w).Encode(users) + _ = json.NewEncoder(w).Encode(users) } type BanRequest struct { @@ -169,7 +169,7 @@ func (srv *APIServer) BanHandler(w http.ResponseWriter, r *http.Request) { } } - w.Write([]byte(`{"msg":"banned"}`)) + _, _ = w.Write([]byte(`{"msg":"banned"}`)) } func (srv *APIServer) UnbanHandler(w http.ResponseWriter, r *http.Request) { @@ -198,7 +198,7 @@ func (srv *APIServer) UnbanHandler(w http.ResponseWriter, r *http.Request) { // TODO: Fallback } - w.Write([]byte(`{"msg":"unbanned"}`)) + _, _ = w.Write([]byte(`{"msg":"unbanned"}`)) } func (srv *APIServer) ListBannedIPsHandler(w http.ResponseWriter, r *http.Request) { @@ -208,7 +208,7 @@ func (srv *APIServer) ListBannedIPsHandler(w http.ResponseWriter, r *http.Reques http.Error(w, "failed to fetch banned IPs", http.StatusInternalServerError) return } - json.NewEncoder(w).Encode(ips) + _ = json.NewEncoder(w).Encode(ips) } else { // TODO: Fallback } @@ -221,7 +221,7 @@ func (srv *APIServer) ListBannedUsernamesHandler(w http.ResponseWriter, r *http. http.Error(w, "failed to fetch banned usernames", http.StatusInternalServerError) return } - json.NewEncoder(w).Encode(users) + _ = json.NewEncoder(w).Encode(users) } else { // TODO: Fallback } @@ -234,7 +234,7 @@ func (srv *APIServer) ListBannedNicknamesHandler(w http.ResponseWriter, r *http. http.Error(w, "failed to fetch banned nicknames", http.StatusInternalServerError) return } - json.NewEncoder(w).Encode(nicks) + _ = json.NewEncoder(w).Encode(nicks) } else { // TODO: Fallback } diff --git a/internal/mobius/ban.go b/internal/mobius/ban.go index 781052b..b4fde95 100644 --- a/internal/mobius/ban.go +++ b/internal/mobius/ban.go @@ -43,7 +43,7 @@ func (bf *BanFile) Load() error { if err != nil { return fmt.Errorf("open file: %v", err) } - defer fh.Close() + defer func() { _ = fh.Close() }() err = yaml.NewDecoder(fh).Decode(&bf.banList) if err != nil { diff --git a/internal/mobius/ban_test.go b/internal/mobius/ban_test.go index 1bf68a4..9f1f5d8 100644 --- a/internal/mobius/ban_test.go +++ b/internal/mobius/ban_test.go @@ -52,7 +52,7 @@ func TestAdd(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp directory: %v", err) } - defer os.RemoveAll(tmpDir) // Clean up the temporary directory. + defer func() { _ = os.RemoveAll(tmpDir) }() // Clean up the temporary directory. // Path to the temporary ban file. tmpFilePath := path.Join(tmpDir, "banfile.yaml") diff --git a/internal/mobius/threaded_news.go b/internal/mobius/threaded_news.go index 67e8282..c7daea4 100644 --- a/internal/mobius/threaded_news.go +++ b/internal/mobius/threaded_news.go @@ -218,7 +218,7 @@ func (n *ThreadedNewsYAML) Load() error { if err != nil { return err } - defer fh.Close() + defer func() { _ = fh.Close() }() n.ThreadedNews = hotline.ThreadedNews{} diff --git a/internal/mobius/threaded_news_test.go b/internal/mobius/threaded_news_test.go index 7f5cdaa..2ff8a84 100644 --- a/internal/mobius/threaded_news_test.go +++ b/internal/mobius/threaded_news_test.go @@ -52,7 +52,7 @@ func TestLoadFromYAMLFile(t *testing.T) { if tt.content != "" { err := os.WriteFile(tt.fileName, []byte(tt.content), 0644) assert.NoError(t, err) - defer os.Remove(tt.fileName) // Cleanup the file after the test + defer func() { _ = os.Remove(tt.fileName) }() // Cleanup the file after the test } var data TestData @@ -161,7 +161,7 @@ func TestThreadedNewsYAML_CreateGrouping(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp directory: %v", err) } - defer os.RemoveAll(tmpDir) // Clean up the temporary directory. + defer func() { _ = os.RemoveAll(tmpDir) }() // Clean up the temporary directory. // Path to the temporary ban file. tmpFilePath := path.Join(tmpDir, "ThreadedNews.yaml") -- cgit From 4f8ec72edb0951c60b71a9fbbef0c16923bf5529 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Fri, 4 Jul 2025 21:24:48 -0700 Subject: Add comprehensive test coverage for Stats and fix decrement edge case - Add complete test suite for Stats with 100% coverage - Fix Decrement method to prevent negative values - Test all methods: Increment, Decrement, Set, Get, Values - Cover edge cases including zero decrements and mixed operations --- hotline/stats.go | 4 +- hotline/stats_test.go | 293 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 296 insertions(+), 1 deletion(-) create mode 100644 hotline/stats_test.go (limited to 'hotline') diff --git a/hotline/stats.go b/hotline/stats.go index 316a67d..9731601 100644 --- a/hotline/stats.go +++ b/hotline/stats.go @@ -61,7 +61,9 @@ func (s *Stats) Decrement(key int) { s.mu.Lock() defer s.mu.Unlock() - s.stats[key]-- + if s.stats[key] > 0 { + s.stats[key]-- + } } func (s *Stats) Set(key, val int) { diff --git a/hotline/stats_test.go b/hotline/stats_test.go new file mode 100644 index 0000000..2174227 --- /dev/null +++ b/hotline/stats_test.go @@ -0,0 +1,293 @@ +package hotline + +import ( + "github.com/stretchr/testify/assert" + "testing" + "time" +) + +func TestStats_Increment(t *testing.T) { + tests := []struct { + name string + keys []int + expected map[int]int + }{ + { + name: "single key increment", + keys: []int{StatCurrentlyConnected}, + expected: map[int]int{ + StatCurrentlyConnected: 1, + }, + }, + { + name: "multiple keys increment", + keys: []int{StatCurrentlyConnected, StatDownloadCounter, StatUploadCounter}, + expected: map[int]int{ + StatCurrentlyConnected: 1, + StatDownloadCounter: 1, + StatUploadCounter: 1, + }, + }, + { + name: "duplicate keys increment", + keys: []int{StatCurrentlyConnected, StatCurrentlyConnected}, + expected: map[int]int{ + StatCurrentlyConnected: 2, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stats := NewStats() + stats.Increment(tt.keys...) + + for key, expectedVal := range tt.expected { + assert.Equal(t, expectedVal, stats.Get(key)) + } + }) + } +} + +func TestStats_Increment_Multiple_Calls(t *testing.T) { + stats := NewStats() + + stats.Increment(StatCurrentlyConnected) + assert.Equal(t, 1, stats.Get(StatCurrentlyConnected)) + + stats.Increment(StatCurrentlyConnected) + assert.Equal(t, 2, stats.Get(StatCurrentlyConnected)) + + stats.Increment(StatCurrentlyConnected, StatDownloadCounter) + assert.Equal(t, 3, stats.Get(StatCurrentlyConnected)) + assert.Equal(t, 1, stats.Get(StatDownloadCounter)) +} + +func TestStats_Decrement(t *testing.T) { + tests := []struct { + name string + setupValue int + key int + expected int + }{ + { + name: "decrement from positive value", + setupValue: 5, + key: StatCurrentlyConnected, + expected: 4, + }, + { + name: "decrement from zero stays zero", + setupValue: 0, + key: StatCurrentlyConnected, + expected: 0, + }, + { + name: "decrement from one", + setupValue: 1, + key: StatCurrentlyConnected, + expected: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stats := NewStats() + stats.Set(tt.key, tt.setupValue) + + stats.Decrement(tt.key) + + assert.Equal(t, tt.expected, stats.Get(tt.key)) + }) + } +} + +func TestStats_Decrement_Multiple_Calls(t *testing.T) { + stats := NewStats() + + stats.Set(StatCurrentlyConnected, 10) + assert.Equal(t, 10, stats.Get(StatCurrentlyConnected)) + + stats.Decrement(StatCurrentlyConnected) + assert.Equal(t, 9, stats.Get(StatCurrentlyConnected)) + + stats.Decrement(StatCurrentlyConnected) + assert.Equal(t, 8, stats.Get(StatCurrentlyConnected)) +} + +func TestStats_Set(t *testing.T) { + tests := []struct { + name string + key int + value int + expected int + }{ + { + name: "set positive value", + key: StatCurrentlyConnected, + value: 42, + expected: 42, + }, + { + name: "set zero value", + key: StatDownloadCounter, + value: 0, + expected: 0, + }, + { + name: "set negative value", + key: StatUploadCounter, + value: -5, + expected: -5, + }, + { + name: "overwrite existing value", + key: StatConnectionPeak, + value: 100, + expected: 100, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stats := NewStats() + + if tt.name == "overwrite existing value" { + stats.Set(tt.key, 50) + } + + stats.Set(tt.key, tt.value) + + assert.Equal(t, tt.expected, stats.Get(tt.key)) + }) + } +} + +func TestStats_Get(t *testing.T) { + tests := []struct { + name string + key int + setValue int + expected int + }{ + { + name: "get initialized value", + key: StatCurrentlyConnected, + setValue: 0, + expected: 0, + }, + { + name: "get set value", + key: StatDownloadCounter, + setValue: 25, + expected: 25, + }, + { + name: "get after increment", + key: StatUploadCounter, + setValue: -1, + expected: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stats := NewStats() + + if tt.name == "get after increment" { + stats.Increment(tt.key) + } else { + stats.Set(tt.key, tt.setValue) + } + + result := stats.Get(tt.key) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestStats_Get_Default_Values(t *testing.T) { + stats := NewStats() + + expectedDefaults := map[int]int{ + StatCurrentlyConnected: 0, + StatDownloadsInProgress: 0, + StatUploadsInProgress: 0, + StatWaitingDownloads: 0, + StatConnectionPeak: 0, + StatDownloadCounter: 0, + StatUploadCounter: 0, + StatConnectionCounter: 0, + } + + for key, expected := range expectedDefaults { + assert.Equal(t, expected, stats.Get(key)) + } +} + +func TestStats_Values(t *testing.T) { + stats := NewStats() + + // Test default values + values := stats.Values() + + assert.Equal(t, 0, values["CurrentlyConnected"]) + assert.Equal(t, 0, values["DownloadsInProgress"]) + assert.Equal(t, 0, values["UploadsInProgress"]) + assert.Equal(t, 0, values["WaitingDownloads"]) + assert.Equal(t, 0, values["ConnectionPeak"]) + assert.Equal(t, 0, values["ConnectionCounter"]) + assert.Equal(t, 0, values["DownloadCounter"]) + assert.Equal(t, 0, values["UploadCounter"]) + assert.NotNil(t, values["Since"]) + + // Verify Since is a time.Time + _, ok := values["Since"].(time.Time) + assert.True(t, ok, "Since should be a time.Time") +} + +func TestStats_Values_WithModifiedStats(t *testing.T) { + stats := NewStats() + + // Modify some stats + stats.Set(StatCurrentlyConnected, 10) + stats.Set(StatDownloadsInProgress, 5) + stats.Increment(StatConnectionCounter) + stats.Increment(StatDownloadCounter, StatUploadCounter) + + values := stats.Values() + + assert.Equal(t, 10, values["CurrentlyConnected"]) + assert.Equal(t, 5, values["DownloadsInProgress"]) + assert.Equal(t, 0, values["UploadsInProgress"]) + assert.Equal(t, 0, values["WaitingDownloads"]) + assert.Equal(t, 0, values["ConnectionPeak"]) + assert.Equal(t, 1, values["ConnectionCounter"]) + assert.Equal(t, 1, values["DownloadCounter"]) + assert.Equal(t, 1, values["UploadCounter"]) +} + +func TestStats_Values_ContainsAllKeys(t *testing.T) { + stats := NewStats() + values := stats.Values() + + expectedKeys := []string{ + "CurrentlyConnected", + "DownloadsInProgress", + "UploadsInProgress", + "WaitingDownloads", + "ConnectionPeak", + "ConnectionCounter", + "DownloadCounter", + "UploadCounter", + "Since", + } + + for _, key := range expectedKeys { + _, exists := values[key] + assert.True(t, exists, "Key %s should exist in Values() output", key) + } + + // Should have exactly 9 keys + assert.Equal(t, 9, len(values)) +} \ No newline at end of file -- cgit From 9e19aa65d1cf6740c1280279b0b70f9423cffbd7 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Fri, 4 Jul 2025 21:25:52 -0700 Subject: Add comprehensive test coverage for AccessBitmap YAML marshaling - Add tests for UnmarshalYAML covering array and map formats - Add tests for MarshalYAML with various permission combinations - Cover edge cases including empty bitmaps and full permissions - Ensure backward compatibility with legacy array format --- hotline/access_test.go | 407 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 407 insertions(+) (limited to 'hotline') diff --git a/hotline/access_test.go b/hotline/access_test.go index f2755f3..fa2f6f6 100644 --- a/hotline/access_test.go +++ b/hotline/access_test.go @@ -2,6 +2,9 @@ package hotline import ( "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" + "strings" "testing" ) @@ -37,3 +40,407 @@ func Test_accessBitmap_IsSet(t *testing.T) { }) } } + +func Test_accessBitmap_UnmarshalYAML(t *testing.T) { + // Test direct method call for explicit coverage + t.Run("direct method call", func(t *testing.T) { + var bits AccessBitmap + err := bits.UnmarshalYAML(func(v interface{}) error { + switch ptr := v.(type) { + case *interface{}: + *ptr = map[string]interface{}{ + "DownloadFile": true, + "UploadFile": true, + } + } + return nil + }) + require.NoError(t, err) + assert.True(t, bits.IsSet(AccessDownloadFile)) + assert.True(t, bits.IsSet(AccessUploadFile)) + }) + + // Original YAML unmarshaling tests + tests := []struct { + name string + yamlData string + expected AccessBitmap + wantErr bool + }{ + { + name: "unmarshal array format (legacy)", + yamlData: "access: [96, 112, 12, 32, 3, 128, 0, 0]", + expected: AccessBitmap{96, 112, 12, 32, 3, 128, 0, 0}, + wantErr: false, + }, + { + name: "unmarshal map format with true values", + yamlData: `access: + DownloadFile: true + UploadFile: true + DeleteFile: true`, + expected: func() AccessBitmap { + var bits AccessBitmap + bits.Set(AccessDownloadFile) + bits.Set(AccessUploadFile) + bits.Set(AccessDeleteFile) + return bits + }(), + wantErr: false, + }, + { + name: "unmarshal map format with false values", + yamlData: `access: + DownloadFile: false + UploadFile: false + DeleteFile: false`, + expected: AccessBitmap{}, + wantErr: false, + }, + { + name: "unmarshal map format with mixed values", + yamlData: `access: + DownloadFile: true + UploadFile: false + DeleteFile: true + CreateFolder: true`, + expected: func() AccessBitmap { + var bits AccessBitmap + bits.Set(AccessDownloadFile) + bits.Set(AccessDeleteFile) + bits.Set(AccessCreateFolder) + return bits + }(), + wantErr: false, + }, + { + name: "unmarshal map format with all permissions", + yamlData: `access: + DeleteFile: true + UploadFile: true + DownloadFile: true + RenameFile: true + MoveFile: true + CreateFolder: true + DeleteFolder: true + RenameFolder: true + MoveFolder: true + ReadChat: true + SendChat: true + OpenChat: true + CloseChat: true + ShowInList: true + CreateUser: true + DeleteUser: true + OpenUser: true + ModifyUser: true + ChangeOwnPass: true + NewsReadArt: true + NewsPostArt: true + DisconnectUser: true + CannotBeDisconnected: true + GetClientInfo: true + UploadAnywhere: true + AnyName: true + NoAgreement: true + SetFileComment: true + SetFolderComment: true + ViewDropBoxes: true + MakeAlias: true + Broadcast: true + NewsDeleteArt: true + NewsCreateCat: true + NewsDeleteCat: true + NewsCreateFldr: true + NewsDeleteFldr: true + SendPrivMsg: true + UploadFolder: true + DownloadFolder: true`, + expected: func() AccessBitmap { + var bits AccessBitmap + bits.Set(AccessDeleteFile) + bits.Set(AccessUploadFile) + bits.Set(AccessDownloadFile) + bits.Set(AccessRenameFile) + bits.Set(AccessMoveFile) + bits.Set(AccessCreateFolder) + bits.Set(AccessDeleteFolder) + bits.Set(AccessRenameFolder) + bits.Set(AccessMoveFolder) + bits.Set(AccessReadChat) + bits.Set(AccessSendChat) + bits.Set(AccessOpenChat) + bits.Set(AccessCloseChat) + bits.Set(AccessShowInList) + bits.Set(AccessCreateUser) + bits.Set(AccessDeleteUser) + bits.Set(AccessOpenUser) + bits.Set(AccessModifyUser) + bits.Set(AccessChangeOwnPass) + bits.Set(AccessNewsReadArt) + bits.Set(AccessNewsPostArt) + bits.Set(AccessDisconUser) + bits.Set(AccessCannotBeDiscon) + bits.Set(AccessGetClientInfo) + bits.Set(AccessUploadAnywhere) + bits.Set(AccessAnyName) + bits.Set(AccessNoAgreement) + bits.Set(AccessSetFileComment) + bits.Set(AccessSetFolderComment) + bits.Set(AccessViewDropBoxes) + bits.Set(AccessMakeAlias) + bits.Set(AccessBroadcast) + bits.Set(AccessNewsDeleteArt) + bits.Set(AccessNewsCreateCat) + bits.Set(AccessNewsDeleteCat) + bits.Set(AccessNewsCreateFldr) + bits.Set(AccessNewsDeleteFldr) + bits.Set(AccessSendPrivMsg) + bits.Set(AccessUploadFolder) + bits.Set(AccessDownloadFolder) + return bits + }(), + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var data struct { + Access AccessBitmap `yaml:"access"` + } + + err := yaml.Unmarshal([]byte(tt.yamlData), &data) + + if tt.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.expected, data.Access) + }) + } +} + +func Test_accessBitmap_MarshalYAML(t *testing.T) { + tests := []struct { + name string + bits AccessBitmap + expected string + }{ + { + name: "empty access bitmap", + bits: AccessBitmap{}, + expected: ` DownloadFile: false + DownloadFolder: false + UploadFile: false + UploadFolder: false + DeleteFile: false + RenameFile: false + MoveFile: false + CreateFolder: false + DeleteFolder: false + RenameFolder: false + MoveFolder: false + ReadChat: false + SendChat: false + OpenChat: false + CloseChat: false + ShowInList: false + CreateUser: false + DeleteUser: false + OpenUser: false + ModifyUser: false + ChangeOwnPass: false + NewsReadArt: false + NewsPostArt: false + DisconnectUser: false + CannotBeDisconnected: false + GetClientInfo: false + UploadAnywhere: false + AnyName: false + NoAgreement: false + SetFileComment: false + SetFolderComment: false + ViewDropBoxes: false + MakeAlias: false + Broadcast: false + NewsDeleteArt: false + NewsCreateCat: false + NewsDeleteCat: false + NewsCreateFldr: false + NewsDeleteFldr: false + SendPrivMsg: false +`, + }, + { + name: "access bitmap with some permissions", + bits: func() AccessBitmap { + var bits AccessBitmap + bits.Set(AccessDownloadFile) + bits.Set(AccessUploadFile) + bits.Set(AccessCreateFolder) + bits.Set(AccessReadChat) + return bits + }(), + expected: ` DownloadFile: true + DownloadFolder: false + UploadFile: true + UploadFolder: false + DeleteFile: false + RenameFile: false + MoveFile: false + CreateFolder: true + DeleteFolder: false + RenameFolder: false + MoveFolder: false + ReadChat: true + SendChat: false + OpenChat: false + CloseChat: false + ShowInList: false + CreateUser: false + DeleteUser: false + OpenUser: false + ModifyUser: false + ChangeOwnPass: false + NewsReadArt: false + NewsPostArt: false + DisconnectUser: false + CannotBeDisconnected: false + GetClientInfo: false + UploadAnywhere: false + AnyName: false + NoAgreement: false + SetFileComment: false + SetFolderComment: false + ViewDropBoxes: false + MakeAlias: false + Broadcast: false + NewsDeleteArt: false + NewsCreateCat: false + NewsDeleteCat: false + NewsCreateFldr: false + NewsDeleteFldr: false + SendPrivMsg: false +`, + }, + { + name: "access bitmap with all permissions", + bits: func() AccessBitmap { + var bits AccessBitmap + bits.Set(AccessDeleteFile) + bits.Set(AccessUploadFile) + bits.Set(AccessDownloadFile) + bits.Set(AccessRenameFile) + bits.Set(AccessMoveFile) + bits.Set(AccessCreateFolder) + bits.Set(AccessDeleteFolder) + bits.Set(AccessRenameFolder) + bits.Set(AccessMoveFolder) + bits.Set(AccessReadChat) + bits.Set(AccessSendChat) + bits.Set(AccessOpenChat) + bits.Set(AccessCloseChat) + bits.Set(AccessShowInList) + bits.Set(AccessCreateUser) + bits.Set(AccessDeleteUser) + bits.Set(AccessOpenUser) + bits.Set(AccessModifyUser) + bits.Set(AccessChangeOwnPass) + bits.Set(AccessNewsReadArt) + bits.Set(AccessNewsPostArt) + bits.Set(AccessDisconUser) + bits.Set(AccessCannotBeDiscon) + bits.Set(AccessGetClientInfo) + bits.Set(AccessUploadAnywhere) + bits.Set(AccessAnyName) + bits.Set(AccessNoAgreement) + bits.Set(AccessSetFileComment) + bits.Set(AccessSetFolderComment) + bits.Set(AccessViewDropBoxes) + bits.Set(AccessMakeAlias) + bits.Set(AccessBroadcast) + bits.Set(AccessNewsDeleteArt) + bits.Set(AccessNewsCreateCat) + bits.Set(AccessNewsDeleteCat) + bits.Set(AccessNewsCreateFldr) + bits.Set(AccessNewsDeleteFldr) + bits.Set(AccessSendPrivMsg) + bits.Set(AccessUploadFolder) + bits.Set(AccessDownloadFolder) + return bits + }(), + expected: ` DownloadFile: true + DownloadFolder: true + UploadFile: true + UploadFolder: true + DeleteFile: true + RenameFile: true + MoveFile: true + CreateFolder: true + DeleteFolder: true + RenameFolder: true + MoveFolder: true + ReadChat: true + SendChat: true + OpenChat: true + CloseChat: true + ShowInList: true + CreateUser: true + DeleteUser: true + OpenUser: true + ModifyUser: true + ChangeOwnPass: true + NewsReadArt: true + NewsPostArt: true + DisconnectUser: true + CannotBeDisconnected: true + GetClientInfo: true + UploadAnywhere: true + AnyName: true + NoAgreement: true + SetFileComment: true + SetFolderComment: true + ViewDropBoxes: true + MakeAlias: true + Broadcast: true + NewsDeleteArt: true + NewsCreateCat: true + NewsDeleteCat: true + NewsCreateFldr: true + NewsDeleteFldr: true + SendPrivMsg: true +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test via YAML marshaling + yamlData, err := yaml.Marshal(struct { + Access AccessBitmap `yaml:"access"` + }{Access: tt.bits}) + + require.NoError(t, err) + + // Extract just the access portion + lines := strings.Split(string(yamlData), "\n") + var accessLines []string + inAccess := false + for _, line := range lines { + if strings.HasPrefix(line, "access:") { + inAccess = true + continue + } + if inAccess && strings.HasPrefix(line, " ") { + accessLines = append(accessLines, line) + } + } + + actualAccess := strings.Join(accessLines, "\n") + "\n" + assert.Equal(t, tt.expected, actualAccess) + }) + } +} -- cgit From b6bca2b7fe943ca4fd8f62a44bcfcc0c9f5a6de1 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Sat, 5 Jul 2025 15:19:15 -0700 Subject: Add comprehensive test coverage for news.go functions - Add table-driven tests for GetNewsArtListData, DataSize, NewsArtListData.Read, NewsArtList.Read, and newsPathScanner - Improve test coverage from 0% to 87.5%-100% for these functions - Add test for Field.DecodeNewsPath function - Fix NewsArtList.Read to return nil instead of io.EOF for proper io.Reader behavior - Add constants for NewsFlavorCount and improve code documentation --- hotline/field_test.go | 67 +++++++++++++ hotline/news.go | 12 +-- hotline/news_test.go | 271 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 344 insertions(+), 6 deletions(-) (limited to 'hotline') diff --git a/hotline/field_test.go b/hotline/field_test.go index 676098c..3440b0d 100644 --- a/hotline/field_test.go +++ b/hotline/field_test.go @@ -229,3 +229,70 @@ func TestField_DecodeInt(t *testing.T) { }) } } + +func TestField_DecodeNewsPath(t *testing.T) { + type fields struct { + Data []byte + } + tests := []struct { + name string + fields fields + want []string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "empty field data", + fields: fields{Data: []byte{}}, + want: []string{}, + wantErr: assert.NoError, + }, + { + name: "single path", + fields: fields{Data: []byte{ + 0x00, 0x01, // path count = 1 + 0x00, 0x00, 0x05, // 2 bytes unused + 1 byte length (5) + 0x48, 0x65, 0x6c, 0x6c, 0x6f, // "Hello" + }}, + want: []string{"Hello"}, + wantErr: assert.NoError, + }, + { + name: "multiple paths", + 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" + 0x00, 0x00, 0x05, // 2 bytes unused + 1 byte length (5) + 0x57, 0x6f, 0x72, 0x6c, 0x64, // "World" + }}, + want: []string{"Hello", "World"}, + wantErr: assert.NoError, + }, + { + name: "example from comments - nested categories", + fields: fields{Data: []byte{ + 0x00, 0x03, // path count = 3 + 0x00, 0x00, 0x10, // 2 bytes unused + 1 byte length (16) + 0x54, 0x6f, 0x70, 0x20, 0x4c, 0x65, 0x76, 0x65, 0x6c, 0x20, 0x42, 0x75, 0x6e, 0x64, 0x6c, 0x65, // "Top Level Bundle" + 0x00, 0x00, 0x13, // 2 bytes unused + 1 byte length (19) + 0x53, 0x65, 0x63, 0x6f, 0x6e, 0x64, 0x20, 0x4c, 0x65, 0x76, 0x65, 0x6c, 0x20, 0x42, 0x75, 0x6e, 0x64, 0x6c, 0x65, // "Second Level Bundle" + 0x00, 0x00, 0x0f, // 2 bytes unused + 1 byte length (15) + 0x4e, 0x65, 0x73, 0x74, 0x65, 0x64, 0x20, 0x43, 0x61, 0x74, 0x65, 0x67, 0x6f, 0x72, 0x79, // "Nested Category" + }}, + want: []string{"Top Level Bundle", "Second Level Bundle", "Nested Category"}, + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &Field{ + Data: tt.fields.Data, + } + got, err := f.DecodeNewsPath() + if !tt.wantErr(t, err, "DecodeNewsPath()") { + return + } + assert.Equalf(t, tt.want, got, "DecodeNewsPath()") + }) + } +} diff --git a/hotline/news.go b/hotline/news.go index c61a76f..1ddeebd 100644 --- a/hotline/news.go +++ b/hotline/news.go @@ -47,7 +47,7 @@ func (newscat *NewsCategoryListData15) GetNewsArtListData() (NewsArtListData, er for i, art := range newscat.Articles { id := make([]byte, 4) - binary.BigEndian.PutUint32(id, i) + binary.BigEndian.PutUint32(id, i) // The article's map key in the Articles map is its ID. newsArts = append(newsArts, NewsArtList{ ID: [4]byte(id), @@ -156,8 +156,8 @@ type NewsArtList struct { } var ( - NewsFlavorLen = []byte{0x0a} - NewsFlavor = []byte("text/plain") + NewsFlavor = []byte("text/plain") // NewsFlavor is always "text/plain" + NewsFlavorCount = []byte{0, 1} // NewsFlavorCount is always 1 ) func (nal *NewsArtList) Read(p []byte) (int, error) { @@ -166,12 +166,12 @@ func (nal *NewsArtList) Read(p []byte) (int, error) { nal.TimeStamp[:], nal.ParentID[:], nal.Flags[:], - []byte{0, 1}, // Flavor Count TODO: make this not hardcoded + NewsFlavorCount, []byte{uint8(len(nal.Title))}, nal.Title, []byte{uint8(len(nal.Poster))}, nal.Poster, - NewsFlavorLen, + []byte{uint8(len(NewsFlavor))}, NewsFlavor, nal.ArticleSize[:], ) @@ -183,7 +183,7 @@ func (nal *NewsArtList) Read(p []byte) (int, error) { n := copy(p, out[nal.readOffset:]) nal.readOffset += n - return n, io.EOF + return n, nil } type NewsFlavorList struct { diff --git a/hotline/news_test.go b/hotline/news_test.go index d1b043e..46df2e3 100644 --- a/hotline/news_test.go +++ b/hotline/news_test.go @@ -97,3 +97,274 @@ func TestNewsCategoryListData15_MarshalBinary(t *testing.T) { }) } } + +func TestNewsCategoryListData15_GetNewsArtListData(t *testing.T) { + tests := []struct { + name string + newscat NewsCategoryListData15 + wantData NewsArtListData + wantErr bool + }{ + { + name: "empty articles", + newscat: NewsCategoryListData15{ + Articles: map[uint32]*NewsArtData{}, + }, + wantData: NewsArtListData{ + Count: 0, + Name: []byte{}, + Description: []byte{}, + NewsArtList: []byte{}, + }, + wantErr: false, + }, + { + name: "single article", + newscat: NewsCategoryListData15{ + Articles: map[uint32]*NewsArtData{ + 1: { + Title: "Test Title", + Poster: "Test Poster", + Date: [8]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}, + Data: "Test content", + }, + }, + }, + wantData: NewsArtListData{ + Count: 1, + Name: []byte{}, + Description: []byte{}, + }, + wantErr: false, + }, + { + name: "multiple articles", + newscat: NewsCategoryListData15{ + Articles: map[uint32]*NewsArtData{ + 2: { + Title: "Second Article", + Poster: "Author2", + Date: [8]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08}, + Data: "Second content", + }, + 1: { + Title: "First Article", + Poster: "Author1", + Date: [8]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}, + Data: "First content", + }, + }, + }, + wantData: NewsArtListData{ + Count: 2, + Name: []byte{}, + Description: []byte{}, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotData, err := tt.newscat.GetNewsArtListData() + if (err != nil) != tt.wantErr { + t.Errorf("GetNewsArtListData() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.wantData.Count, gotData.Count) + assert.Equal(t, tt.wantData.Name, gotData.Name) + assert.Equal(t, tt.wantData.Description, gotData.Description) + if tt.wantData.Count > 0 { + assert.NotEmpty(t, gotData.NewsArtList) + } + }) + } +} + +func TestNewsArtData_DataSize(t *testing.T) { + tests := []struct { + name string + art NewsArtData + want [2]byte + }{ + { + name: "empty data", + art: NewsArtData{Data: ""}, + want: [2]byte{0x00, 0x00}, + }, + { + name: "short data", + art: NewsArtData{Data: "hello"}, + want: [2]byte{0x00, 0x05}, + }, + { + name: "longer data", + art: NewsArtData{Data: "This is a longer test message with more content"}, + want: [2]byte{0x00, 0x2F}, // 47 bytes + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.art.DataSize() + assert.Equal(t, tt.want, got) + }) + } +} + +func TestNewsArtListData_Read(t *testing.T) { + tests := []struct { + name string + nald NewsArtListData + bufferSize int + wantN int + wantErr bool + }{ + { + name: "empty data", + nald: NewsArtListData{ + ID: [4]byte{0x00, 0x01, 0x02, 0x03}, + Name: []byte("test"), + Description: []byte("desc"), + NewsArtList: []byte{}, + Count: 0, + }, + bufferSize: 100, + wantN: 18, // 4 (ID) + 4 (count) + 1 (name len) + 4 (name) + 1 (desc len) + 4 (desc) + 0 (news art list) + wantErr: false, + }, + { + name: "with article list", + nald: NewsArtListData{ + ID: [4]byte{0x00, 0x01, 0x02, 0x03}, + Name: []byte("test"), + Description: []byte("desc"), + NewsArtList: []byte{0x01, 0x02, 0x03}, + Count: 1, + }, + bufferSize: 100, + wantN: 21, // 4 (ID) + 4 (count) + 1 (name len) + 4 (name) + 1 (desc len) + 4 (desc) + 3 (news art list) + wantErr: false, + }, + { + name: "small buffer", + nald: NewsArtListData{ + ID: [4]byte{0x00, 0x01, 0x02, 0x03}, + Name: []byte("test"), + Description: []byte("desc"), + Count: 0, + }, + bufferSize: 5, + wantN: 5, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := make([]byte, tt.bufferSize) + gotN, err := tt.nald.Read(p) + if (err != nil) != tt.wantErr { + t.Errorf("Read() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.wantN, gotN) + }) + } +} + +func TestNewsArtList_Read(t *testing.T) { + tests := []struct { + name string + nal NewsArtList + bufferSize int + wantN int + wantErr bool + }{ + { + name: "basic article", + nal: NewsArtList{ + ID: [4]byte{0x00, 0x01, 0x02, 0x03}, + TimeStamp: [8]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}, + ParentID: [4]byte{0x00, 0x00, 0x00, 0x00}, + Flags: [4]byte{0x00, 0x00, 0x00, 0x00}, + Title: []byte("Test Title"), + Poster: []byte("Test Poster"), + ArticleSize: [2]byte{0x00, 0x0A}, + }, + bufferSize: 100, + wantN: 58, // 4 (ID) + 8 (timestamp) + 4 (parent) + 4 (flags) + 2 (flavor count) + 1 (title len) + 10 (title) + 1 (poster len) + 11 (poster) + 1 (flavor len) + 10 (flavor) + 2 (article size) + wantErr: false, + }, + { + name: "small buffer", + nal: NewsArtList{ + ID: [4]byte{0x00, 0x01, 0x02, 0x03}, + TimeStamp: [8]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}, + Title: []byte("Test"), + Poster: []byte("Author"), + }, + bufferSize: 10, + wantN: 10, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := make([]byte, tt.bufferSize) + gotN, err := tt.nal.Read(p) + if (err != nil) != tt.wantErr { + t.Errorf("Read() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.wantN, gotN) + }) + } +} + +func TestNewsPathScanner(t *testing.T) { + tests := []struct { + name string + data []byte + wantAdvance int + wantToken []byte + wantErr bool + }{ + { + name: "insufficient data", + data: []byte{0x00, 0x01}, + wantAdvance: 0, + wantToken: nil, + wantErr: false, + }, + { + name: "valid token", + data: []byte{0x00, 0x01, 0x04, 0x74, 0x65, 0x73, 0x74}, // length 4, "test" + wantAdvance: 7, + wantToken: []byte("test"), + wantErr: false, + }, + { + name: "zero length token", + data: []byte{0x00, 0x01, 0x00}, + wantAdvance: 3, + wantToken: []byte{}, + wantErr: false, + }, + { + name: "single character token", + data: []byte{0x00, 0x01, 0x01, 0x61}, // length 1, "a" + wantAdvance: 4, + wantToken: []byte("a"), + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotAdvance, gotToken, err := newsPathScanner(tt.data, false) + if (err != nil) != tt.wantErr { + t.Errorf("newsPathScanner() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.wantAdvance, gotAdvance) + assert.Equal(t, tt.wantToken, gotToken) + }) + } +} -- cgit From cab4e2192d5b39e51933ced3dd569de9fc182a04 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Sat, 5 Jul 2025 16:02:33 -0700 Subject: Fix string splitting bug and add comprehensive tracker registration tests - Fix critical bug in tracker address parsing: strings.Split(t, ":") instead of strings.Split(":", t) - Add dependency injection for TrackerRegistrar to improve testability - Extract registerWithAllTrackers() helper function to eliminate code duplication - Add comprehensive test suite covering: * Unit tests for parseTrackerPassword with edge cases and special characters * Integration tests for registerWithTrackers with mocking * Context cancellation and graceful shutdown testing * Error handling for network failures and malformed addresses * Edge cases like zero ports, long names, and empty configurations - Improve password parsing to handle colons in passwords correctly - Add MockTrackerRegistrar for isolated testing without network dependencies The string splitting bug would have prevented tracker registration from working with password-protected trackers. All tests pass and verify the fix works correctly. --- hotline/server.go | 124 +++++++---- hotline/server_test.go | 583 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 659 insertions(+), 48 deletions(-) (limited to 'hotline') diff --git a/hotline/server.go b/hotline/server.go index 19c7acc..0395d7b 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -68,6 +68,9 @@ type Server struct { MessageBoard io.ReadWriteSeeker Redis *redis.Client + + // TrackerRegistrar handles tracker registration (injectable for testing) + TrackerRegistrar TrackerRegistrar } type Option = func(s *Server) @@ -98,19 +101,27 @@ func WithInterface(netInterface string) func(s *Server) { } } +// WithTrackerRegistrar optionally sets a custom tracker registrar (useful for testing). +func WithTrackerRegistrar(registrar TrackerRegistrar) func(s *Server) { + return func(s *Server) { + s.TrackerRegistrar = registrar + } +} + type ServerConfig struct { } func NewServer(options ...Option) (*Server, error) { server := Server{ - handlers: make(map[TranType]HandlerFunc), - outbox: make(chan Transaction), - rateLimiters: make(map[string]*rate.Limiter), - FS: &OSFileStore{}, - ChatMgr: NewMemChatManager(), - ClientMgr: NewMemClientMgr(), - FileTransferMgr: NewMemFileTransferMgr(), - Stats: NewStats(), + handlers: make(map[TranType]HandlerFunc), + outbox: make(chan Transaction), + rateLimiters: make(map[string]*rate.Limiter), + FS: &OSFileStore{}, + ChatMgr: NewMemChatManager(), + ClientMgr: NewMemClientMgr(), + FileTransferMgr: NewMemFileTransferMgr(), + Stats: NewStats(), + TrackerRegistrar: NewRealTrackerRegistrar(), } for _, opt := range options { @@ -266,6 +277,61 @@ func (s *Server) Serve(ctx context.Context, ln net.Listener) error { // time in seconds between tracker re-registration const trackerUpdateFrequency = 300 +// TrackerRegistrar interface for tracker registration operations +type TrackerRegistrar interface { + Register(tracker string, registration *TrackerRegistration) error +} + +// RealTrackerRegistrar implements TrackerRegistrar using the real network operations +type RealTrackerRegistrar struct { + dialer Dialer +} + +func NewRealTrackerRegistrar() *RealTrackerRegistrar { + return &RealTrackerRegistrar{ + dialer: &RealDialer{}, + } +} + +func (r *RealTrackerRegistrar) Register(tracker string, registration *TrackerRegistration) error { + return register(r.dialer, tracker, registration) +} + +// parseTrackerPassword extracts the password from a tracker address in format "host:port:password" +// Returns empty string if no password is present or if the format is invalid +// For addresses with more than 3 parts (like passwords containing colons), everything after the second colon is treated as the password +func parseTrackerPassword(trackerAddr string) string { + splitAddr := strings.Split(trackerAddr, ":") + if len(splitAddr) >= 3 { + // Join everything from the third part onwards (index 2+) to handle passwords with colons + return strings.Join(splitAddr[2:], ":") + } + return "" +} + +// registerWithAllTrackers performs tracker registration for all configured trackers +func (s *Server) registerWithAllTrackers() { + if !s.Config.EnableTrackerRegistration { + return + } + + for _, t := range s.Config.Trackers { + tr := &TrackerRegistration{ + UserCount: len(s.ClientMgr.List()), + PassID: s.TrackerPassID, + Name: s.Config.Name, + Description: s.Config.Description, + } + binary.BigEndian.PutUint16(tr.Port[:], uint16(s.Port)) + + tr.Password = parseTrackerPassword(t) + + if err := s.TrackerRegistrar.Register(t, tr); err != nil { + s.Logger.Error(fmt.Sprintf("Unable to register with tracker %v", t), "error", err) + } + } +} + // registerWithTrackers runs every trackerUpdateFrequency seconds to update the server's tracker entry on all configured // trackers. func (s *Server) registerWithTrackers(ctx context.Context) { @@ -274,26 +340,7 @@ func (s *Server) registerWithTrackers(ctx context.Context) { } // Do the first registration immediately - if s.Config.EnableTrackerRegistration { - for _, t := range s.Config.Trackers { - tr := &TrackerRegistration{ - UserCount: len(s.ClientMgr.List()), - PassID: s.TrackerPassID, - Name: s.Config.Name, - Description: s.Config.Description, - } - binary.BigEndian.PutUint16(tr.Port[:], uint16(s.Port)) - - splitAddr := strings.Split(":", t) - if len(splitAddr) == 3 { - tr.Password = splitAddr[2] - } - - if err := register(&RealDialer{}, t, tr); err != nil { - s.Logger.Error(fmt.Sprintf("Unable to register with tracker %v", t), "error", err) - } - } - } + s.registerWithAllTrackers() ticker := time.NewTicker(trackerUpdateFrequency * time.Second) defer ticker.Stop() @@ -303,26 +350,7 @@ func (s *Server) registerWithTrackers(ctx context.Context) { case <-ctx.Done(): return case <-ticker.C: - if s.Config.EnableTrackerRegistration { - for _, t := range s.Config.Trackers { - tr := &TrackerRegistration{ - UserCount: len(s.ClientMgr.List()), - PassID: s.TrackerPassID, - Name: s.Config.Name, - Description: s.Config.Description, - } - binary.BigEndian.PutUint16(tr.Port[:], uint16(s.Port)) - - splitAddr := strings.Split(":", t) - if len(splitAddr) == 3 { - tr.Password = splitAddr[2] - } - - if err := register(&RealDialer{}, t, tr); err != nil { - s.Logger.Error(fmt.Sprintf("Unable to register with tracker %v", t), "error", err) - } - } - } + s.registerWithAllTrackers() } } } diff --git a/hotline/server_test.go b/hotline/server_test.go index d0e2372..574aae7 100644 --- a/hotline/server_test.go +++ b/hotline/server_test.go @@ -3,12 +3,15 @@ package hotline import ( "bytes" "context" + "encoding/binary" "fmt" "github.com/stretchr/testify/assert" "io" "log/slog" "os" + "strings" "testing" + "time" ) type mockReadWriter struct { @@ -181,3 +184,583 @@ func TestServer_handleFileTransfer(t *testing.T) { }) } } + +func TestParseTrackerPassword(t *testing.T) { + tests := []struct { + name string + trackerAddr string + wantPassword string + }{ + { + name: "tracker address with password", + trackerAddr: "tracker.example.com:5500:mypassword", + wantPassword: "mypassword", + }, + { + name: "tracker address without password", + trackerAddr: "tracker.example.com:5500", + wantPassword: "", + }, + { + name: "tracker address with empty password", + trackerAddr: "tracker.example.com:5500:", + wantPassword: "", + }, + { + name: "tracker address with password containing special characters", + trackerAddr: "tracker.example.com:5500:pass@word#123", + wantPassword: "pass@word#123", + }, + { + name: "tracker address with password containing colons", + trackerAddr: "tracker.example.com:5500:pass:word:123", + wantPassword: "pass:word:123", + }, + { + name: "IPv4 address with password", + trackerAddr: "192.168.1.100:5500:secret", + wantPassword: "secret", + }, + { + name: "IPv4 address without password", + trackerAddr: "192.168.1.100:5500", + wantPassword: "", + }, + { + name: "malformed address - no port", + trackerAddr: "tracker.example.com", + wantPassword: "", + }, + { + name: "malformed address - empty string", + trackerAddr: "", + wantPassword: "", + }, + { + name: "malformed address - only colons", + trackerAddr: ":::", + wantPassword: ":", + }, + { + name: "IPv6 address handling (edge case - not properly supported)", + trackerAddr: "[::1]:5500:password", + wantPassword: "1]:5500:password", // IPv6 addresses aren't properly handled by simple colon splitting + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseTrackerPassword(tt.trackerAddr) + assert.Equal(t, tt.wantPassword, got) + }) + } +} + +// MockTrackerRegistrar is a mock implementation of TrackerRegistrar for testing +type MockTrackerRegistrar struct { + RegisterCalls []RegisterCall + RegisterFunc func(tracker string, registration *TrackerRegistration) error +} + +type RegisterCall struct { + Tracker string + Registration *TrackerRegistration +} + +func (m *MockTrackerRegistrar) Register(tracker string, registration *TrackerRegistration) error { + // Record the call + m.RegisterCalls = append(m.RegisterCalls, RegisterCall{ + Tracker: tracker, + Registration: registration, + }) + + // Use custom function if provided, otherwise return nil (success) + if m.RegisterFunc != nil { + return m.RegisterFunc(tracker, registration) + } + return nil +} + +func (m *MockTrackerRegistrar) Reset() { + m.RegisterCalls = nil + m.RegisterFunc = nil +} + +func TestServer_registerWithTrackers(t *testing.T) { + tests := []struct { + name string + config Config + wantImmediateRegistration bool + wantTrackerCalls []string + mockRegisterFunc func(tracker string, registration *TrackerRegistration) error + expectError bool + }{ + { + name: "disabled tracker registration", + config: Config{ + EnableTrackerRegistration: false, + Trackers: []string{"tracker1.example.com:5500", "tracker2.example.com:5500:password"}, + }, + wantImmediateRegistration: false, + wantTrackerCalls: []string{}, + }, + { + name: "enabled tracker registration with multiple trackers", + config: Config{ + EnableTrackerRegistration: true, + Trackers: []string{"tracker1.example.com:5500", "tracker2.example.com:5500:password"}, + Name: "Test Server", + Description: "Test Description", + }, + wantImmediateRegistration: true, + wantTrackerCalls: []string{"tracker1.example.com:5500", "tracker2.example.com:5500:password"}, + }, + { + name: "enabled tracker registration with empty tracker list", + config: Config{ + EnableTrackerRegistration: true, + Trackers: []string{}, + Name: "Test Server", + Description: "Test Description", + }, + wantImmediateRegistration: true, + wantTrackerCalls: []string{}, + }, + { + name: "tracker registration with network errors", + config: Config{ + EnableTrackerRegistration: true, + Trackers: []string{"tracker1.example.com:5500"}, + Name: "Test Server", + Description: "Test Description", + }, + wantImmediateRegistration: true, + wantTrackerCalls: []string{"tracker1.example.com:5500"}, + mockRegisterFunc: func(tracker string, registration *TrackerRegistration) error { + return assert.AnError // Simulate network error + }, + expectError: false, // Errors are logged but don't stop the function + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create mock registrar + mockRegistrar := &MockTrackerRegistrar{ + RegisterFunc: tt.mockRegisterFunc, + } + + // Create server with mock registrar + server, err := NewServer( + WithConfig(tt.config), + WithLogger(NewTestLogger()), + WithTrackerRegistrar(mockRegistrar), + ) + assert.NoError(t, err) + + // Create a context that we can cancel + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start the registerWithTrackers function in a goroutine + done := make(chan struct{}) + go func() { + defer close(done) + server.registerWithTrackers(ctx) + }() + + // Give it a moment to do the immediate registration + time.Sleep(100 * time.Millisecond) + + // Cancel the context to stop the goroutine + cancel() + + // Wait for the goroutine to finish (should be quick after cancellation) + select { + case <-done: + // Success + case <-time.After(1 * time.Second): + t.Fatal("registerWithTrackers did not exit after context cancellation") + } + + // Verify the calls made to the mock registrar + assert.Len(t, mockRegistrar.RegisterCalls, len(tt.wantTrackerCalls)) + + for i, expectedTracker := range tt.wantTrackerCalls { + if i < len(mockRegistrar.RegisterCalls) { + call := mockRegistrar.RegisterCalls[i] + assert.Equal(t, expectedTracker, call.Tracker) + assert.Equal(t, tt.config.Name, call.Registration.Name) + assert.Equal(t, tt.config.Description, call.Registration.Description) + assert.Equal(t, parseTrackerPassword(expectedTracker), call.Registration.Password) + } + } + }) + } +} + +func TestServer_registerWithTrackers_ContextCancellation(t *testing.T) { + tests := []struct { + name string + cancelAfter time.Duration + expectedCalls int // Number of expected registration calls before cancellation + trackerCount int + }{ + { + name: "immediate cancellation", + cancelAfter: 10 * time.Millisecond, + expectedCalls: 2, // Should complete immediate registration + trackerCount: 2, + }, + { + name: "cancellation after first ticker", + cancelAfter: 100 * time.Millisecond, + expectedCalls: 2, // Should only do immediate registration within 100ms + trackerCount: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockRegistrar := &MockTrackerRegistrar{} + config := Config{ + EnableTrackerRegistration: true, + Trackers: make([]string, tt.trackerCount), + Name: "Test Server", + Description: "Test Description", + } + + // Fill trackers array + for i := 0; i < tt.trackerCount; i++ { + config.Trackers[i] = fmt.Sprintf("tracker%d.example.com:5500", i+1) + } + + server, err := NewServer( + WithConfig(config), + WithLogger(NewTestLogger()), + WithTrackerRegistrar(mockRegistrar), + ) + assert.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + done := make(chan struct{}) + go func() { + defer close(done) + server.registerWithTrackers(ctx) + }() + + // Wait for the specified time then cancel + time.Sleep(tt.cancelAfter) + cancel() + + // Wait for graceful shutdown + select { + case <-done: + // Success + case <-time.After(1 * time.Second): + t.Fatal("registerWithTrackers did not exit after context cancellation") + } + + // Verify that the function respects context cancellation + assert.Equal(t, tt.expectedCalls, len(mockRegistrar.RegisterCalls)) + }) + } +} + +func TestServer_registerWithTrackers_PeriodicRegistration(t *testing.T) { + t.Skip("Skipping timing-sensitive test - would take 5+ minutes to run reliably") + + // This test would verify that periodic re-registration happens every trackerUpdateFrequency seconds + // but it's impractical to run in normal test suites due to the 300-second interval + + mockRegistrar := &MockTrackerRegistrar{} + config := Config{ + EnableTrackerRegistration: true, + Trackers: []string{"tracker1.example.com:5500"}, + Name: "Test Server", + Description: "Test Description", + } + + server, err := NewServer( + WithConfig(config), + WithLogger(NewTestLogger()), + WithTrackerRegistrar(mockRegistrar), + ) + assert.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + done := make(chan struct{}) + go func() { + defer close(done) + server.registerWithTrackers(ctx) + }() + + // Wait for timeout or completion + <-ctx.Done() + + // Should have done immediate registration only (1 call) in 10 seconds + // since trackerUpdateFrequency is 300 seconds + assert.Equal(t, 1, len(mockRegistrar.RegisterCalls)) +} + +func TestServer_registerWithTrackers_ErrorHandling(t *testing.T) { + tests := []struct { + name string + trackers []string + mockRegisterFunc func(tracker string, registration *TrackerRegistration) error + expectPanic bool + }{ + { + name: "handles network errors gracefully", + trackers: []string{"tracker1.example.com:5500", "tracker2.example.com:5500:password"}, + mockRegisterFunc: func(tracker string, registration *TrackerRegistration) error { + if tracker == "tracker1.example.com:5500" { + return fmt.Errorf("network error: connection refused") + } + return nil // Second tracker succeeds + }, + expectPanic: false, + }, + { + name: "handles all trackers failing", + trackers: []string{"tracker1.example.com:5500", "tracker2.example.com:5500"}, + mockRegisterFunc: func(tracker string, registration *TrackerRegistration) error { + return fmt.Errorf("network error") + }, + expectPanic: false, + }, + { + name: "handles empty tracker addresses", + trackers: []string{"", "valid.tracker.com:5500", ""}, + mockRegisterFunc: func(tracker string, registration *TrackerRegistration) error { + if tracker == "" { + return fmt.Errorf("invalid tracker address") + } + return nil + }, + expectPanic: false, + }, + { + name: "handles malformed tracker addresses", + trackers: []string{"invalid-address", "another:invalid", "valid.tracker.com:5500:password"}, + mockRegisterFunc: func(tracker string, registration *TrackerRegistration) error { + return nil // Accept all for this test + }, + expectPanic: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockRegistrar := &MockTrackerRegistrar{ + RegisterFunc: tt.mockRegisterFunc, + } + + config := Config{ + EnableTrackerRegistration: true, + Trackers: tt.trackers, + Name: "Test Server", + Description: "Test Description", + } + + server, err := NewServer( + WithConfig(config), + WithLogger(NewTestLogger()), + WithTrackerRegistrar(mockRegistrar), + ) + assert.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + if tt.expectPanic { + assert.Panics(t, func() { + server.registerWithTrackers(ctx) + }) + return + } + + done := make(chan struct{}) + go func() { + defer close(done) + server.registerWithTrackers(ctx) + }() + + // Give it time to process + time.Sleep(50 * time.Millisecond) + cancel() + + select { + case <-done: + // Success - function completed without panicking + case <-time.After(1 * time.Second): + t.Fatal("registerWithTrackers did not exit after context cancellation") + } + + // Verify all trackers were attempted + assert.Equal(t, len(tt.trackers), len(mockRegistrar.RegisterCalls)) + }) + } +} + +func TestServer_registerWithTrackers_EdgeCases(t *testing.T) { + tests := []struct { + name string + config Config + expectedCalls int + validateResult func(t *testing.T, calls []RegisterCall) + }{ + { + name: "server with zero port", + config: Config{ + EnableTrackerRegistration: true, + Trackers: []string{"tracker.example.com:5500"}, + Name: "Test Server", + Description: "Test Description", + }, + expectedCalls: 1, + validateResult: func(t *testing.T, calls []RegisterCall) { + assert.Equal(t, uint16(0), binary.BigEndian.Uint16(calls[0].Registration.Port[:])) + }, + }, + { + name: "server with very long name and description", + config: Config{ + EnableTrackerRegistration: true, + Trackers: []string{"tracker.example.com:5500"}, + Name: strings.Repeat("A", 255), // Max uint8 length + Description: strings.Repeat("B", 255), + }, + expectedCalls: 1, + validateResult: func(t *testing.T, calls []RegisterCall) { + assert.Equal(t, strings.Repeat("A", 255), calls[0].Registration.Name) + assert.Equal(t, strings.Repeat("B", 255), calls[0].Registration.Description) + }, + }, + { + name: "empty server name and description", + config: Config{ + EnableTrackerRegistration: true, + Trackers: []string{"tracker.example.com:5500"}, + Name: "", + Description: "", + }, + expectedCalls: 1, + validateResult: func(t *testing.T, calls []RegisterCall) { + assert.Equal(t, "", calls[0].Registration.Name) + assert.Equal(t, "", calls[0].Registration.Description) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockRegistrar := &MockTrackerRegistrar{} + + server, err := NewServer( + WithConfig(tt.config), + WithLogger(NewTestLogger()), + WithTrackerRegistrar(mockRegistrar), + ) + assert.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + done := make(chan struct{}) + go func() { + defer close(done) + server.registerWithTrackers(ctx) + }() + + time.Sleep(50 * time.Millisecond) + cancel() + + select { + case <-done: + // Success + case <-time.After(1 * time.Second): + t.Fatal("registerWithTrackers did not exit after context cancellation") + } + + assert.Equal(t, tt.expectedCalls, len(mockRegistrar.RegisterCalls)) + if tt.validateResult != nil { + tt.validateResult(t, mockRegistrar.RegisterCalls) + } + }) + } +} + +func TestServer_registerWithAllTrackers(t *testing.T) { + tests := []struct { + name string + config Config + expectRegistrationAttempt bool + expectedTrackerCalls []string + }{ + { + name: "disabled tracker registration", + config: Config{ + EnableTrackerRegistration: false, + Trackers: []string{"tracker1.example.com:5500"}, + }, + expectRegistrationAttempt: false, + expectedTrackerCalls: []string{}, + }, + { + name: "enabled tracker registration with multiple trackers", + config: Config{ + EnableTrackerRegistration: true, + Trackers: []string{"tracker1.example.com:5500", "tracker2.example.com:5500:password"}, + Name: "Test Server", + Description: "Test Description", + }, + expectRegistrationAttempt: true, + expectedTrackerCalls: []string{"tracker1.example.com:5500", "tracker2.example.com:5500:password"}, + }, + { + name: "enabled tracker registration with empty tracker list", + config: Config{ + EnableTrackerRegistration: true, + Trackers: []string{}, + Name: "Test Server", + Description: "Test Description", + }, + expectRegistrationAttempt: true, + expectedTrackerCalls: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockRegistrar := &MockTrackerRegistrar{} + + server, err := NewServer( + WithConfig(tt.config), + WithLogger(NewTestLogger()), + WithTrackerRegistrar(mockRegistrar), + ) + assert.NoError(t, err) + + // Call the extracted function directly + server.registerWithAllTrackers() + + // Verify the expected number of calls + assert.Equal(t, len(tt.expectedTrackerCalls), len(mockRegistrar.RegisterCalls)) + + // Verify each call + for i, expectedTracker := range tt.expectedTrackerCalls { + if i < len(mockRegistrar.RegisterCalls) { + call := mockRegistrar.RegisterCalls[i] + assert.Equal(t, expectedTracker, call.Tracker) + assert.Equal(t, tt.config.Name, call.Registration.Name) + assert.Equal(t, tt.config.Description, call.Registration.Description) + assert.Equal(t, parseTrackerPassword(expectedTracker), call.Registration.Password) + } + } + }) + } +} -- cgit From e61b3160bc8936b7ffb0af5c89b13742a7081826 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Sun, 6 Jul 2025 16:02:53 -0700 Subject: Add ForkType type alias for improved type safety - Replace [4]byte with ForkType in ForkInfoList struct - Update constants ForkTypeDATA and ForkTypeMACR to use ForkType - Add String() method to ForkType for better debugging - Improve consistency with existing type patterns (FieldType, TranType) - Clean up unused code and improve documentation --- hotline/file_resume_data.go | 57 +++++++++++++++++++-------------------------- hotline/file_transfer.go | 27 +-------------------- hotline/file_wrapper.go | 2 +- 3 files changed, 26 insertions(+), 60 deletions(-) (limited to 'hotline') diff --git a/hotline/file_resume_data.go b/hotline/file_resume_data.go index 5dfd2d8..80f5fe5 100644 --- a/hotline/file_resume_data.go +++ b/hotline/file_resume_data.go @@ -3,38 +3,54 @@ package hotline import ( "bytes" "encoding/binary" + "fmt" ) // FileResumeData is sent when a client or server would like to resume a transfer from an offset type FileResumeData struct { Format [4]byte // "RFLT" Version [2]byte // Always 1 - RSVD [34]byte // Unused + RSVD [34]byte // Present in the Hotline protocol docs, but unused. Left here for documentation purposes. ForkCount [2]byte // Length of ForkInfoList. Either 2 or 3 depending on whether file has a resource fork ForkInfoList []ForkInfoList +} + +// ForkType represents a 4-byte fork type identifier +type ForkType [4]byte - //readOffset int // TODO +// String returns a string representation of the fork type +func (ft ForkType) String() string { + return fmt.Sprintf("%c%c%c%c", ft[0], ft[1], ft[2], ft[3]) } type ForkInfoList struct { - Fork [4]byte // "DATA" or "MACR" - DataSize [4]byte // offset from which to resume the transfer of data - RSVDA [4]byte // Unused - RSVDB [4]byte // Unused + Fork ForkType // "DATA" or "MACR" + DataSize [4]byte // offset from which to resume the transfer of data + RSVDA [4]byte // Present in the Hotline protocol docs, but unused. Left here for documentation purposes. + RSVDB [4]byte // Present in the Hotline protocol docs, but unused. Left here for documentation purposes. } +var ( + ForkTypeDATA = ForkType{0x44, 0x41, 0x54, 0x41} // DATA: Data fork + ForkTypeMACR = ForkType{0x4d, 0x41, 0x43, 0x52} // MACR: Mac resource fork +) + func NewForkInfoList(b []byte) *ForkInfoList { return &ForkInfoList{ - Fork: [4]byte{0x44, 0x41, 0x54, 0x41}, + Fork: ForkTypeDATA, DataSize: [4]byte{b[0], b[1], b[2], b[3]}, RSVDA: [4]byte{}, RSVDB: [4]byte{}, } } +var ( + FormatRFLT = [4]byte{0x52, 0x46, 0x4C, 0x54} // File resume format: "RFLT" (?) +) + func NewFileResumeData(list []ForkInfoList) *FileResumeData { return &FileResumeData{ - Format: [4]byte{0x52, 0x46, 0x4C, 0x54}, // RFLT + Format: FormatRFLT, Version: [2]byte{0, 1}, RSVD: [34]byte{}, ForkCount: [2]byte{0, uint8(len(list))}, @@ -42,31 +58,6 @@ func NewFileResumeData(list []ForkInfoList) *FileResumeData { } } -// -//func (frd *FileResumeData) Read(p []byte) (int, error) { -// buf := slices.Concat( -// frd.Format[:], -// frd.Version[:], -// frd.RSVD[:], -// frd.ForkCount[:], -// ) -// for _, fil := range frd.ForkInfoList { -// buf = append(buf, fil...) -// _ = binary.Write(&buf, binary.LittleEndian, fil) -// } -// -// var buf bytes.Buffer -// _ = binary.Write(&buf, binary.LittleEndian, frd.Format) -// _ = binary.Write(&buf, binary.LittleEndian, frd.Version) -// _ = binary.Write(&buf, binary.LittleEndian, frd.RSVD) -// _ = binary.Write(&buf, binary.LittleEndian, frd.ForkCount) -// for _, fil := range frd.ForkInfoList { -// _ = binary.Write(&buf, binary.LittleEndian, fil) -// } -// -// return buf.Bytes(), nil -//} - func (frd *FileResumeData) BinaryMarshal() ([]byte, error) { var buf bytes.Buffer _ = binary.Write(&buf, binary.LittleEndian, frd.Format) diff --git a/hotline/file_transfer.go b/hotline/file_transfer.go index 701259a..521b963 100644 --- a/hotline/file_transfer.go +++ b/hotline/file_transfer.go @@ -172,23 +172,6 @@ type folderUpload struct { FileNamePath []byte } -//func (fu *folderUpload) Write(p []byte) (int, error) { -// if len(p) < 7 { -// return 0, errors.New("buflen too short") -// } -// copy(fu.DataSize[:], p[0:2]) -// copy(fu.IsFolder[:], p[2:4]) -// copy(fu.PathItemCount[:], p[4:6]) -// -// fu.FileNamePath = make([]byte, binary.BigEndian.Uint16(fu.DataSize[:])-4) // -4 to subtract the path separator bytes TODO: wat -// n, err := io.ReadFull(rwc, fu.FileNamePath) -// if err != nil { -// return 0, err -// } -// -// return n + 6, nil -//} - // pathSegmentScanner implements bufio.SplitFunc for parsing path segments func pathSegmentScanner(data []byte, _ bool) (advance int, token []byte, err error) { if len(data) < 3 { @@ -269,12 +252,6 @@ func (fh *FileHeader) Read(p []byte) (int, error) { } func DownloadHandler(w io.Writer, fullPath string, fileTransfer *FileTransfer, fs FileStore, rLogger *slog.Logger, preserveForks bool) error { - //s.Stats.DownloadCounter += 1 - //s.Stats.DownloadsInProgress += 1 - //defer func() { - // s.Stats.DownloadsInProgress -= 1 - //}() - var dataOffset int64 if fileTransfer.FileResumeData != nil { dataOffset = int64(binary.BigEndian.Uint32(fileTransfer.FileResumeData.ForkInfoList[0].DataSize[:])) @@ -520,7 +497,6 @@ func DownloadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *Fil return fmt.Errorf("error opening file: %w", err) } - // wr := bufio.NewWriterSize(rwc, 1460) if _, err = io.Copy(rwc, io.TeeReader(file, fileTransfer.bytesSentCounter)); err != nil { return fmt.Errorf("error sending file: %w", err) } @@ -576,7 +552,6 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT //s.Stats.UploadCounter += 1 var fu folderUpload - // TODO: implement io.Writer on folderUpload and replace this if _, err := io.ReadFull(rwc, fu.DataSize[:]); err != nil { return err } @@ -586,7 +561,7 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT if _, err := io.ReadFull(rwc, fu.PathItemCount[:]); err != nil { return err } - fu.FileNamePath = make([]byte, binary.BigEndian.Uint16(fu.DataSize[:])-4) // -4 to subtract the path separator bytes TODO: wat + fu.FileNamePath = make([]byte, binary.BigEndian.Uint16(fu.DataSize[:])-4) // -4 to subtract the length of the DataSize and IsFolder fields if _, err := io.ReadFull(rwc, fu.FileNamePath); err != nil { return err } diff --git a/hotline/file_wrapper.go b/hotline/file_wrapper.go index b13e0dc..b6aff94 100644 --- a/hotline/file_wrapper.go +++ b/hotline/file_wrapper.go @@ -85,7 +85,7 @@ func (f *fileWrapper) rsrcForkSize() (s [4]byte) { func (f *fileWrapper) rsrcForkHeader() FlatFileForkHeader { return FlatFileForkHeader{ - ForkType: [4]byte{0x4D, 0x41, 0x43, 0x52}, // "MACR" + ForkType: ForkTypeMACR, DataSize: f.rsrcForkSize(), } } -- cgit From 16f0a8527a0b9b736afbd4e71e6aac55e31b60e8 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Tue, 18 Nov 2025 19:51:02 -0800 Subject: Add support for GIF banners with validation and improved error messages - Add custom validator for banner file extensions (.jpg, .jpeg, .gif) - Update HandleTranAgreed to dynamically detect banner type from file extension - Export FileTypeFromFilename function for use across packages - Add comprehensive test coverage for banner validation and type detection - Improve error messages to clearly communicate allowed file extensions --- hotline/config.go | 2 +- hotline/files.go | 8 +-- internal/mobius/config.go | 19 ++++++ internal/mobius/config_test.go | 95 ++++++++++++++++++++++++++++ internal/mobius/transaction_handlers.go | 3 +- internal/mobius/transaction_handlers_test.go | 50 +++++++++++++++ 6 files changed, 171 insertions(+), 6 deletions(-) create mode 100644 internal/mobius/config_test.go (limited to 'hotline') diff --git a/hotline/config.go b/hotline/config.go index a7bd81e..75d5c4d 100644 --- a/hotline/config.go +++ b/hotline/config.go @@ -3,7 +3,7 @@ package hotline type Config struct { Name string `yaml:"Name" validate:"required,max=50"` // Name used for Tracker registration Description string `yaml:"Description" validate:"required,max=200"` // Description used for Tracker registration - BannerFile string `yaml:"BannerFile"` // Path to Banner jpg + BannerFile string `yaml:"BannerFile" validate:"omitempty,bannerext"` // Path to Banner jpg or gif FileRoot string `yaml:"FileRoot" validate:"required"` // Path to Files EnableTrackerRegistration bool `yaml:"EnableTrackerRegistration"` // Toggle Tracker Registration Trackers []string `yaml:"Trackers" validate:"dive,hostname_port"` // List of trackers that the server should register with diff --git a/hotline/files.go b/hotline/files.go index a1db329..e9f7b4f 100644 --- a/hotline/files.go +++ b/hotline/files.go @@ -12,7 +12,7 @@ import ( "strings" ) -func fileTypeFromFilename(filename string) fileType { +func FileTypeFromFilename(filename string) fileType { fileExt := strings.ToLower(filepath.Ext(filename)) ft, ok := fileTypes[fileExt] if ok { @@ -26,7 +26,7 @@ func fileTypeFromInfo(info fs.FileInfo) (ft fileType, err error) { ft.CreatorCode = "n/a " ft.TypeCode = "fldr" } else { - ft = fileTypeFromFilename(info.Name()) + ft = FileTypeFromFilename(info.Name()) } return ft, nil @@ -87,8 +87,8 @@ func GetFileNameList(path string, ignoreList []string) (fields []Field, err erro copy(fnwi.Creator[:], fileCreator) } else { binary.BigEndian.PutUint32(fnwi.FileSize[:], uint32(rFile.Size())) - copy(fnwi.Type[:], fileTypeFromFilename(rFile.Name()).TypeCode) - copy(fnwi.Creator[:], fileTypeFromFilename(rFile.Name()).CreatorCode) + copy(fnwi.Type[:], FileTypeFromFilename(rFile.Name()).TypeCode) + copy(fnwi.Creator[:], FileTypeFromFilename(rFile.Name()).CreatorCode) } } else if file.IsDir() { dir, err := os.ReadDir(filepath.Join(path, file.Name())) diff --git a/internal/mobius/config.go b/internal/mobius/config.go index d04b14d..c798e54 100644 --- a/internal/mobius/config.go +++ b/internal/mobius/config.go @@ -7,6 +7,7 @@ import ( "gopkg.in/yaml.v3" "os" "path/filepath" + "strings" ) var ConfigSearchOrder = []string{ @@ -28,7 +29,25 @@ func LoadConfig(path string) (*hotline.Config, error) { } validate := validator.New() + if err = validate.RegisterValidation("bannerext", func(fl validator.FieldLevel) bool { + filename := fl.Field().String() + if filename == "" { + return true // Allow empty since BannerFile is optional + } + ext := strings.ToLower(filepath.Ext(filename)) + return ext == ".jpg" || ext == ".jpeg" || ext == ".gif" + }); err != nil { + return nil, fmt.Errorf("register validation: %v", err) + } if err = validate.Struct(config); err != nil { + // Check if this is a BannerFile validation error and provide a better message + if validationErrs, ok := err.(validator.ValidationErrors); ok { + for _, fieldErr := range validationErrs { + if fieldErr.Field() == "BannerFile" && fieldErr.Tag() == "bannerext" { + return nil, fmt.Errorf("BannerFile must have a .jpg, .jpeg, or .gif extension (got: %s)", config.BannerFile) + } + } + } return nil, fmt.Errorf("validate config: %v", err) } diff --git a/internal/mobius/config_test.go b/internal/mobius/config_test.go new file mode 100644 index 0000000..b76b16d --- /dev/null +++ b/internal/mobius/config_test.go @@ -0,0 +1,95 @@ +package mobius + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestLoadConfig_InvalidBannerFileExtension(t *testing.T) { + // Create a temporary directory for test files + tmpDir, err := os.MkdirTemp("", "mobius-config-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create a test config file with an invalid banner file extension + configContent := ` +Name: "Test Server" +Description: "Test Description" +BannerFile: "banner.png" +FileRoot: "files" +` + configPath := filepath.Join(tmpDir, "config.yaml") + if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { + t.Fatalf("Failed to write config file: %v", err) + } + + // Attempt to load the config + _, err = LoadConfig(configPath) + + // Verify that we get the improved error message + if err == nil { + t.Fatal("Expected error for invalid banner file extension, got nil") + } + + expectedMsg := "BannerFile must have a .jpg, .jpeg, or .gif extension (got: banner.png)" + if !strings.Contains(err.Error(), expectedMsg) { + t.Errorf("Expected error message to contain %q, got: %v", expectedMsg, err) + } +} + +func TestLoadConfig_ValidBannerFileExtensions(t *testing.T) { + tests := []struct { + name string + bannerFile string + }{ + {"jpg extension", "banner.jpg"}, + {"jpeg extension", "banner.jpeg"}, + {"gif extension", "banner.gif"}, + {"empty banner", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a temporary directory for test files + tmpDir, err := os.MkdirTemp("", "mobius-config-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create files subdirectory + filesDir := filepath.Join(tmpDir, "files") + if err := os.Mkdir(filesDir, 0755); err != nil { + t.Fatalf("Failed to create files dir: %v", err) + } + + // Create a test config file with a valid banner file extension + configContent := ` +Name: "Test Server" +Description: "Test Description" +BannerFile: "` + tt.bannerFile + `" +FileRoot: "files" +` + configPath := filepath.Join(tmpDir, "config.yaml") + if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { + t.Fatalf("Failed to write config file: %v", err) + } + + // Attempt to load the config + config, err := LoadConfig(configPath) + + // Verify that we don't get a validation error + if err != nil { + t.Errorf("Expected no error for %s, got: %v", tt.bannerFile, err) + } + + if config.BannerFile != tt.bannerFile { + t.Errorf("Expected BannerFile to be %q, got %q", tt.bannerFile, config.BannerFile) + } + }) + } +} diff --git a/internal/mobius/transaction_handlers.go b/internal/mobius/transaction_handlers.go index c5b1bbb..52ea29a 100644 --- a/internal/mobius/transaction_handlers.go +++ b/internal/mobius/transaction_handlers.go @@ -1095,7 +1095,8 @@ func HandleTranAgreed(cc *hotline.ClientConn, t *hotline.Transaction) (res []hot res = append(res, trans...) if cc.Server.Config.BannerFile != "" { - res = append(res, hotline.NewTransaction(hotline.TranServerBanner, cc.ID, hotline.NewField(hotline.FieldBannerType, []byte("JPEG")))) + bannerType := hotline.FileTypeFromFilename(cc.Server.Config.BannerFile).TypeCode + res = append(res, hotline.NewTransaction(hotline.TranServerBanner, cc.ID, hotline.NewField(hotline.FieldBannerType, []byte(bannerType)))) } res = append(res, cc.NewReply(t)) diff --git a/internal/mobius/transaction_handlers_test.go b/internal/mobius/transaction_handlers_test.go index 0d975a7..1c3fac0 100644 --- a/internal/mobius/transaction_handlers_test.go +++ b/internal/mobius/transaction_handlers_test.go @@ -2873,6 +2873,56 @@ func TestHandleTranAgreed(t *testing.T) { }, }, }, + { + name: "with gif banner", + args: args{ + cc: &hotline.ClientConn{ + Account: &hotline.Account{ + Access: func() hotline.AccessBitmap { + var bits hotline.AccessBitmap + bits.Set(hotline.AccessDisconUser) + bits.Set(hotline.AccessAnyName) + return bits + }()}, + Icon: []byte{0, 1}, + Flags: [2]byte{0, 1}, + Version: []byte{0, 1}, + ID: [2]byte{0, 1}, + Logger: NewTestLogger(), + Server: &hotline.Server{ + Config: hotline.Config{ + BannerFile: "Banner.gif", + }, + ClientMgr: func() *hotline.MockClientMgr { + m := hotline.MockClientMgr{} + m.On("List").Return([]*hotline.ClientConn{}, + ) + return &m + }(), + }, + }, + t: hotline.NewTransaction( + hotline.TranAgreed, [2]byte{}, + hotline.NewField(hotline.FieldUserName, []byte("username")), + hotline.NewField(hotline.FieldUserIconID, []byte{0, 1}), + hotline.NewField(hotline.FieldOptions, []byte{0, 0}), + ), + }, + wantRes: []hotline.Transaction{ + { + ClientID: [2]byte{0, 1}, + Type: [2]byte{0, 0x7a}, + Fields: []hotline.Field{ + hotline.NewField(hotline.FieldBannerType, []byte("GIFf")), + }, + }, + { + ClientID: [2]byte{0, 1}, + IsReply: 0x01, + Fields: []hotline.Field{}, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { -- cgit From 489e26d1b3c224beffba2649406efd83580989fe Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Tue, 18 Nov 2025 20:56:12 -0800 Subject: Replace hardcoded magic values with named constants and improve type safety Adds new constants for file format identifiers (FormatFILP), fork types (ForkTypeINFO), and platform identifiers (PlatformAMAC, PlatformMWIN) to improve code maintainability and readability. Changes FlatFileForkHeader.ForkType field from [4]byte to ForkType type alias for better compile-time type checking, matching the pattern used in ForkInfoList.Fork. --- hotline/file_resume_data.go | 5 +++++ hotline/file_wrapper.go | 8 ++++---- hotline/flattened_file_object.go | 4 ++-- hotline/transfer_test.go | 4 ++-- internal/mobius/transaction_handlers_test.go | 6 +++--- 5 files changed, 16 insertions(+), 11 deletions(-) (limited to 'hotline') diff --git a/hotline/file_resume_data.go b/hotline/file_resume_data.go index 80f5fe5..c9f27f6 100644 --- a/hotline/file_resume_data.go +++ b/hotline/file_resume_data.go @@ -32,6 +32,7 @@ type ForkInfoList struct { var ( ForkTypeDATA = ForkType{0x44, 0x41, 0x54, 0x41} // DATA: Data fork + ForkTypeINFO = ForkType{0x49, 0x4E, 0x46, 0x4F} // INFO: Information fork ForkTypeMACR = ForkType{0x4d, 0x41, 0x43, 0x52} // MACR: Mac resource fork ) @@ -45,7 +46,11 @@ func NewForkInfoList(b []byte) *ForkInfoList { } var ( + FormatFILP = [4]byte{0x46, 0x49, 0x4c, 0x50} // Flattened file format: "FILP" FormatRFLT = [4]byte{0x52, 0x46, 0x4C, 0x54} // File resume format: "RFLT" (?) + + PlatformAMAC = [4]byte{0x41, 0x4D, 0x41, 0x43} // Mac platform: "AMAC" + PlatformMWIN = [4]byte{0x4D, 0x57, 0x49, 0x4E} // Windows platform: "MWIN" ) func NewFileResumeData(list []ForkInfoList) *FileResumeData { diff --git a/hotline/file_wrapper.go b/hotline/file_wrapper.go index b6aff94..5ec99c3 100644 --- a/hotline/file_wrapper.go +++ b/hotline/file_wrapper.go @@ -227,7 +227,7 @@ func (f *fileWrapper) flattenedFileObject() (*flattenedFileObject, error) { } f.Ffo.FlatFileHeader = FlatFileHeader{ - Format: [4]byte{0x46, 0x49, 0x4c, 0x50}, // "FILP" + Format: FormatFILP, Version: [2]byte{0, 1}, ForkCount: [2]byte{0, 2}, } @@ -247,7 +247,7 @@ func (f *fileWrapper) flattenedFileObject() (*flattenedFileObject, error) { } } else { f.Ffo.FlatFileInformationFork = FlatFileInformationFork{ - Platform: [4]byte{0x41, 0x4D, 0x41, 0x43}, // "AMAC" TODO: Remove hardcode to support "AWIN" Platform (maybe?) + Platform: PlatformAMAC, // TODO: Remove hardcode to support "MWIN" Platform (maybe?) TypeSignature: [4]byte([]byte(ft.TypeCode)), CreatorSignature: [4]byte([]byte(ft.CreatorCode)), PlatformFlags: [4]byte{0, 0, 1, 0}, // TODO: What is this? @@ -263,12 +263,12 @@ func (f *fileWrapper) flattenedFileObject() (*flattenedFileObject, error) { } f.Ffo.FlatFileInformationForkHeader = FlatFileForkHeader{ - ForkType: [4]byte{0x49, 0x4E, 0x46, 0x4F}, // "INFO" + ForkType: ForkTypeINFO, DataSize: f.Ffo.FlatFileInformationFork.Size(), } f.Ffo.FlatFileDataForkHeader = FlatFileForkHeader{ - ForkType: [4]byte{0x44, 0x41, 0x54, 0x41}, // "DATA" + ForkType: ForkTypeDATA, DataSize: [4]byte{dataSize[0], dataSize[1], dataSize[2], dataSize[3]}, } f.Ffo.FlatFileResForkHeader = f.rsrcForkHeader() diff --git a/hotline/flattened_file_object.go b/hotline/flattened_file_object.go index 99ce204..7c0cd7d 100644 --- a/hotline/flattened_file_object.go +++ b/hotline/flattened_file_object.go @@ -45,7 +45,7 @@ type FlatFileInformationFork struct { func NewFlatFileInformationFork(fileName string, modifyTime [8]byte, typeSignature string, creatorSignature string) FlatFileInformationFork { return FlatFileInformationFork{ - Platform: [4]byte{0x41, 0x4D, 0x41, 0x43}, // "AMAC" TODO: Remove hardcode to support "AWIN" Platform (maybe?) + Platform: PlatformAMAC, // TODO: Remove hardcode to support "MWIN" Platform (maybe?) TypeSignature: [4]byte([]byte(typeSignature)), // TODO: Don't infer types from filename CreatorSignature: [4]byte([]byte(creatorSignature)), // TODO: Don't infer types from filename PlatformFlags: [4]byte{0, 0, 1, 0}, // TODO: What is this? @@ -128,7 +128,7 @@ func (ffif *FlatFileInformationFork) ReadNameSize() []byte { } type FlatFileForkHeader struct { - ForkType [4]byte // Either INFO, DATA or MACR + ForkType ForkType // Either INFO, DATA or MACR CompressionType [4]byte RSVD [4]byte DataSize [4]byte diff --git a/hotline/transfer_test.go b/hotline/transfer_test.go index 4aa142b..c915498 100644 --- a/hotline/transfer_test.go +++ b/hotline/transfer_test.go @@ -120,14 +120,14 @@ func Test_receiveFile(t *testing.T) { conn: func() io.Reader { testFile := flattenedFileObject{ FlatFileHeader: FlatFileHeader{ - Format: [4]byte{0x46, 0x49, 0x4c, 0x50}, // "FILP" + Format: FormatFILP, Version: [2]byte{0, 1}, ForkCount: [2]byte{0, 2}, }, FlatFileInformationForkHeader: FlatFileForkHeader{}, FlatFileInformationFork: NewFlatFileInformationFork("testfile.txt", [8]byte{}, "TEXT", "TEXT"), FlatFileDataForkHeader: FlatFileForkHeader{ - ForkType: [4]byte{0x4d, 0x41, 0x43, 0x52}, // DATA + ForkType: ForkTypeMACR, DataSize: [4]byte{0x00, 0x00, 0x00, 0x03}, }, } diff --git a/internal/mobius/transaction_handlers_test.go b/internal/mobius/transaction_handlers_test.go index 1c3fac0..d943649 100644 --- a/internal/mobius/transaction_handlers_test.go +++ b/internal/mobius/transaction_handlers_test.go @@ -1917,11 +1917,11 @@ func TestHandleDownloadFile(t *testing.T) { ForkCount: [2]byte{0, 2}, ForkInfoList: []hotline.ForkInfoList{ { - Fork: [4]byte{0x44, 0x41, 0x54, 0x41}, // "DATA" - DataSize: [4]byte{0, 0, 0x01, 0x00}, // request offset 256 + Fork: hotline.ForkTypeDATA, + DataSize: [4]byte{0, 0, 0x01, 0x00}, // request offset 256 }, { - Fork: [4]byte{0x4d, 0x41, 0x43, 0x52}, // "MACR" + Fork: hotline.ForkTypeMACR, DataSize: [4]byte{0, 0, 0, 0}, }, }, -- cgit From b395859da4937fa4c3064f4e022e46690df8f2d4 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Tue, 18 Nov 2025 21:01:55 -0800 Subject: Rename hotline.fileWrapper struct to hotline.File --- hotline/file_resume_data.go | 2 +- hotline/file_wrapper.go | 38 +++++++++++++++++++------------------- hotline/transfer_test.go | 2 +- 3 files changed, 21 insertions(+), 21 deletions(-) (limited to 'hotline') diff --git a/hotline/file_resume_data.go b/hotline/file_resume_data.go index c9f27f6..1926bc6 100644 --- a/hotline/file_resume_data.go +++ b/hotline/file_resume_data.go @@ -24,7 +24,7 @@ func (ft ForkType) String() string { } type ForkInfoList struct { - Fork ForkType // "DATA" or "MACR" + Fork ForkType // "DATA", "INFO", or "MACR" DataSize [4]byte // offset from which to resume the transfer of data RSVDA [4]byte // Present in the Hotline protocol docs, but unused. Left here for documentation purposes. RSVDB [4]byte // Present in the Hotline protocol docs, but unused. Left here for documentation purposes. diff --git a/hotline/file_wrapper.go b/hotline/file_wrapper.go index 5ec99c3..41b3338 100644 --- a/hotline/file_wrapper.go +++ b/hotline/file_wrapper.go @@ -17,8 +17,8 @@ const ( RsrcForkNameTemplate = ".rsrc_%s" // template string for resource fork filenames ) -// fileWrapper encapsulates the data, info, and resource forks of a Hotline file and provides methods to manage the files. -type fileWrapper struct { +// File encapsulates the data, info, and resource forks of a Hotline file and provides methods to manage the files. +type File struct { fs FileStore Name string // Name of the file path string // path to file directory @@ -30,10 +30,10 @@ type fileWrapper struct { Ffo *flattenedFileObject } -func NewFileWrapper(fs FileStore, path string, dataOffset int64) (*fileWrapper, error) { +func NewFileWrapper(fs FileStore, path string, dataOffset int64) (*File, error) { dir := filepath.Dir(path) fName := filepath.Base(path) - f := fileWrapper{ + f := File{ fs: fs, Name: fName, path: dir, @@ -54,7 +54,7 @@ func NewFileWrapper(fs FileStore, path string, dataOffset int64) (*fileWrapper, return &f, nil } -func (f *fileWrapper) TotalSize() []byte { +func (f *File) TotalSize() []byte { var s int64 size := make([]byte, 4) @@ -73,7 +73,7 @@ func (f *fileWrapper) TotalSize() []byte { return size } -func (f *fileWrapper) rsrcForkSize() (s [4]byte) { +func (f *File) rsrcForkSize() (s [4]byte) { info, err := f.fs.Stat(f.rsrcPath) if err != nil { return s @@ -83,26 +83,26 @@ func (f *fileWrapper) rsrcForkSize() (s [4]byte) { return s } -func (f *fileWrapper) rsrcForkHeader() FlatFileForkHeader { +func (f *File) rsrcForkHeader() FlatFileForkHeader { return FlatFileForkHeader{ ForkType: ForkTypeMACR, DataSize: f.rsrcForkSize(), } } -func (f *fileWrapper) incompleteDataName() string { +func (f *File) incompleteDataName() string { return f.Name + IncompleteFileSuffix } -func (f *fileWrapper) rsrcForkName() string { +func (f *File) rsrcForkName() string { return fmt.Sprintf(RsrcForkNameTemplate, f.Name) } -func (f *fileWrapper) infoForkName() string { +func (f *File) infoForkName() string { return fmt.Sprintf(InfoForkNameTemplate, f.Name) } -func (f *fileWrapper) rsrcForkWriter() (io.WriteCloser, error) { +func (f *File) rsrcForkWriter() (io.WriteCloser, error) { file, err := os.OpenFile(f.rsrcPath, os.O_CREATE|os.O_WRONLY, 0644) if err != nil { return nil, err @@ -111,7 +111,7 @@ func (f *fileWrapper) rsrcForkWriter() (io.WriteCloser, error) { return file, nil } -func (f *fileWrapper) InfoForkWriter() (io.WriteCloser, error) { +func (f *File) InfoForkWriter() (io.WriteCloser, error) { file, err := os.OpenFile(f.infoPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) if err != nil { return nil, err @@ -120,7 +120,7 @@ func (f *fileWrapper) InfoForkWriter() (io.WriteCloser, error) { return file, nil } -func (f *fileWrapper) incFileWriter() (io.WriteCloser, error) { +func (f *File) incFileWriter() (io.WriteCloser, error) { file, err := os.OpenFile(f.incompletePath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) if err != nil { return nil, err @@ -129,15 +129,15 @@ func (f *fileWrapper) incFileWriter() (io.WriteCloser, error) { return file, nil } -func (f *fileWrapper) dataForkReader() (io.Reader, error) { +func (f *File) dataForkReader() (io.Reader, error) { return f.fs.Open(f.dataPath) } -func (f *fileWrapper) rsrcForkFile() (*os.File, error) { +func (f *File) rsrcForkFile() (*os.File, error) { return f.fs.Open(f.rsrcPath) } -func (f *fileWrapper) DataFile() (os.FileInfo, error) { +func (f *File) DataFile() (os.FileInfo, error) { if fi, err := f.fs.Stat(f.dataPath); err == nil { return fi, nil } @@ -154,7 +154,7 @@ func (f *fileWrapper) DataFile() (os.FileInfo, error) { // * Resource fork starting with .rsrc_ // * Info fork starting with .info // During Move of the meta files, os.ErrNotExist is ignored as these files may legitimately not exist. -func (f *fileWrapper) Move(newPath string) error { +func (f *File) Move(newPath string) error { err := f.fs.Rename(f.dataPath, filepath.Join(newPath, f.Name)) if err != nil { return err @@ -179,7 +179,7 @@ func (f *fileWrapper) Move(newPath string) error { } // Delete a file and its associated metadata files if they exist -func (f *fileWrapper) Delete() error { +func (f *File) Delete() error { err := f.fs.RemoveAll(f.dataPath) if err != nil { return err @@ -203,7 +203,7 @@ func (f *fileWrapper) Delete() error { return nil } -func (f *fileWrapper) flattenedFileObject() (*flattenedFileObject, error) { +func (f *File) flattenedFileObject() (*flattenedFileObject, error) { dataSize := make([]byte, 4) mTime := [8]byte{} diff --git a/hotline/transfer_test.go b/hotline/transfer_test.go index c915498..9155ad7 100644 --- a/hotline/transfer_test.go +++ b/hotline/transfer_test.go @@ -143,7 +143,7 @@ func Test_receiveFile(t *testing.T) { wantErr: assert.NoError, }, // { - // Name: "transfers fileWrapper when there is a resource fork", + // Name: "transfers File when there is a resource fork", // args: args{ // conn: func() io.Reader { // testFile := flattenedFileObject{ -- cgit From 1b2df8a630bfc83304ef90610bdf7ebe91d4e555 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Wed, 19 Nov 2025 16:36:06 -0800 Subject: Fix error when downloading incomplete files --- hotline/file_transfer.go | 16 ++++++++-------- hotline/file_wrapper.go | 8 ++++++-- hotline/files.go | 4 ++-- internal/mobius/transaction_handlers.go | 15 +++++++-------- 4 files changed, 23 insertions(+), 20 deletions(-) (limited to 'hotline') diff --git a/hotline/file_transfer.go b/hotline/file_transfer.go index 521b963..e8acbb3 100644 --- a/hotline/file_transfer.go +++ b/hotline/file_transfer.go @@ -257,7 +257,7 @@ func DownloadHandler(w io.Writer, fullPath string, fileTransfer *FileTransfer, f dataOffset = int64(binary.BigEndian.Uint32(fileTransfer.FileResumeData.ForkInfoList[0].DataSize[:])) } - fw, err := NewFileWrapper(fs, fullPath, 0) + hlFile, err := NewFile(fs, fullPath, 0) if err != nil { return fmt.Errorf("reading file header: %v", err) } @@ -267,12 +267,12 @@ func DownloadHandler(w io.Writer, fullPath string, fileTransfer *FileTransfer, f // If file transfer options are included, that means this is a "quick preview" request. In this case skip sending // the flat file info and proceed directly to sending the file data. if fileTransfer.Options == nil { - if _, err = io.Copy(w, fw.Ffo); err != nil { + if _, err = io.Copy(w, hlFile.Ffo); err != nil { return fmt.Errorf("send flat file object: %v", err) } } - file, err := fw.dataForkReader() + file, err := hlFile.dataForkReader() if err != nil { return fmt.Errorf("open data fork reader: %v", err) } @@ -288,13 +288,13 @@ func DownloadHandler(w io.Writer, fullPath string, fileTransfer *FileTransfer, f // If the client requested to resume transfer, do not send the resource fork header. if fileTransfer.FileResumeData == nil { - err = binary.Write(w, binary.BigEndian, fw.rsrcForkHeader()) + err = binary.Write(w, binary.BigEndian, hlFile.rsrcForkHeader()) if err != nil { return fmt.Errorf("send resource fork header: %v", err) } } - rFile, _ := fw.rsrcForkFile() + rFile, _ := hlFile.rsrcForkFile() //if err != nil { // // return fmt.Errorf("open resource fork file: %v", err) //} @@ -332,7 +332,7 @@ func UploadHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileTransfe return fmt.Errorf("open temp file for uploade: %w", err) } - f, err := NewFileWrapper(fileStore, fullPath, 0) + f, err := NewFile(fileStore, fullPath, 0) if err != nil { return err } @@ -424,7 +424,7 @@ func DownloadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *Fil return nil } - hlFile, err := NewFileWrapper(fileStore, path, 0) + hlFile, err := NewFile(fileStore, path, 0) if err != nil { return err } @@ -645,7 +645,7 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT filePath := path.Join(fullPath, fu.FormattedPath()) - hlFile, err := NewFileWrapper(fileStore, filePath, 0) + hlFile, err := NewFile(fileStore, filePath, 0) if err != nil { return err } diff --git a/hotline/file_wrapper.go b/hotline/file_wrapper.go index 41b3338..b80bc4b 100644 --- a/hotline/file_wrapper.go +++ b/hotline/file_wrapper.go @@ -30,7 +30,7 @@ type File struct { Ffo *flattenedFileObject } -func NewFileWrapper(fs FileStore, path string, dataOffset int64) (*File, error) { +func NewFile(fs FileStore, path string, dataOffset int64) (*File, error) { dir := filepath.Dir(path) fName := filepath.Base(path) f := File{ @@ -130,7 +130,11 @@ func (f *File) incFileWriter() (io.WriteCloser, error) { } func (f *File) dataForkReader() (io.Reader, error) { - return f.fs.Open(f.dataPath) + if _, err := f.fs.Stat(f.dataPath); err == nil { + return f.fs.Open(f.dataPath) + } + + return f.fs.Open(f.incompletePath) } func (f *File) rsrcForkFile() (*os.File, error) { diff --git a/hotline/files.go b/hotline/files.go index e9f7b4f..581b11c 100644 --- a/hotline/files.go +++ b/hotline/files.go @@ -112,9 +112,9 @@ func GetFileNameList(path string, ignoreList []string) (fields []Field, err erro continue } - hlFile, err := NewFileWrapper(&OSFileStore{}, path+"/"+file.Name(), 0) + hlFile, err := NewFile(&OSFileStore{}, path+"/"+file.Name(), 0) if err != nil { - return nil, fmt.Errorf("NewFileWrapper: %w", err) + return nil, fmt.Errorf("NewFile: %w", err) } copy(fnwi.FileSize[:], hlFile.TotalSize()) diff --git a/internal/mobius/transaction_handlers.go b/internal/mobius/transaction_handlers.go index 52ea29a..20a315d 100644 --- a/internal/mobius/transaction_handlers.go +++ b/internal/mobius/transaction_handlers.go @@ -86,11 +86,10 @@ const ( // General error messages ErrMsgAccountNotFound = "Account not found." - ErrMsgUserNotFound = "User not found." - ErrMsgCreateAlias = "Error creating alias" + ErrMsgUserNotFound = "User not found." + ErrMsgCreateAlias = "Error creating alias" ) - // Converts bytes from Mac Roman encoding to UTF-8 var txtDecoder = charmap.Macintosh.NewDecoder() @@ -307,7 +306,7 @@ func HandleGetFileInfo(cc *hotline.ClientConn, t *hotline.Transaction) (res []ho return res } - fw, err := hotline.NewFileWrapper(cc.Server.FS, fullFilePath, 0) + fw, err := hotline.NewFile(cc.Server.FS, fullFilePath, 0) if err != nil { return res } @@ -361,7 +360,7 @@ func HandleSetFileInfo(cc *hotline.ClientConn, t *hotline.Transaction) (res []ho return res } - hlFile, err := hotline.NewFileWrapper(cc.Server.FS, fullFilePath, 0) + hlFile, err := hotline.NewFile(cc.Server.FS, fullFilePath, 0) if err != nil { return res } @@ -449,7 +448,7 @@ func HandleDeleteFile(cc *hotline.ClientConn, t *hotline.Transaction) (res []hot return res } - hlFile, err := hotline.NewFileWrapper(cc.Server.FS, fullFilePath, 0) + hlFile, err := hotline.NewFile(cc.Server.FS, fullFilePath, 0) if err != nil { return res } @@ -494,7 +493,7 @@ func HandleMoveFile(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotli cc.Logger.Info("Move file", "src", filePath+"/"+fileName, "dst", fileNewPath+"/"+fileName) - hlFile, err := hotline.NewFileWrapper(cc.Server.FS, filePath, 0) + hlFile, err := hotline.NewFile(cc.Server.FS, filePath, 0) if err != nil { return res } @@ -1553,7 +1552,7 @@ func HandleDownloadFile(cc *hotline.ClientConn, t *hotline.Transaction) (res []h return res } - hlFile, err := hotline.NewFileWrapper(cc.Server.FS, fullFilePath, dataOffset) + hlFile, err := hotline.NewFile(cc.Server.FS, fullFilePath, dataOffset) if err != nil { return res } -- cgit From 357baa94bf9b1d4b623f8a9c04b9d591e9f60406 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Sat, 22 Nov 2025 19:42:32 -0800 Subject: Add optional TLS support for encrypted client connections - Add TLSConfig and TLSPort fields to Server struct - Add WithTLS option function for configuration - Add ServeWithTLS and ServeFileTransfersWithTLS methods - Update ListenAndServe to start TLS listeners when configured - Add -tls-cert, -tls-key, -tls-port command-line flags - Fix data race in rateLimiters map access with mutex - Add TLS documentation with certificate generation instructions --- cmd/mobius-hotline-server/main.go | 26 +++++++++- docs/tls.md | 101 ++++++++++++++++++++++++++++++++++++++ hotline/server.go | 47 +++++++++++++++++- 3 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 docs/tls.md (limited to 'hotline') diff --git a/cmd/mobius-hotline-server/main.go b/cmd/mobius-hotline-server/main.go index aa41279..cc24f5f 100644 --- a/cmd/mobius-hotline-server/main.go +++ b/cmd/mobius-hotline-server/main.go @@ -2,6 +2,7 @@ package main import ( "context" + "crypto/tls" "embed" "flag" "fmt" @@ -44,6 +45,9 @@ func main() { logLevel := flag.String("log-level", "info", "Log level") logFile := flag.String("log-file", "", "Path to log file") init := flag.Bool("init", false, "Populate the config dir with default configuration") + tlsCert := flag.String("tls-cert", "", "Path to TLS certificate file") + tlsKey := flag.String("tls-key", "", "Path to TLS key file") + tlsPort := flag.Int("tls-port", 5600, "Base TLS port. TLS file transfer port is base + 1.") flag.Parse() @@ -78,12 +82,27 @@ func main() { os.Exit(1) } - srv, err := hotline.NewServer( + var tlsConfig *tls.Config + if *tlsCert != "" && *tlsKey != "" { + cert, err := tls.LoadX509KeyPair(*tlsCert, *tlsKey) + if err != nil { + slogger.Error("Error loading TLS certificate", "err", err) + os.Exit(1) + } + tlsConfig = &tls.Config{Certificates: []tls.Certificate{cert}} + } + + opts := []hotline.Option{ hotline.WithInterface(*netInterface), hotline.WithLogger(slogger), hotline.WithPort(*basePort), hotline.WithConfig(*config), - ) + } + if tlsConfig != nil { + opts = append(opts, hotline.WithTLS(tlsConfig, *tlsPort)) + } + + srv, err := hotline.NewServer(opts...) if err != nil { slogger.Error("Error starting server", "err", err) os.Exit(1) @@ -174,6 +193,9 @@ func main() { }() slogger.Info("Hotline server started", "version", version, "config", *configDir) + if tlsConfig != nil { + slogger.Info("TLS enabled", "port", *tlsPort, "fileTransferPort", *tlsPort+1) + } // Assign functions to handle specific Hotline transaction types mobius.RegisterHandlers(srv) diff --git a/docs/tls.md b/docs/tls.md new file mode 100644 index 0000000..6cb2fd5 --- /dev/null +++ b/docs/tls.md @@ -0,0 +1,101 @@ +# TLS Support + +Mobius supports TLS (Transport Layer Security) for encrypted connections between clients and the server. When enabled, TLS runs on separate ports alongside the standard unencrypted ports, allowing both secure and legacy client connections simultaneously. + +## Ports + +| Service | Standard Port | TLS Port (default) | +|---------------|---------------|-------------------| +| Hotline | 5500 | 5600 | +| File Transfer | 5501 | 5601 | + +## Generating Certificates + +### Self-Signed Certificate (Testing/Private Use) + +For testing or private servers, you can generate a self-signed certificate using OpenSSL: + +```bash +openssl req -x509 -newkey rsa:4096 -keyout server.key -out server.crt -days 365 -nodes -subj "/CN=localhost" +``` + +This creates: +- `server.key` - Private key file +- `server.crt` - Certificate file + +For a certificate that includes your server's hostname or IP address: + +```bash +openssl req -x509 -newkey rsa:4096 -keyout server.key -out server.crt -days 365 -nodes \ + -subj "/CN=your-hostname.example.com" \ + -addext "subjectAltName=DNS:your-hostname.example.com,IP:192.168.1.100" +``` + +### Let's Encrypt (Production) + +For production servers with a public domain name, use [Let's Encrypt](https://letsencrypt.org/) with certbot: + +```bash +certbot certonly --standalone -d your-hostname.example.com +``` + +The certificates are typically stored at: +- `/etc/letsencrypt/live/your-hostname.example.com/fullchain.pem` +- `/etc/letsencrypt/live/your-hostname.example.com/privkey.pem` + +## Command-Line Options + +| Flag | Description | Default | +|------|-------------|---------| +| `-tls-cert` | Path to TLS certificate file | (none) | +| `-tls-key` | Path to TLS private key file | (none) | +| `-tls-port` | Base TLS port (file transfer uses base + 1) | 5600 | + +TLS is enabled when both `-tls-cert` and `-tls-key` are provided. + +## Usage Examples + +### Basic TLS Setup + +```bash +mobius-hotline-server -tls-cert server.crt -tls-key server.key +``` + +### Custom TLS Port + +```bash +mobius-hotline-server -tls-cert server.crt -tls-key server.key -tls-port 5700 +``` + +### Full Example with All Options + +```bash +mobius-hotline-server \ + -config /path/to/config \ + -bind 5500 \ + -tls-cert /etc/letsencrypt/live/example.com/fullchain.pem \ + -tls-key /etc/letsencrypt/live/example.com/privkey.pem \ + -tls-port 5600 +``` + +## Verifying TLS is Working + +When TLS is enabled, you'll see a log message at startup: + +``` +TLS enabled port=5600 fileTransferPort=5601 +``` + +You can verify the TLS connection using OpenSSL: + +```bash +openssl s_client -connect localhost:5600 +``` + +## Client Configuration + +Clients connecting via TLS must: +1. Connect to the TLS port (default 5600) instead of the standard port (5500) +2. Support TLS connections (client-dependent) + +Note: Self-signed certificates may require clients to accept or trust the certificate manually. diff --git a/hotline/server.go b/hotline/server.go index 0395d7b..2c25e93 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "crypto/rand" + "crypto/tls" "encoding/binary" "errors" "fmt" @@ -40,7 +41,8 @@ type Server struct { NetInterface string Port int - rateLimiters map[string]*rate.Limiter + rateLimiters map[string]*rate.Limiter + rateLimitersMu sync.Mutex handlers map[TranType]HandlerFunc @@ -71,6 +73,9 @@ type Server struct { // TrackerRegistrar handles tracker registration (injectable for testing) TrackerRegistrar TrackerRegistrar + + TLSConfig *tls.Config + TLSPort int } type Option = func(s *Server) @@ -108,6 +113,14 @@ func WithTrackerRegistrar(registrar TrackerRegistrar) func(s *Server) { } } +// WithTLS optionally enables TLS support on the specified port. +func WithTLS(tlsConfig *tls.Config, port int) func(s *Server) { + return func(s *Server) { + s.TLSConfig = tlsConfig + s.TLSPort = port + } +} + type ServerConfig struct { } @@ -168,6 +181,28 @@ func (s *Server) ListenAndServe(ctx context.Context) error { log.Fatal(s.ServeFileTransfers(ctx, ln)) }() + if s.TLSConfig != nil { + wg.Add(1) + go func() { + ln, err := net.Listen("tcp", fmt.Sprintf("%s:%v", s.NetInterface, s.TLSPort)) + if err != nil { + log.Fatal(err) + } + + log.Fatal(s.ServeWithTLS(ctx, ln)) + }() + + wg.Add(1) + go func() { + ln, err := net.Listen("tcp", fmt.Sprintf("%s:%v", s.NetInterface, s.TLSPort+1)) + if err != nil { + log.Fatal(err) + } + + log.Fatal(s.ServeFileTransfersWithTLS(ctx, ln)) + }() + } + wg.Wait() return nil @@ -195,6 +230,14 @@ func (s *Server) ServeFileTransfers(ctx context.Context, ln net.Listener) error } } +func (s *Server) ServeWithTLS(ctx context.Context, ln net.Listener) error { + return s.Serve(ctx, tls.NewListener(ln, s.TLSConfig)) +} + +func (s *Server) ServeFileTransfersWithTLS(ctx context.Context, ln net.Listener) error { + return s.ServeFileTransfers(ctx, tls.NewListener(ln, s.TLSConfig)) +} + func (s *Server) sendTransaction(t Transaction) error { client := s.ClientMgr.Get(t.ClientID) @@ -249,11 +292,13 @@ func (s *Server) Serve(ctx context.Context, ln net.Listener) error { defer func() { _ = conn.Close() }() // Check if we have an existing rate limit for the IP and create one if we do not. + s.rateLimitersMu.Lock() rl, ok := s.rateLimiters[ipAddr] if !ok { rl = rate.NewLimiter(perIPRateLimit, 1) s.rateLimiters[ipAddr] = rl } + s.rateLimitersMu.Unlock() // Check if the rate limit is exceeded and close the connection if so. if !rl.Allow() { -- cgit From ab16c7d82c8381a4df80fef4b2fe81e7ff23dad8 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:52:37 -0800 Subject: Fix tracker server list parsing for batched responses The tracker protocol splits large server lists into batches, with each batch preceded by a ServerInfoHeader. Previously, GetListing only read the initial header and failed when encountering subsequent headers mid-stream, causing the scanner to misinterpret header bytes as server record data. Changes: - Refactored GetListing to use bufio.Reader instead of bufio.Scanner to handle heterogeneous data (headers and server records) - Added readServerRecord helper function for sequential reading - Renamed ServerInfoHeader.SrvCountDup to BatchSize for clarity - Added comprehensive documentation explaining the batching protocol - Removed unused serverScanner function and tests - Added table tests covering multiple batch scenarios The BatchSize field indicates the number of servers in the current batch, while SrvCount indicates the total across all batches. --- hotline/tracker.go | 114 +++++++++------ hotline/tracker_test.go | 375 ++++++++++++++++++++++++++++++------------------ 2 files changed, 307 insertions(+), 182 deletions(-) (limited to 'hotline') diff --git a/hotline/tracker.go b/hotline/tracker.go index edd973d..bfba69d 100644 --- a/hotline/tracker.go +++ b/hotline/tracker.go @@ -91,11 +91,22 @@ type TrackerHeader struct { Version [2]byte // Old protocol (1) or new (2) } +// ServerInfoHeader represents a batch header in the tracker response. +// The tracker protocol splits large server lists into batches, with each batch +// preceded by its own ServerInfoHeader. The first header indicates the total +// number of servers across all batches, and each header (including the first) +// indicates how many servers are in the current batch. +// +// Example flow for 106 servers split into batches: +// 1. First ServerInfoHeader: SrvCount=106, BatchSize=97 +// 2. Read 97 ServerRecords +// 3. Second ServerInfoHeader: SrvCount=106, BatchSize=9 +// 4. Read 9 ServerRecords (total: 106) type ServerInfoHeader struct { MsgType [2]byte // Always has value of 1 MsgDataSize [2]byte // Remaining size of request - SrvCount [2]byte // Number of servers in the server list - SrvCountDup [2]byte // Same as previous field ¯\_(ツ)_/¯ + SrvCount [2]byte // Total number of servers across all batches + BatchSize [2]byte // Number of servers in the current batch } // ServerRecord is a tracker listing for a single server @@ -128,79 +139,92 @@ func GetListing(conn io.ReadWriteCloser) ([]ServerRecord, error) { return nil, err } + // Use a buffered reader so we can read both headers and server records from the same buffer + reader := bufio.NewReader(conn) + var info ServerInfoHeader - if err := binary.Read(conn, binary.BigEndian, &info); err != nil { + if err := binary.Read(reader, binary.BigEndian, &info); err != nil { return nil, err } totalSrv := int(binary.BigEndian.Uint16(info.SrvCount[:])) + batchSize := int(binary.BigEndian.Uint16(info.BatchSize[:])) - scanner := bufio.NewScanner(conn) - scanner.Split(serverScanner) + servers := make([]ServerRecord, 0, totalSrv) + serversInCurrentBatch := 0 - var servers []ServerRecord - for { - scanner.Scan() + for len(servers) < totalSrv { + // Check if we've read all servers in the current batch + if serversInCurrentBatch == batchSize { + // Read the next ServerInfoHeader for the next batch from the buffered reader + if err := binary.Read(reader, binary.BigEndian, &info); err != nil { + return nil, fmt.Errorf("failed to read next ServerInfoHeader after %d servers: %w", len(servers), err) + } - // Make a new []byte slice and copy the scanner bytes to it. This is critical as the - // scanner re-uses the buffer for subsequent scans. - buf := make([]byte, len(scanner.Bytes())) - copy(buf, scanner.Bytes()) + batchSize = int(binary.BigEndian.Uint16(info.BatchSize[:])) + serversInCurrentBatch = 0 + } - var srv ServerRecord - _, err = srv.Write(buf) + // Read a server record using our helper function + srv, err := readServerRecord(reader) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read server record %d: %w", len(servers)+1, err) } servers = append(servers, srv) - if len(servers) == totalSrv { - break - } + serversInCurrentBatch++ } return servers, nil } -// serverScanner implements bufio.SplitFunc for parsing the tracker list into ServerRecords tokens -// Example payload: -// 00000000 18 05 30 63 15 7c 00 02 00 00 10 54 68 65 20 4d |..0c.|.....The M| -// 00000010 6f 62 69 75 73 20 53 74 72 69 70 40 48 6f 6d 65 |obius Strip@Home| -// 00000020 20 6f 66 20 74 68 65 20 4d 6f 62 69 75 73 20 48 | of the Mobius H| -// 00000030 6f 74 6c 69 6e 65 20 73 65 72 76 65 72 20 61 6e |otline server an| -// 00000040 64 20 63 6c 69 65 6e 74 20 7c 20 54 52 54 50 48 |d client | TRTPH| -// 00000050 4f 54 4c 2e 63 6f 6d 3a 35 35 30 30 2d 4f 3a b2 |OTL.com:5500-O:.| -// 00000060 15 7c 00 00 00 00 08 53 65 6e 65 63 74 75 73 20 |.|.....Senectus | -func serverScanner(data []byte, _ bool) (advance int, token []byte, err error) { - // The name length field is the 11th byte of the server record. If we don't have that many bytes, - // return nil token so the Scanner reads more data and continues scanning. - if len(data) < 10 { - return 0, nil, nil +// readServerRecord reads a single ServerRecord from the reader +func readServerRecord(reader *bufio.Reader) (ServerRecord, error) { + var srv ServerRecord + + // Read fixed header: IP (4) + Port (2) + NumUsers (2) + Unused (2) = 10 bytes + header := make([]byte, 10) + if _, err := io.ReadFull(reader, header); err != nil { + return srv, fmt.Errorf("failed to read server header: %w", err) } - // A server entry has two variable length fields: the name and description. - // To get the token length, we first need the name length from the 10th byte - nameLen := int(data[10]) + copy(srv.IPAddr[:], header[0:4]) + copy(srv.Port[:], header[4:6]) + copy(srv.NumUsers[:], header[6:8]) + copy(srv.Unused[:], header[8:10]) - // The description length field is at the 12th + nameLen byte of the server record. - // If we don't have that many bytes, return nil token so the Scanner reads more data and continues scanning. - if len(data) < 11+nameLen { - return 0, nil, nil + // Read name size + nameSizeByte, err := reader.ReadByte() + if err != nil { + return srv, fmt.Errorf("failed to read name size: %w", err) } + srv.NameSize = nameSizeByte - // Next we need the description length from the 11+nameLen byte: - descLen := int(data[11+nameLen]) + // Read name + srv.Name = make([]byte, srv.NameSize) + if _, err := io.ReadFull(reader, srv.Name); err != nil { + return srv, fmt.Errorf("failed to read name: %w", err) + } + + // Read description size + descSizeByte, err := reader.ReadByte() + if err != nil { + return srv, fmt.Errorf("failed to read description size: %w", err) + } + srv.DescriptionSize = descSizeByte - if len(data) < 12+nameLen+descLen { - return 0, nil, nil + // Read description + srv.Description = make([]byte, srv.DescriptionSize) + if _, err := io.ReadFull(reader, srv.Description); err != nil { + return srv, fmt.Errorf("failed to read description: %w", err) } - return 12 + nameLen + descLen, data[0 : 12+nameLen+descLen], nil + return srv, nil } // Write implements io.Writer for ServerRecord func (s *ServerRecord) Write(b []byte) (n int, err error) { - if len(b) < 13 { + if len(b) < 12 { return 0, errors.New("too few bytes") } copy(s.IPAddr[:], b[0:4]) diff --git a/hotline/tracker_test.go b/hotline/tracker_test.go index 5457e47..8295582 100644 --- a/hotline/tracker_test.go +++ b/hotline/tracker_test.go @@ -2,11 +2,11 @@ package hotline import ( "bytes" - "fmt" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "io" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTrackerRegistration_Payload(t *testing.T) { @@ -62,138 +62,6 @@ func TestTrackerRegistration_Payload(t *testing.T) { } } -func Test_serverScanner(t *testing.T) { - type args struct { - data []byte - atEOF bool - } - tests := []struct { - name string - args args - wantAdvance int - wantToken []byte - wantErr assert.ErrorAssertionFunc - }{ - { - name: "when a full server entry is provided", - args: args{ - data: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - atEOF: false, - }, - wantAdvance: 18, - wantToken: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - wantErr: assert.NoError, - }, - { - name: "when extra bytes are provided", - args: args{ - data: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - 0x54, 0x54, 0x54, 0x54, 0x54, 0x54, 0x54, 0x54, 0x54, - }, - atEOF: false, - }, - wantAdvance: 18, - wantToken: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - wantErr: assert.NoError, - }, - { - name: "when insufficient bytes are provided", - args: args{ - data: []byte{ - 0, 0, - }, - atEOF: false, - }, - wantAdvance: 0, - wantToken: []byte(nil), - wantErr: assert.NoError, - }, - { - name: "when nameLen exceeds provided data", - args: args{ - data: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0xff, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - atEOF: false, - }, - wantAdvance: 0, - wantToken: []byte(nil), - wantErr: assert.NoError, - }, - { - name: "when description len exceeds provided data", - args: args{ - data: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0xff, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - atEOF: false, - }, - wantAdvance: 0, - wantToken: []byte(nil), - wantErr: assert.NoError, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotAdvance, gotToken, err := serverScanner(tt.args.data, tt.args.atEOF) - if !tt.wantErr(t, err, fmt.Sprintf("serverScanner(%v, %v)", tt.args.data, tt.args.atEOF)) { - return - } - assert.Equalf(t, tt.wantAdvance, gotAdvance, "serverScanner(%v, %v)", tt.args.data, tt.args.atEOF) - assert.Equalf(t, tt.wantToken, gotToken, "serverScanner(%v, %v)", tt.args.data, tt.args.atEOF) - }) - } -} - type mockConn struct { readBuffer *bytes.Buffer writeBuffer *bytes.Buffer @@ -231,7 +99,7 @@ func TestGetListing(t *testing.T) { 0x00, 0x01, // MsgType (1) 0x00, 0x14, // MsgDataSize (20) 0x00, 0x02, // SrvCount (2) - 0x00, 0x02, // SrvCountDup (2) + 0x00, 0x02, // BatchSize (2) // ServerRecord 1 192, 168, 1, 1, // IP address 0x1F, 0x90, // Port 8080 @@ -324,7 +192,7 @@ func TestGetListing(t *testing.T) { 0x00, 0x01, // MsgType (1) 0x00, 0x14, // MsgDataSize (20) 0x00, 0x01, // SrvCount (1) - 0x00, 0x01, // SrvCountDup (1) + 0x00, 0x01, // BatchSize (1) // incomplete ServerRecord to cause scanner error 192, 168, 1, 1, }), @@ -333,6 +201,239 @@ func TestGetListing(t *testing.T) { wantErr: true, wantResult: nil, }, + { + name: "Multiple batches with ServerInfoHeaders", + mockConn: &mockConn{ + readBuffer: bytes.NewBuffer([]byte{ + // TrackerHeader + 0x48, 0x54, 0x52, 0x4B, // Protocol "HTRK" + 0x00, 0x01, // Version 1 + // First ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x14, // MsgDataSize (20) + 0x00, 0x03, // SrvCount (3 total) + 0x00, 0x02, // BatchSize (2 in first batch) + // ServerRecord 1 + 192, 168, 1, 1, // IP address + 0x1F, 0x90, // Port 8080 + 0x00, 0x0A, // NumUsers 10 + 0x00, 0x00, // Unused + 0x07, // NameSize + 'S', 'e', 'r', 'v', 'e', 'r', '1', // Name + 0x0C, // DescriptionSize + 'F', 'i', 'r', 's', 't', ' ', 'b', 'a', 't', 'c', 'h', '1', // Description + // ServerRecord 2 + 192, 168, 1, 2, // IP address + 0x1F, 0x91, // Port 8081 + 0x00, 0x14, // NumUsers 20 + 0x00, 0x00, // Unused + 0x07, // NameSize + 'S', 'e', 'r', 'v', 'e', 'r', '2', // Name + 0x0C, // DescriptionSize + 'F', 'i', 'r', 's', 't', ' ', 'b', 'a', 't', 'c', 'h', '2', // Description + // Second ServerInfoHeader (next batch) + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize (10) + 0x00, 0x03, // SrvCount (3 total - same) + 0x00, 0x01, // BatchSize (1 in second batch) + // ServerRecord 3 + 192, 168, 1, 3, // IP address + 0x1F, 0x92, // Port 8082 + 0x00, 0x1E, // NumUsers 30 + 0x00, 0x00, // Unused + 0x07, // NameSize + 'S', 'e', 'r', 'v', 'e', 'r', '3', // Name + 0x0D, // DescriptionSize + 'S', 'e', 'c', 'o', 'n', 'd', ' ', 'b', 'a', 't', 'c', 'h', '1', // Description + }), + writeBuffer: &bytes.Buffer{}, + }, + wantErr: false, + wantResult: []ServerRecord{ + { + IPAddr: [4]byte{192, 168, 1, 1}, + Port: [2]byte{0x1F, 0x90}, + NumUsers: [2]byte{0x00, 0x0A}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 7, + Name: []byte("Server1"), + DescriptionSize: 12, + Description: []byte("First batch1"), + }, + { + IPAddr: [4]byte{192, 168, 1, 2}, + Port: [2]byte{0x1F, 0x91}, + NumUsers: [2]byte{0x00, 0x14}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 7, + Name: []byte("Server2"), + DescriptionSize: 12, + Description: []byte("First batch2"), + }, + { + IPAddr: [4]byte{192, 168, 1, 3}, + Port: [2]byte{0x1F, 0x92}, + NumUsers: [2]byte{0x00, 0x1E}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 7, + Name: []byte("Server3"), + DescriptionSize: 13, + Description: []byte("Second batch1"), + }, + }, + }, + { + name: "Three batches", + mockConn: &mockConn{ + readBuffer: bytes.NewBuffer([]byte{ + // TrackerHeader + 0x48, 0x54, 0x52, 0x4B, // Protocol "HTRK" + 0x00, 0x01, // Version 1 + // First ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize + 0x00, 0x04, // SrvCount (4 total) + 0x00, 0x02, // BatchSize (2 in first batch) + // ServerRecord 1 + 192, 168, 1, 1, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x01, // NumUsers 1 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'A', // Name + 0x01, // DescriptionSize + '1', // Description + // ServerRecord 2 + 192, 168, 1, 2, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x02, // NumUsers 2 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'B', // Name + 0x01, // DescriptionSize + '2', // Description + // Second ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize + 0x00, 0x04, // SrvCount (4 total) + 0x00, 0x01, // BatchSize (1 in second batch) + // ServerRecord 3 + 192, 168, 1, 3, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x03, // NumUsers 3 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'C', // Name + 0x01, // DescriptionSize + '3', // Description + // Third ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize + 0x00, 0x04, // SrvCount (4 total) + 0x00, 0x01, // BatchSize (1 in third batch) + // ServerRecord 4 + 192, 168, 1, 4, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x04, // NumUsers 4 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'D', // Name + 0x01, // DescriptionSize + '4', // Description + }), + writeBuffer: &bytes.Buffer{}, + }, + wantErr: false, + wantResult: []ServerRecord{ + { + IPAddr: [4]byte{192, 168, 1, 1}, + Port: [2]byte{0x15, 0x7c}, + NumUsers: [2]byte{0x00, 0x01}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 1, + Name: []byte("A"), + DescriptionSize: 1, + Description: []byte("1"), + }, + { + IPAddr: [4]byte{192, 168, 1, 2}, + Port: [2]byte{0x15, 0x7c}, + NumUsers: [2]byte{0x00, 0x02}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 1, + Name: []byte("B"), + DescriptionSize: 1, + Description: []byte("2"), + }, + { + IPAddr: [4]byte{192, 168, 1, 3}, + Port: [2]byte{0x15, 0x7c}, + NumUsers: [2]byte{0x00, 0x03}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 1, + Name: []byte("C"), + DescriptionSize: 1, + Description: []byte("3"), + }, + { + IPAddr: [4]byte{192, 168, 1, 4}, + Port: [2]byte{0x15, 0x7c}, + NumUsers: [2]byte{0x00, 0x04}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 1, + Name: []byte("D"), + DescriptionSize: 1, + Description: []byte("4"), + }, + }, + }, + { + name: "Error reading second ServerInfoHeader", + mockConn: &mockConn{ + readBuffer: bytes.NewBuffer([]byte{ + // TrackerHeader + 0x48, 0x54, 0x52, 0x4B, // Protocol "HTRK" + 0x00, 0x01, // Version 1 + // First ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize + 0x00, 0x02, // SrvCount (2 total) + 0x00, 0x01, // BatchSize (1 in first batch) + // ServerRecord 1 + 192, 168, 1, 1, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x01, // NumUsers 1 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'A', // Name + 0x01, // DescriptionSize + '1', // Description + // Incomplete second ServerInfoHeader + 0x00, 0x01, // MsgType only + }), + writeBuffer: &bytes.Buffer{}, + }, + wantErr: true, + wantResult: nil, + }, + { + name: "Empty server list", + mockConn: &mockConn{ + readBuffer: bytes.NewBuffer([]byte{ + // TrackerHeader + 0x48, 0x54, 0x52, 0x4B, // Protocol "HTRK" + 0x00, 0x01, // Version 1 + // ServerInfoHeader with 0 servers + 0x00, 0x01, // MsgType (1) + 0x00, 0x00, // MsgDataSize (0) + 0x00, 0x00, // SrvCount (0) + 0x00, 0x00, // BatchSize (0) + }), + writeBuffer: &bytes.Buffer{}, + }, + wantErr: false, + wantResult: []ServerRecord{}, + }, } for _, tt := range tests { -- cgit From 8ddb9bb228389b198a76d6df21de005da4fad66b Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Tue, 25 Nov 2025 10:30:30 -0800 Subject: Add TLS port field to tracker registration protocol Repurpose the previously unused 2-byte field in the tracker protocol to advertise the server's TLS port. This allows clients to discover which servers support TLS connections. Note: currently only the official, original, 1990's Tracker software sends this server provided 2-byte field to tracker clients. I'm adding this to Mobius in the hope that the modern day trackers might update their behavior to match the original software, which would put these unused 2 bytes to a useful purpose. --- hotline/server.go | 1 + hotline/tracker.go | 9 ++--- hotline/tracker_test.go | 90 ++++++++++++++++++++++++------------------------- 3 files changed, 51 insertions(+), 49 deletions(-) (limited to 'hotline') diff --git a/hotline/server.go b/hotline/server.go index 2c25e93..9b74d99 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -368,6 +368,7 @@ func (s *Server) registerWithAllTrackers() { Description: s.Config.Description, } binary.BigEndian.PutUint16(tr.Port[:], uint16(s.Port)) + binary.BigEndian.PutUint16(tr.TLSPort[:], uint16(s.TLSPort)) tr.Password = parseTrackerPassword(t) diff --git a/hotline/tracker.go b/hotline/tracker.go index bfba69d..ee4fc05 100644 --- a/hotline/tracker.go +++ b/hotline/tracker.go @@ -15,6 +15,7 @@ import ( type TrackerRegistration struct { Port [2]byte // Server's listening TCP port number UserCount int // Number of users connected to this particular server + TLSPort [2]byte // TLSPort PassID [4]byte // Random number generated by the server Name string // Server Name Description string // Description of the server @@ -32,7 +33,7 @@ func (tr *TrackerRegistration) Read(p []byte) (int, error) { []byte{0x00, 0x01}, // Magic number, always 1 tr.Port[:], userCount, - []byte{0x00, 0x00}, // Magic number, always 0 + tr.TLSPort[:], tr.PassID[:], []byte{uint8(len(tr.Name))}, []byte(tr.Name), @@ -114,7 +115,7 @@ type ServerRecord struct { IPAddr [4]byte Port [2]byte NumUsers [2]byte // Number of users connected to this particular server - Unused [2]byte + TLSPort [2]byte NameSize byte // Length of Name string Name []byte // Server Name DescriptionSize byte @@ -182,7 +183,7 @@ func GetListing(conn io.ReadWriteCloser) ([]ServerRecord, error) { func readServerRecord(reader *bufio.Reader) (ServerRecord, error) { var srv ServerRecord - // Read fixed header: IP (4) + Port (2) + NumUsers (2) + Unused (2) = 10 bytes + // Read fixed header: IP (4) + Port (2) + NumUsers (2) + TLSPort (2) = 10 bytes header := make([]byte, 10) if _, err := io.ReadFull(reader, header); err != nil { return srv, fmt.Errorf("failed to read server header: %w", err) @@ -191,7 +192,7 @@ func readServerRecord(reader *bufio.Reader) (ServerRecord, error) { copy(srv.IPAddr[:], header[0:4]) copy(srv.Port[:], header[4:6]) copy(srv.NumUsers[:], header[6:8]) - copy(srv.Unused[:], header[8:10]) + copy(srv.TLSPort[:], header[8:10]) // Read name size nameSizeByte, err := reader.ReadByte() diff --git a/hotline/tracker_test.go b/hotline/tracker_test.go index 8295582..242deb1 100644 --- a/hotline/tracker_test.go +++ b/hotline/tracker_test.go @@ -104,7 +104,7 @@ func TestGetListing(t *testing.T) { 192, 168, 1, 1, // IP address 0x1F, 0x90, // Port 8080 0x00, 0x10, // NumUsers 16 - 0x00, 0x00, // Unused + 0x00, 0x00, // TLSPort 0x04, // NameSize 'S', 'e', 'r', 'v', // Name 0x0B, // DescriptionSize @@ -113,7 +113,7 @@ func TestGetListing(t *testing.T) { 10, 0, 0, 1, // IP address 0x1F, 0x91, // Port 8081 0x00, 0x05, // NumUsers 5 - 0x00, 0x00, // Unused + 0x00, 0x00, // TLSPort 0x04, // NameSize 'S', 'e', 'r', 'v', // Name 0x0B, // DescriptionSize @@ -127,7 +127,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{192, 168, 1, 1}, Port: [2]byte{0x1F, 0x90}, NumUsers: [2]byte{0x00, 0x10}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 4, Name: []byte("Serv"), DescriptionSize: 11, @@ -137,7 +137,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{10, 0, 0, 1}, Port: [2]byte{0x1F, 0x91}, NumUsers: [2]byte{0x00, 0x05}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 4, Name: []byte("Serv"), DescriptionSize: 11, @@ -217,19 +217,19 @@ func TestGetListing(t *testing.T) { 192, 168, 1, 1, // IP address 0x1F, 0x90, // Port 8080 0x00, 0x0A, // NumUsers 10 - 0x00, 0x00, // Unused - 0x07, // NameSize + 0x00, 0x00, // TLSPort + 0x07, // NameSize 'S', 'e', 'r', 'v', 'e', 'r', '1', // Name - 0x0C, // DescriptionSize + 0x0C, // DescriptionSize 'F', 'i', 'r', 's', 't', ' ', 'b', 'a', 't', 'c', 'h', '1', // Description // ServerRecord 2 192, 168, 1, 2, // IP address 0x1F, 0x91, // Port 8081 0x00, 0x14, // NumUsers 20 - 0x00, 0x00, // Unused - 0x07, // NameSize + 0x00, 0x00, // TLSPort + 0x07, // NameSize 'S', 'e', 'r', 'v', 'e', 'r', '2', // Name - 0x0C, // DescriptionSize + 0x0C, // DescriptionSize 'F', 'i', 'r', 's', 't', ' ', 'b', 'a', 't', 'c', 'h', '2', // Description // Second ServerInfoHeader (next batch) 0x00, 0x01, // MsgType (1) @@ -240,10 +240,10 @@ func TestGetListing(t *testing.T) { 192, 168, 1, 3, // IP address 0x1F, 0x92, // Port 8082 0x00, 0x1E, // NumUsers 30 - 0x00, 0x00, // Unused - 0x07, // NameSize + 0x00, 0x00, // TLSPort + 0x07, // NameSize 'S', 'e', 'r', 'v', 'e', 'r', '3', // Name - 0x0D, // DescriptionSize + 0x0D, // DescriptionSize 'S', 'e', 'c', 'o', 'n', 'd', ' ', 'b', 'a', 't', 'c', 'h', '1', // Description }), writeBuffer: &bytes.Buffer{}, @@ -254,7 +254,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{192, 168, 1, 1}, Port: [2]byte{0x1F, 0x90}, NumUsers: [2]byte{0x00, 0x0A}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 7, Name: []byte("Server1"), DescriptionSize: 12, @@ -264,7 +264,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{192, 168, 1, 2}, Port: [2]byte{0x1F, 0x91}, NumUsers: [2]byte{0x00, 0x14}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 7, Name: []byte("Server2"), DescriptionSize: 12, @@ -274,7 +274,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{192, 168, 1, 3}, Port: [2]byte{0x1F, 0x92}, NumUsers: [2]byte{0x00, 0x1E}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 7, Name: []byte("Server3"), DescriptionSize: 13, @@ -298,20 +298,20 @@ func TestGetListing(t *testing.T) { 192, 168, 1, 1, // IP 0x15, 0x7c, // Port 5500 0x00, 0x01, // NumUsers 1 - 0x00, 0x00, // Unused - 0x01, // NameSize - 'A', // Name - 0x01, // DescriptionSize - '1', // Description + 0x00, 0x00, // TLSPort + 0x01, // NameSize + 'A', // Name + 0x01, // DescriptionSize + '1', // Description // ServerRecord 2 192, 168, 1, 2, // IP 0x15, 0x7c, // Port 5500 0x00, 0x02, // NumUsers 2 - 0x00, 0x00, // Unused - 0x01, // NameSize - 'B', // Name - 0x01, // DescriptionSize - '2', // Description + 0x00, 0x00, // TLSPort + 0x01, // NameSize + 'B', // Name + 0x01, // DescriptionSize + '2', // Description // Second ServerInfoHeader 0x00, 0x01, // MsgType (1) 0x00, 0x0A, // MsgDataSize @@ -321,11 +321,11 @@ func TestGetListing(t *testing.T) { 192, 168, 1, 3, // IP 0x15, 0x7c, // Port 5500 0x00, 0x03, // NumUsers 3 - 0x00, 0x00, // Unused - 0x01, // NameSize - 'C', // Name - 0x01, // DescriptionSize - '3', // Description + 0x00, 0x00, // TLSPort + 0x01, // NameSize + 'C', // Name + 0x01, // DescriptionSize + '3', // Description // Third ServerInfoHeader 0x00, 0x01, // MsgType (1) 0x00, 0x0A, // MsgDataSize @@ -335,11 +335,11 @@ func TestGetListing(t *testing.T) { 192, 168, 1, 4, // IP 0x15, 0x7c, // Port 5500 0x00, 0x04, // NumUsers 4 - 0x00, 0x00, // Unused - 0x01, // NameSize - 'D', // Name - 0x01, // DescriptionSize - '4', // Description + 0x00, 0x00, // TLSPort + 0x01, // NameSize + 'D', // Name + 0x01, // DescriptionSize + '4', // Description }), writeBuffer: &bytes.Buffer{}, }, @@ -349,7 +349,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{192, 168, 1, 1}, Port: [2]byte{0x15, 0x7c}, NumUsers: [2]byte{0x00, 0x01}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 1, Name: []byte("A"), DescriptionSize: 1, @@ -359,7 +359,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{192, 168, 1, 2}, Port: [2]byte{0x15, 0x7c}, NumUsers: [2]byte{0x00, 0x02}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 1, Name: []byte("B"), DescriptionSize: 1, @@ -369,7 +369,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{192, 168, 1, 3}, Port: [2]byte{0x15, 0x7c}, NumUsers: [2]byte{0x00, 0x03}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 1, Name: []byte("C"), DescriptionSize: 1, @@ -379,7 +379,7 @@ func TestGetListing(t *testing.T) { IPAddr: [4]byte{192, 168, 1, 4}, Port: [2]byte{0x15, 0x7c}, NumUsers: [2]byte{0x00, 0x04}, - Unused: [2]byte{0x00, 0x00}, + TLSPort: [2]byte{0x00, 0x00}, NameSize: 1, Name: []byte("D"), DescriptionSize: 1, @@ -403,11 +403,11 @@ func TestGetListing(t *testing.T) { 192, 168, 1, 1, // IP 0x15, 0x7c, // Port 5500 0x00, 0x01, // NumUsers 1 - 0x00, 0x00, // Unused - 0x01, // NameSize - 'A', // Name - 0x01, // DescriptionSize - '1', // Description + 0x00, 0x00, // TLSPort + 0x01, // NameSize + 'A', // Name + 0x01, // DescriptionSize + '1', // Description // Incomplete second ServerInfoHeader 0x00, 0x01, // MsgType only }), -- cgit