From a202228b69d66a901f54650a38142d21eeac623c Mon Sep 17 00:00:00 2001 From: Vlad Date: Tue, 6 Aug 2024 16:44:45 +0300 Subject: [PATCH 01/10] fix panic on failed fs.LoadFs() --- server/server.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/server.go b/server/server.go index be48466c..36bc33e8 100644 --- a/server/server.go +++ b/server/server.go @@ -158,6 +158,9 @@ func (s *Server) loadFs(access *confpar.Access) (afero.Fs, error) { } newFs, err := fs.LoadFs(access, s.logger) + if err != nil { + return nil, err + } if access.Shared { s.logger.Debug("Saving fs instance for later use", "user", access.User, "fsType", newFs.Name()) cache.accesses[access.User] = newFs From ac5e96b355c1f587977fada3e1f8ae3d55b70344 Mon Sep 17 00:00:00 2001 From: Vlad Date: Tue, 14 Oct 2025 21:36:55 +0300 Subject: [PATCH 02/10] Add comprehensive tests and improve error handling for telegram filesystem - Add telegram_test.go with 640 lines of unit tests covering: * Fake filesystem operations and concurrency * File read/write operations with thread safety * File extension matching * Size limits and validation * Error conditions and edge cases - Improve error handling: * Use os.ErrNotExist consistently in Stat/LstatIfPossible * Add ErrFileTooLarge for size limit violations * Add UTF-8 validation for text files - Add safety features: * File size validation (50MB Telegram limit) * Message size constant (4096 chars) * Directory size constant (4096 bytes) * Mutex protection for File read/write operations - Code cleanup: * Remove commented-out code in fake_fs.go * Add security warnings to bot command handlers * Improve imports organization * Add graceful bot shutdown with Stop() method --- fs/telegram/fake_fs.go | 3 - fs/telegram/file_info.go | 5 +- fs/telegram/telegram.go | 96 ++++-- fs/telegram/telegram_test.go | 640 +++++++++++++++++++++++++++++++++++ 4 files changed, 717 insertions(+), 27 deletions(-) create mode 100644 fs/telegram/telegram_test.go diff --git a/fs/telegram/fake_fs.go b/fs/telegram/fake_fs.go index bb1ac828..57400671 100644 --- a/fs/telegram/fake_fs.go +++ b/fs/telegram/fake_fs.go @@ -10,15 +10,12 @@ import ( type fakeFilesystem struct { sync.Mutex dict map[string]*FileInfo - // dir fakeDir } - // newFakeFilesystem creates a new fake filesystem func newFakeFilesystem() *fakeFilesystem { return &fakeFilesystem{ dict: map[string]*FileInfo{}, - // dir: fakeDir{content: []os.FileInfo{}}, } } diff --git a/fs/telegram/file_info.go b/fs/telegram/file_info.go index b6d30fdc..a2eecd48 100644 --- a/fs/telegram/file_info.go +++ b/fs/telegram/file_info.go @@ -6,6 +6,9 @@ import ( "time" ) +// defaultDirSize is a standard directory size (4096 bytes, typical for filesystems) +const defaultDirSize = 4096 + // FileData is a simple structure to store file information and implement os.FileInfo interface type FileData struct { name string @@ -31,7 +34,7 @@ func (s *FileInfo) IsDir() bool { return s.dir } func (s *FileInfo) Sys() interface{} { return nil } func (s *FileInfo) Size() int64 { if s.IsDir() { - return int64(42) + return defaultDirSize } return s.size } diff --git a/fs/telegram/telegram.go b/fs/telegram/telegram.go index 1a55f974..519d2eab 100644 --- a/fs/telegram/telegram.go +++ b/fs/telegram/telegram.go @@ -5,21 +5,20 @@ import ( "errors" "fmt" "io" + "os" "path/filepath" "strconv" "strings" - - "os" - "sync/atomic" + "sync" "time" + "unicode/utf8" log "github.com/fclairamb/go-log" - tele "gopkg.in/telebot.v3" - "github.com/spf13/afero" + tele "gopkg.in/telebot.v3" + "gopkg.in/telebot.v3/middleware" "github.com/fclairamb/ftpserver/config/confpar" - "gopkg.in/telebot.v3/middleware" ) // ErrNotImplemented is returned when something is not implemented @@ -31,6 +30,16 @@ var ErrNotFound = errors.New("not found") // ErrInvalidParameter is returned when a parameter is invalid var ErrInvalidParameter = errors.New("invalid parameter") +// ErrFileTooLarge is returned when a file exceeds Telegram's size limit +var ErrFileTooLarge = errors.New("file size exceeds Telegram limit") + +const ( + // maxFileSize is Telegram's file size limit (50MB) + maxFileSize = 50 * 1024 * 1024 + // maxTextSize is Telegram's message size limit (4096 characters) + maxTextSize = 4096 +) + // Fs is a write-only afero.Fs implementation using telegram as backend type Fs struct { // Bot is the telegram bot instance @@ -43,6 +52,9 @@ type Fs struct { // fakeFs is a lightweight fake filesystem intended for store temporary info about files // since some ftp clients expect to perform mkdir() + stat() on files and directories before upload fakeFs *fakeFilesystem + + // stopChan is used to signal bot shutdown + stopChan chan struct{} } // File is the afero.File implementation @@ -55,6 +67,8 @@ type File struct { Fs *Fs // At is the current position in the file At int64 + // mu protects Content and At from concurrent access + mu sync.Mutex } // imageExtensions is the list of supported image extensions @@ -98,18 +112,21 @@ func LoadFs(access *confpar.Access, logger log.Logger) (afero.Fs, error) { bot.Handle("/start", startHandler) bot.Handle("/help", helpHandler) + fs := &Fs{ + Bot: bot, + Logger: logger, + ChatID: chatID, + fakeFs: newFakeFilesystem(), + stopChan: make(chan struct{}), + } + go func() { // Run bot in the background + logger.Info("Starting telegram bot") bot.Start() + logger.Info("Telegram bot stopped") }() - fs := &Fs{ - Bot: bot, - Logger: logger, - ChatID: chatID, - fakeFs: newFakeFilesystem(), - } - return fs, nil } @@ -135,8 +152,15 @@ func (f *File) Close() error { } else if isExtension(f.Path, audioExtensions) { audio := tele.Audio{File: tele.FromReader(f), Caption: basePath} _, err = f.Fs.Bot.Send(&chat, &audio) - } else if isExtension(f.Path, textExtensions) && len(f.Content) < 4096 { - if isExtension(f.Path, []string{".md"}) { + } else if isExtension(f.Path, textExtensions) && len(f.Content) < maxTextSize { + // Validate UTF-8 for text files + if !utf8.Valid(f.Content) { + f.Fs.Logger.Warn("Invalid UTF-8 in text file, sending as document", "path", f.Path) + document := tele.Document{File: tele.FromReader(f), Caption: basePath} + document.FileName = basePath + document.FileLocal = basePath + _, err = f.Fs.Bot.Send(&chat, &document) + } else if isExtension(f.Path, []string{".md"}) { _, err = f.Fs.Bot.Send(&chat, string(f.Content), tele.ModeMarkdown) } else { _, err = f.Fs.Bot.Send(&chat, string(f.Content)) @@ -165,20 +189,20 @@ func (f *File) Close() error { // Read stores the received file content into the local buffer func (f *File) Read(b []byte) (int, error) { - n := 0 + f.mu.Lock() + defer f.mu.Unlock() if len(b) > 0 && int(f.At) == len(f.Content) { return 0, io.EOF } - if len(f.Content)-int(f.At) >= len(b) { - n = len(b) - } else { + n := len(b) + if len(f.Content)-int(f.At) < n { n = len(f.Content) - int(f.At) } copy(b, f.Content[f.At:f.At+int64(n)]) - atomic.AddInt64(&f.At, int64(n)) + f.At += int64(n) return n, nil } @@ -213,7 +237,7 @@ func (f *File) Stat() (os.FileInfo, error) { fileInfo := f.Fs.fakeFs.stat(f.Path) if fileInfo == nil { - return nil, &os.PathError{Op: "stat", Path: f.Path, Err: nil} + return nil, &os.PathError{Op: "stat", Path: f.Path, Err: os.ErrNotExist} } return fileInfo, nil } @@ -234,6 +258,15 @@ func (f *File) WriteAt(b []byte, off int64) (int, error) { } func (f *File) Write(b []byte) (int, error) { + f.mu.Lock() + defer f.mu.Unlock() + + // Check if writing would exceed Telegram's file size limit + newSize := int64(len(f.Content)) + int64(len(b)) + if newSize > maxFileSize { + return 0, fmt.Errorf("%w: attempted size %d bytes, limit is %d bytes", ErrFileTooLarge, newSize, maxFileSize) + } + f.Content = append(f.Content, b...) return len(b), nil @@ -244,6 +277,16 @@ func (m *Fs) Name() string { return "telegram" } +// Stop gracefully shuts down the telegram bot +func (m *Fs) Stop() error { + if m.Bot != nil { + m.Logger.Info("Stopping telegram bot") + m.Bot.Stop() + close(m.stopChan) + } + return nil +} + // Chtimes is not implemented func (m *Fs) Chtimes(name string, atime, mtime time.Time) error { return nil @@ -312,14 +355,14 @@ func (m *Fs) Stat(name string) (os.FileInfo, error) { fileInfo := m.fakeFs.stat(name) if fileInfo == nil { - return nil, &os.PathError{Op: "stat", Path: name, Err: nil} + return nil, &os.PathError{Op: "stat", Path: name, Err: os.ErrNotExist} } return fileInfo, nil } // LstatIfPossible is not implemented func (m *Fs) LstatIfPossible(name string) (os.FileInfo, bool, error) { - return nil, false, &os.PathError{Op: "lstat", Path: name, Err: nil} + return nil, false, &os.PathError{Op: "lstat", Path: name, Err: os.ErrNotExist} } func isExtension(filename string, extensions []string) bool { @@ -335,6 +378,8 @@ func isExtension(filename string, extensions []string) bool { const readMeURL = "https://github.com/slayer/ftpserver" // /start command handler +// Note: This handler responds to any user. For production use, consider adding +// authorization checks to restrict access to specific chat IDs only. func startHandler(c tele.Context) error { err := helpHandler(c) if err != nil { @@ -346,10 +391,15 @@ func startHandler(c tele.Context) error { chatID = chat.ID } + // Warning: This reveals the chat ID to any user who sends /start + // In a production environment, consider restricting this information err = c.Send(fmt.Sprintf("Current `ChatID` is `%d`", chatID), tele.ModeMarkdown) return err } +// helpHandler responds to /help command +// Note: This handler responds to any user. For production use, consider adding +// authorization checks to restrict access to specific chat IDs only. func helpHandler(c tele.Context) error { firstName := "" if c.Sender() != nil { diff --git a/fs/telegram/telegram_test.go b/fs/telegram/telegram_test.go new file mode 100644 index 00000000..cd13892c --- /dev/null +++ b/fs/telegram/telegram_test.go @@ -0,0 +1,640 @@ +package telegram + +import ( + "errors" + "io" + "os" + "sync" + "testing" + "time" +) + +// TestFakeFilesystem tests the in-memory fake filesystem +func TestFakeFilesystem(t *testing.T) { + fs := newFakeFilesystem() + + // Test mkdir + fs.mkdir("/test", 0755) + info := fs.stat("/test") + if info == nil { + t.Fatal("Expected directory to exist") + } + if !info.IsDir() { + t.Error("Expected /test to be a directory") + } + if info.Mode() != 0755 { + t.Errorf("Expected mode 0755, got %v", info.Mode()) + } + + // Test create + fs.create("/test/file.txt") + info = fs.stat("/test/file.txt") + if info == nil { + t.Fatal("Expected file to exist") + } + if info.IsDir() { + t.Error("Expected /test/file.txt to be a file") + } + + // Test setSize + fs.setSize("/test/file.txt", 1234) + info = fs.stat("/test/file.txt") + if info.Size() != 1234 { + t.Errorf("Expected size 1234, got %d", info.Size()) + } + + // Test non-existent file + info = fs.stat("/nonexistent") + if info != nil { + t.Error("Expected nil for non-existent file") + } +} + +// TestFakeFilesystemConcurrency tests thread safety of fake filesystem +func TestFakeFilesystemConcurrency(t *testing.T) { + fs := newFakeFilesystem() + var wg sync.WaitGroup + + // Create multiple goroutines to test concurrent access + for i := 0; i < 100; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + path := "/concurrent/file" + fs.mkdir("/concurrent", 0755) + fs.create(path) + fs.setSize(path, int64(n)) + _ = fs.stat(path) + }(i) + } + + wg.Wait() +} + +// TestFileInfo tests the FileInfo implementation +func TestFileInfo(t *testing.T) { + // Test directory + dirInfo := &FileInfo{&FileData{ + name: "/path/to/dir", + dir: true, + mode: 0755, + }} + + if dirInfo.Name() != "dir" { + t.Errorf("Expected name 'dir', got '%s'", dirInfo.Name()) + } + if !dirInfo.IsDir() { + t.Error("Expected IsDir to be true") + } + if dirInfo.Mode() != 0755 { + t.Errorf("Expected mode 0755, got %v", dirInfo.Mode()) + } + // Directory size is returned as a constant value (42 in the current implementation) + if !dirInfo.IsDir() || dirInfo.Size() <= 0 { + t.Errorf("Expected positive dir size, got %d", dirInfo.Size()) + } + if dirInfo.Sys() != nil { + t.Error("Expected Sys() to return nil") + } + + // Test file + fileInfo := &FileInfo{&FileData{ + name: "/path/to/file.txt", + dir: false, + mode: 0644, + size: 5678, + }} + + if fileInfo.Name() != "file.txt" { + t.Errorf("Expected name 'file.txt', got '%s'", fileInfo.Name()) + } + if fileInfo.IsDir() { + t.Error("Expected IsDir to be false") + } + if fileInfo.Size() != 5678 { + t.Errorf("Expected size 5678, got %d", fileInfo.Size()) + } +} + +// TestIsExtension tests the file extension matching +func TestIsExtension(t *testing.T) { + tests := []struct { + filename string + extensions []string + expected bool + }{ + {"test.jpg", imageExtensions, true}, + {"test.JPG", imageExtensions, true}, + {"test.jpeg", imageExtensions, true}, + {"TEST.PNG", imageExtensions, true}, + {"test.txt", imageExtensions, false}, + {"test.mp4", videoExtensions, true}, + {"test.MP4", videoExtensions, true}, + {"test.avi", videoExtensions, true}, + {"test.jpg", videoExtensions, false}, + {"test.mp3", audioExtensions, true}, + {"test.ogg", audioExtensions, true}, + {"test.txt", textExtensions, true}, + {"test.md", textExtensions, true}, + {"test", imageExtensions, false}, + {"", imageExtensions, false}, + } + + for _, tt := range tests { + result := isExtension(tt.filename, tt.extensions) + if result != tt.expected { + t.Errorf("isExtension(%q, extensions) = %v, want %v", tt.filename, result, tt.expected) + } + } +} + +// TestFileReadWrite tests concurrent read/write operations on File +func TestFileReadWrite(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + file := &File{ + Path: "/test.txt", + Fs: fs, + } + + // Test Write + data := []byte("Hello, World!") + n, err := file.Write(data) + if err != nil { + t.Fatalf("Write failed: %v", err) + } + if n != len(data) { + t.Errorf("Expected to write %d bytes, wrote %d", len(data), n) + } + + // Test Read + buf := make([]byte, 5) + n, err = file.Read(buf) + if err != nil { + t.Fatalf("Read failed: %v", err) + } + if n != 5 { + t.Errorf("Expected to read 5 bytes, read %d", n) + } + if string(buf) != "Hello" { + t.Errorf("Expected 'Hello', got '%s'", string(buf)) + } + + // Test Read remainder + buf = make([]byte, 20) + n, err = file.Read(buf) + if err != nil { + t.Fatalf("Read failed: %v", err) + } + if n != 8 { + t.Errorf("Expected to read 8 bytes, read %d", n) + } + if string(buf[:n]) != ", World!" { + t.Errorf("Expected ', World!', got '%s'", string(buf[:n])) + } + + // Test EOF + n, err = file.Read(buf) + if err != io.EOF { + t.Errorf("Expected EOF, got %v", err) + } + if n != 0 { + t.Errorf("Expected 0 bytes at EOF, got %d", n) + } +} + +// TestFileReadWriteConcurrency tests thread safety of File operations +func TestFileReadWriteConcurrency(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + file := &File{ + Path: "/test.txt", + Fs: fs, + } + + var wg sync.WaitGroup + + // Concurrent writes + for i := 0; i < 10; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + data := []byte{byte(n)} + _, _ = file.Write(data) + }(i) + } + + wg.Wait() + + // Verify we wrote 10 bytes + if len(file.Content) != 10 { + t.Errorf("Expected 10 bytes written, got %d", len(file.Content)) + } +} + +// TestFileReadEmptyContent tests reading from empty file +func TestFileReadEmptyContent(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + file := &File{ + Path: "/empty.txt", + Fs: fs, + } + + buf := make([]byte, 10) + n, err := file.Read(buf) + if err != io.EOF { + t.Errorf("Expected EOF for empty file, got %v", err) + } + if n != 0 { + t.Errorf("Expected 0 bytes read, got %d", n) + } +} + +// TestFileStat tests File.Stat() method +func TestFileStat(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + // Create file in fake filesystem + fs.fakeFs.create("/test.txt") + fs.fakeFs.setSize("/test.txt", 100) + + file := &File{ + Path: "/test.txt", + Fs: fs, + } + + info, err := file.Stat() + if err != nil { + t.Fatalf("Stat failed: %v", err) + } + if info.Name() != "test.txt" { + t.Errorf("Expected name 'test.txt', got '%s'", info.Name()) + } + if info.Size() != 100 { + t.Errorf("Expected size 100, got %d", info.Size()) + } + + // Test Stat on non-existent file + file2 := &File{ + Path: "/nonexistent.txt", + Fs: fs, + } + + _, err = file2.Stat() + if err == nil { + t.Error("Expected error for non-existent file") + } + if !os.IsNotExist(err) { + t.Errorf("Expected os.ErrNotExist, got %v", err) + } +} + +// TestFsName tests filesystem name +func TestFsName(t *testing.T) { + fs := &Fs{} + if fs.Name() != "telegram" { + t.Errorf("Expected name 'telegram', got '%s'", fs.Name()) + } +} + +// TestFsMkdir tests Mkdir functionality +func TestFsMkdir(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + err := fs.Mkdir("/testdir", 0755) + if err != nil { + t.Fatalf("Mkdir failed: %v", err) + } + + info := fs.fakeFs.stat("/testdir") + if info == nil { + t.Fatal("Expected directory to exist") + } + if !info.IsDir() { + t.Error("Expected /testdir to be a directory") + } +} + +// TestFsMkdirAll tests MkdirAll functionality +func TestFsMkdirAll(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + err := fs.MkdirAll("/path/to/deep/dir", 0755) + if err != nil { + t.Fatalf("MkdirAll failed: %v", err) + } + + // Check all directories exist + paths := []string{"/path", "/path/to", "/path/to/deep", "/path/to/deep/dir"} + for _, path := range paths { + info := fs.fakeFs.stat(path) + if info == nil { + t.Errorf("Expected directory %s to exist", path) + } + } +} + +// TestFsCreate tests Create functionality +func TestFsCreate(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + file, err := fs.Create("/test.txt") + if err != nil { + t.Fatalf("Create failed: %v", err) + } + if file == nil { + t.Fatal("Expected file to be non-nil") + } + + info := fs.fakeFs.stat("/test.txt") + if info == nil { + t.Error("Expected file to exist in fake filesystem") + } +} + +// TestFsStat tests Stat functionality +func TestFsStat(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + // Test stat on non-existent file + _, err := fs.Stat("/nonexistent") + if err == nil { + t.Error("Expected error for non-existent file") + } + if !os.IsNotExist(err) { + t.Errorf("Expected os.ErrNotExist, got %v", err) + } + + // Create and stat a file + fs.fakeFs.create("/test.txt") + fs.fakeFs.setSize("/test.txt", 42) + + info, err := fs.Stat("/test.txt") + if err != nil { + t.Fatalf("Stat failed: %v", err) + } + if info.Name() != "test.txt" { + t.Errorf("Expected name 'test.txt', got '%s'", info.Name()) + } + if info.Size() != 42 { + t.Errorf("Expected size 42, got %d", info.Size()) + } +} + +// TestFsLstatIfPossible tests LstatIfPossible functionality +func TestFsLstatIfPossible(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + _, supported, err := fs.LstatIfPossible("/test") + if err == nil { + t.Error("Expected error for LstatIfPossible") + } + if supported { + t.Error("Expected LstatIfPossible to not be supported") + } + if !os.IsNotExist(err) { + t.Errorf("Expected os.ErrNotExist, got %v", err) + } +} + +// TestFileNotImplementedMethods tests methods that return not implemented +func TestFileNotImplementedMethods(t *testing.T) { + file := &File{} + + // ReadAt + _, err := file.ReadAt(nil, 0) + if err != ErrNotImplemented { + t.Errorf("Expected ErrNotImplemented, got %v", err) + } + + // WriteString + _, err = file.WriteString("test") + if err != ErrNotImplemented { + t.Errorf("Expected ErrNotImplemented, got %v", err) + } + + // WriteAt + _, err = file.WriteAt(nil, 0) + if err != ErrNotImplemented { + t.Errorf("Expected ErrNotImplemented, got %v", err) + } +} + +// TestFileNoOpMethods tests methods that are no-ops +func TestFileNoOpMethods(t *testing.T) { + file := &File{} + + // Truncate + err := file.Truncate(0) + if err != nil { + t.Errorf("Truncate should return nil, got %v", err) + } + + // Sync + err = file.Sync() + if err != nil { + t.Errorf("Sync should return nil, got %v", err) + } + + // Seek + n, err := file.Seek(0, 0) + if err != nil { + t.Errorf("Seek should return nil error, got %v", err) + } + if n != 0 { + t.Errorf("Seek should return 0, got %d", n) + } + + // Readdir + infos, err := file.Readdir(0) + if err != nil { + t.Errorf("Readdir should return nil error, got %v", err) + } + if len(infos) != 0 { + t.Errorf("Readdir should return empty slice, got %d items", len(infos)) + } + + // Readdirnames + names, err := file.Readdirnames(0) + if err != nil { + t.Errorf("Readdirnames should return nil error, got %v", err) + } + if len(names) != 0 { + t.Errorf("Readdirnames should return empty slice, got %d items", len(names)) + } +} + +// TestFsNoOpMethods tests filesystem methods that are no-ops +func TestFsNoOpMethods(t *testing.T) { + fs := &Fs{} + + // Chtimes + err := fs.Chtimes("/test", time.Now(), time.Now()) + if err != nil { + t.Errorf("Chtimes should return nil, got %v", err) + } + + // Chmod + err = fs.Chmod("/test", 0755) + if err != nil { + t.Errorf("Chmod should return nil, got %v", err) + } + + // Rename + err = fs.Rename("/old", "/new") + if err != nil { + t.Errorf("Rename should return nil, got %v", err) + } + + // Chown + err = fs.Chown("/test", 0, 0) + if err != nil { + t.Errorf("Chown should return nil, got %v", err) + } + + // RemoveAll + err = fs.RemoveAll("/test") + if err != nil { + t.Errorf("RemoveAll should return nil, got %v", err) + } + + // Remove + err = fs.Remove("/test") + if err != nil { + t.Errorf("Remove should return nil, got %v", err) + } +} + +// TestFsStop tests the Stop method +func TestFsStop(t *testing.T) { + // Test Stop with nil bot (should not panic and not close channel) + fs := &Fs{ + stopChan: make(chan struct{}), + Bot: nil, + } + + err := fs.Stop() + if err != nil { + t.Errorf("Stop should return nil, got %v", err) + } + + // With nil Bot, the channel should NOT be closed (this is intentional behavior) + select { + case <-fs.stopChan: + t.Error("Channel should not be closed when Bot is nil") + default: + // Good, channel is still open + } + + // Note: Testing with real Bot is not practical in unit tests + // as it would require valid Telegram credentials and network access +} + +// TestFileWriteSizeLimit tests that Write enforces the file size limit +func TestFileWriteSizeLimit(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + file := &File{ + Path: "/large.bin", + Fs: fs, + } + + // Write should succeed for data under the limit + smallData := make([]byte, 1024) + n, err := file.Write(smallData) + if err != nil { + t.Fatalf("Write of small data failed: %v", err) + } + if n != len(smallData) { + t.Errorf("Expected to write %d bytes, wrote %d", len(smallData), n) + } + + // Try to write data that would exceed the limit + // maxFileSize is 50MB, so create data that would push us over + largeData := make([]byte, maxFileSize) + n, err = file.Write(largeData) + if err == nil { + t.Error("Expected error when exceeding file size limit") + } + if n != 0 { + t.Errorf("Expected 0 bytes written on error, got %d", n) + } + if !errors.Is(err, ErrFileTooLarge) { + t.Errorf("Expected ErrFileTooLarge, got %v", err) + } +} + +// TestFileWriteSizeLimitExactly tests writing exactly at the limit +func TestFileWriteSizeLimitExactly(t *testing.T) { + fs := &Fs{ + fakeFs: newFakeFilesystem(), + } + + file := &File{ + Path: "/exact.bin", + Fs: fs, + } + + // Write exactly maxFileSize bytes (should succeed) + data := make([]byte, maxFileSize) + n, err := file.Write(data) + if err != nil { + t.Fatalf("Write of exactly max size failed: %v", err) + } + if n != len(data) { + t.Errorf("Expected to write %d bytes, wrote %d", len(data), n) + } + + // Try to write one more byte (should fail) + n, err = file.Write([]byte{0}) + if err == nil { + t.Error("Expected error when exceeding file size limit by 1 byte") + } + if !errors.Is(err, ErrFileTooLarge) { + t.Errorf("Expected ErrFileTooLarge, got %v", err) + } +} + +// TestConstants tests that constants are defined correctly +func TestConstants(t *testing.T) { + if maxFileSize != 50*1024*1024 { + t.Errorf("Expected maxFileSize to be 50MB, got %d", maxFileSize) + } + if maxTextSize != 4096 { + t.Errorf("Expected maxTextSize to be 4096, got %d", maxTextSize) + } + if defaultDirSize != 4096 { + t.Errorf("Expected defaultDirSize to be 4096, got %d", defaultDirSize) + } +} + +// TestErrFileTooLarge tests the error type +func TestErrFileTooLarge(t *testing.T) { + if ErrFileTooLarge == nil { + t.Error("ErrFileTooLarge should not be nil") + } + if ErrFileTooLarge.Error() == "" { + t.Error("ErrFileTooLarge should have an error message") + } +} From 6663f94c6eb5d1217a25e05ada39c48d57f5082f Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 15 Oct 2025 00:14:41 +0300 Subject: [PATCH 03/10] telegram testing Test Files Created: 1. test_helpers.go (210 lines) - Test infrastructure including: - testLogger - Full logging implementation for tests - MockBot - Mock Telegram bot for testing (prepared for future use) - Helper functions: createTestBot(), createTestAccess(), createTestAccessWithToken() 2. telegram_integration_test.go (392 lines) - Integration tests with 13 new test functions: - LoadFs validation (empty token, invalid ChatID) - File extension detection (images, videos, audio, text, documents) - File size validation and limits - Text size limit enforcement - Filesystem operations - Multiple file creation - File read/write integration --- fs/telegram/telegram_integration_test.go | 391 +++++++++++++++++++++++ fs/telegram/test_helpers.go | 209 ++++++++++++ 2 files changed, 600 insertions(+) create mode 100644 fs/telegram/telegram_integration_test.go create mode 100644 fs/telegram/test_helpers.go diff --git a/fs/telegram/telegram_integration_test.go b/fs/telegram/telegram_integration_test.go new file mode 100644 index 00000000..1fa899e9 --- /dev/null +++ b/fs/telegram/telegram_integration_test.go @@ -0,0 +1,391 @@ +package telegram + +import ( + "errors" + "strings" + "testing" + + "github.com/fclairamb/ftpserver/config/confpar" +) + +// TestLoadFs_EmptyToken tests LoadFs with empty token +func TestLoadFs_EmptyToken(t *testing.T) { + logger := newTestLogger() + + access := &confpar.Access{ + Params: map[string]string{ + "Token": "", + "ChatID": "123456789", + }, + } + + _, err := LoadFs(access, logger) + if err == nil { + t.Error("Expected error with empty token") + } + if !strings.Contains(err.Error(), "Token is empty") { + t.Errorf("Expected 'Token is empty' error, got: %v", err) + } +} + +// TestLoadFs_InvalidChatID tests LoadFs with invalid ChatID +func TestLoadFs_InvalidChatID(t *testing.T) { + logger := newTestLogger() + + access := &confpar.Access{ + Params: map[string]string{ + "Token": "test-token", + "ChatID": "invalid-not-a-number", + }, + } + + _, err := LoadFs(access, logger) + if err == nil { + t.Error("Expected error with invalid ChatID") + } + if !strings.Contains(err.Error(), "invalid ChatID") { + t.Errorf("Expected 'invalid ChatID' error, got: %v", err) + } +} + +// TestFileExtensionDetection_Images tests image file type detection +func TestFileExtensionDetection_Images(t *testing.T) { + testCases := []string{ + "/photo.jpg", + "/image.jpeg", + "/picture.png", + "/animation.gif", + "/bitmap.bmp", + "/PHOTO.JPG", // Test case insensitivity + } + + for _, path := range testCases { + if !isExtension(path, imageExtensions) { + t.Errorf("File %s should be detected as image", path) + } + + // Verify it's not detected as other types + if isExtension(path, videoExtensions) { + t.Errorf("File %s should not be detected as video", path) + } + if isExtension(path, audioExtensions) { + t.Errorf("File %s should not be detected as audio", path) + } + } +} + +// TestFileExtensionDetection_Videos tests video file type detection +func TestFileExtensionDetection_Videos(t *testing.T) { + testCases := []string{ + "/movie.mp4", + "/clip.avi", + "/video.mkv", + "/recording.mov", + "/VIDEO.MP4", // Test case insensitivity + } + + for _, path := range testCases { + if !isExtension(path, videoExtensions) { + t.Errorf("File %s should be detected as video", path) + } + + // Verify it's not detected as other types + if isExtension(path, imageExtensions) { + t.Errorf("File %s should not be detected as image", path) + } + if isExtension(path, audioExtensions) { + t.Errorf("File %s should not be detected as audio", path) + } + } +} + +// TestFileExtensionDetection_Audio tests audio file type detection +func TestFileExtensionDetection_Audio(t *testing.T) { + testCases := []string{ + "/song.mp3", + "/audio.ogg", + "/music.flac", + "/recording.wav", + "/SONG.MP3", // Test case insensitivity + } + + for _, path := range testCases { + if !isExtension(path, audioExtensions) { + t.Errorf("File %s should be detected as audio", path) + } + + // Verify it's not detected as other types + if isExtension(path, imageExtensions) { + t.Errorf("File %s should not be detected as image", path) + } + if isExtension(path, videoExtensions) { + t.Errorf("File %s should not be detected as video", path) + } + } +} + +// TestFileExtensionDetection_Text tests text file type detection +func TestFileExtensionDetection_Text(t *testing.T) { + testCases := []string{ + "/readme.txt", + "/notes.md", + "/README.TXT", // Test case insensitivity + } + + for _, path := range testCases { + if !isExtension(path, textExtensions) { + t.Errorf("File %s should be detected as text", path) + } + } +} + +// TestFileExtensionDetection_Documents tests generic document detection +func TestFileExtensionDetection_Documents(t *testing.T) { + testCases := []string{ + "/document.pdf", + "/archive.zip", + "/data.json", + "/unknown.xyz", + "/noextension", + } + + for _, path := range testCases { + // These should not match any specific type + if isExtension(path, imageExtensions) { + t.Errorf("File %s should not be detected as image", path) + } + if isExtension(path, videoExtensions) { + t.Errorf("File %s should not be detected as video", path) + } + if isExtension(path, audioExtensions) { + t.Errorf("File %s should not be detected as audio", path) + } + if isExtension(path, textExtensions) { + t.Errorf("File %s should not be detected as text", path) + } + } +} + +// TestFileClose_NilFs tests error handling when Fs is nil +func TestFileClose_NilFs(t *testing.T) { + file := &File{ + Path: "/test.txt", + Content: []byte("test"), + Fs: nil, + } + + err := file.Close() + if err == nil { + t.Error("Expected error when Fs is nil") + } + if !errors.Is(err, ErrNotFound) { + t.Errorf("Expected ErrNotFound, got: %v", err) + } +} + +// TestFileSizeValidation tests file size limit enforcement +func TestFileSizeValidation(t *testing.T) { + logger := newTestLogger() + + fs := &Fs{ + Bot: nil, + Logger: logger, + ChatID: 123456789, + fakeFs: newFakeFilesystem(), + stopChan: make(chan struct{}), + } + + // Test small file (should succeed) + file := &File{ + Path: "/small.bin", + Fs: fs, + } + + smallData := make([]byte, 1024) + n, err := file.Write(smallData) + if err != nil { + t.Fatalf("Write of small data failed: %v", err) + } + if n != len(smallData) { + t.Errorf("Expected to write %d bytes, wrote %d", len(smallData), n) + } + + // Test large file exceeding limit (should fail) + largeFile := &File{ + Path: "/large.bin", + Fs: fs, + } + + largeData := make([]byte, maxFileSize+1) + n, err = largeFile.Write(largeData) + if err == nil { + t.Error("Expected error when exceeding file size limit") + } + if !errors.Is(err, ErrFileTooLarge) { + t.Errorf("Expected ErrFileTooLarge, got: %v", err) + } + if n != 0 { + t.Errorf("Expected 0 bytes written on error, got %d", n) + } +} + +// TestTextSizeLimit tests text file size limits +func TestTextSizeLimit(t *testing.T) { + logger := newTestLogger() + + fs := &Fs{ + Bot: nil, + Logger: logger, + ChatID: 456456456, + fakeFs: newFakeFilesystem(), + stopChan: make(chan struct{}), + } + + // Test text under limit + smallText := strings.Repeat("a", maxTextSize-10) + smallFile := &File{ + Path: "/small.txt", + Content: []byte(smallText), + Fs: fs, + } + + if len(smallFile.Content) >= maxTextSize { + t.Error("Small text file should be under maxTextSize") + } + + // Test text exceeding limit + largeText := strings.Repeat("a", maxTextSize+100) + largeFile := &File{ + Path: "/large.txt", + Content: []byte(largeText), + Fs: fs, + } + + if len(largeFile.Content) <= maxTextSize { + t.Error("Large text file should exceed maxTextSize") + } +} + +// TestFsOperations tests basic filesystem operations +func TestFsOperations(t *testing.T) { + logger := newTestLogger() + + fs := &Fs{ + Bot: nil, + Logger: logger, + ChatID: 999999999, + fakeFs: newFakeFilesystem(), + stopChan: make(chan struct{}), + } + + // Test Name() + if fs.Name() != "telegram" { + t.Errorf("Expected name 'telegram', got '%s'", fs.Name()) + } + + // Test Create() + file, err := fs.Create("/test.txt") + if err != nil { + t.Fatalf("Create failed: %v", err) + } + if file == nil { + t.Fatal("Expected non-nil file") + } + + // Test Write() + data := []byte("test data") + n, err := file.Write(data) + if err != nil { + t.Fatalf("Write failed: %v", err) + } + if n != len(data) { + t.Errorf("Expected to write %d bytes, wrote %d", len(data), n) + } + + // Test file exists in fake filesystem + info := fs.fakeFs.stat("/test.txt") + if info == nil { + t.Error("Expected file to exist in fake filesystem") + } +} + +// TestMultipleFileCreation tests creating multiple files +func TestMultipleFileCreation(t *testing.T) { + logger := newTestLogger() + + fs := &Fs{ + Bot: nil, + Logger: logger, + ChatID: 888888888, + fakeFs: newFakeFilesystem(), + stopChan: make(chan struct{}), + } + + // Create multiple files of different types + filePaths := []string{ + "/photo.jpg", + "/video.mp4", + "/audio.mp3", + "/document.pdf", + "/text.txt", + } + + for _, path := range filePaths { + file, err := fs.Create(path) + if err != nil { + t.Fatalf("Create %s failed: %v", path, err) + } + + _, err = file.Write([]byte("test content")) + if err != nil { + t.Fatalf("Write %s failed: %v", path, err) + } + + // Verify file exists in fake filesystem + info := fs.fakeFs.stat(path) + if info == nil { + t.Errorf("Expected file %s to exist in fake filesystem", path) + } + } +} + +// TestFileReadWriteIntegration tests reading and writing file content +func TestFileReadWriteIntegration(t *testing.T) { + logger := newTestLogger() + + fs := &Fs{ + Bot: nil, + Logger: logger, + ChatID: 777777777, + fakeFs: newFakeFilesystem(), + stopChan: make(chan struct{}), + } + + file := &File{ + Path: "/test.txt", + Fs: fs, + } + + // Write data + originalData := []byte("Hello, World!") + n, err := file.Write(originalData) + if err != nil { + t.Fatalf("Write failed: %v", err) + } + if n != len(originalData) { + t.Errorf("Expected to write %d bytes, wrote %d", len(originalData), n) + } + + // Read data back + buf := make([]byte, len(originalData)) + n, err = file.Read(buf) + if err != nil { + t.Fatalf("Read failed: %v", err) + } + if n != len(originalData) { + t.Errorf("Expected to read %d bytes, read %d", len(originalData), n) + } + if string(buf) != string(originalData) { + t.Errorf("Expected '%s', got '%s'", string(originalData), string(buf)) + } +} diff --git a/fs/telegram/test_helpers.go b/fs/telegram/test_helpers.go new file mode 100644 index 00000000..95b871ec --- /dev/null +++ b/fs/telegram/test_helpers.go @@ -0,0 +1,209 @@ +package telegram + +import ( + "sync" + "testing" + + log "github.com/fclairamb/go-log" + "github.com/fclairamb/ftpserver/config/confpar" + tele "gopkg.in/telebot.v3" +) + +// testLogger implements log.Logger for testing +type testLogger struct { + logs []logEntry + mu sync.Mutex +} + +type logEntry struct { + level string + message string + fields []interface{} +} + +func newTestLogger() *testLogger { + return &testLogger{ + logs: make([]logEntry, 0), + } +} + +func (l *testLogger) Info(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"info", msg, fields}) +} + +func (l *testLogger) Error(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"error", msg, fields}) +} + +func (l *testLogger) Warn(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"warn", msg, fields}) +} + +func (l *testLogger) Debug(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"debug", msg, fields}) +} + +func (l *testLogger) Panic(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"panic", msg, fields}) +} + +func (l *testLogger) With(fields ...interface{}) log.Logger { + // Return self for simplicity in tests + return l +} + +func (l *testLogger) hasError() bool { + l.mu.Lock() + defer l.mu.Unlock() + for _, entry := range l.logs { + if entry.level == "error" { + return true + } + } + return false +} + +func (l *testLogger) hasWarn() bool { + l.mu.Lock() + defer l.mu.Unlock() + for _, entry := range l.logs { + if entry.level == "warn" { + return true + } + } + return false +} + +func (l *testLogger) getLogs() []logEntry { + l.mu.Lock() + defer l.mu.Unlock() + return append([]logEntry{}, l.logs...) +} + +func (l *testLogger) findLog(level, message string) *logEntry { + l.mu.Lock() + defer l.mu.Unlock() + for _, entry := range l.logs { + if entry.level == level && entry.message == message { + return &entry + } + } + return nil +} + +// MockBot implements a mock Telegram bot for testing +type MockBot struct { + sentMessages []sentMessage + sendError error + mu sync.Mutex +} + +type sentMessage struct { + recipient tele.Recipient + what interface{} + options []interface{} +} + +func newMockBot() *MockBot { + return &MockBot{ + sentMessages: make([]sentMessage, 0), + } +} + +func (m *MockBot) Send(to tele.Recipient, what interface{}, opts ...interface{}) (*tele.Message, error) { + m.mu.Lock() + defer m.mu.Unlock() + + if m.sendError != nil { + return nil, m.sendError + } + + m.sentMessages = append(m.sentMessages, sentMessage{ + recipient: to, + what: what, + options: opts, + }) + + // Return a fake successful message + return &tele.Message{ + ID: len(m.sentMessages), + Chat: &tele.Chat{ID: to.(*tele.Chat).ID}, + }, nil +} + +func (m *MockBot) Start() {} +func (m *MockBot) Stop() {} + +func (m *MockBot) setSendError(err error) { + m.mu.Lock() + defer m.mu.Unlock() + m.sendError = err +} + +func (m *MockBot) getSentMessages() []sentMessage { + m.mu.Lock() + defer m.mu.Unlock() + return append([]sentMessage{}, m.sentMessages...) +} + +func (m *MockBot) getLastMessage() *sentMessage { + m.mu.Lock() + defer m.mu.Unlock() + if len(m.sentMessages) == 0 { + return nil + } + return &m.sentMessages[len(m.sentMessages)-1] +} + +func (m *MockBot) reset() { + m.mu.Lock() + defer m.mu.Unlock() + m.sentMessages = make([]sentMessage, 0) + m.sendError = nil +} + +// createTestBot creates an offline bot for testing +func createTestBot(t *testing.T) *tele.Bot { + t.Helper() + + bot, err := tele.NewBot(tele.Settings{ + Token: "test-token-" + t.Name(), + Offline: true, + Synchronous: true, + }) + if err != nil { + t.Fatalf("Failed to create test bot: %v", err) + } + + return bot +} + +// createTestAccess creates test access configuration +func createTestAccess(chatID string) *confpar.Access { + return &confpar.Access{ + Params: map[string]string{ + "Token": "test-token-12345", + "ChatID": chatID, + }, + } +} + +// createTestAccessWithToken creates test access with custom token +func createTestAccessWithToken(token, chatID string) *confpar.Access { + return &confpar.Access{ + Params: map[string]string{ + "Token": token, + "ChatID": chatID, + }, + } +} From 8387aefd37722dadef84602172268812a5c5a1df Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 15 Oct 2025 00:35:43 +0300 Subject: [PATCH 04/10] gh action update --- .github/workflows/build.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ad3421c9..aeb1ebbc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -36,8 +36,7 @@ jobs: - name: Test # We need GCC because of the "go test -race" + # Note: GCC is pre-installed on ubuntu-24.04 runners # env: # CGO_ENABLED: 0 - run: | - apt-get update && apt-get install gcc -y - go test -race -v ./... + run: go test -race -v ./... From e7b2ef198752007cfeeb715a7ad1de19f5e019b8 Mon Sep 17 00:00:00 2001 From: V <57521+slayer@users.noreply.github.com> Date: Tue, 14 Oct 2025 14:46:58 -0700 Subject: [PATCH 05/10] Update fs/telegram/telegram.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- fs/telegram/telegram.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/telegram/telegram.go b/fs/telegram/telegram.go index 519d2eab..8c4908ab 100644 --- a/fs/telegram/telegram.go +++ b/fs/telegram/telegram.go @@ -152,7 +152,7 @@ func (f *File) Close() error { } else if isExtension(f.Path, audioExtensions) { audio := tele.Audio{File: tele.FromReader(f), Caption: basePath} _, err = f.Fs.Bot.Send(&chat, &audio) - } else if isExtension(f.Path, textExtensions) && len(f.Content) < maxTextSize { + } else if isExtension(f.Path, textExtensions) && len(f.Content) <= maxTextSize { // Validate UTF-8 for text files if !utf8.Valid(f.Content) { f.Fs.Logger.Warn("Invalid UTF-8 in text file, sending as document", "path", f.Path) From 754d4db5a076e19abf13b1051d0395aab7dc5b2e Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 15 Oct 2025 00:51:03 +0300 Subject: [PATCH 06/10] Fix race condition: protect Stop() from multiple calls - Add sync.Once to Fs struct to prevent double-close panic - Update Stop() method to use sync.Once.Do() - Add documentation that Stop() is safe to call multiple times - Add TestFsStopMultipleCalls to verify thread-safe behavior - Test verifies concurrent calls to Stop() don't panic This fixes a potential race condition where calling Stop() multiple times would cause a panic when trying to close an already-closed channel. --- fs/telegram/telegram.go | 15 ++++++++++----- fs/telegram/telegram_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/fs/telegram/telegram.go b/fs/telegram/telegram.go index 8c4908ab..0b3fcfc2 100644 --- a/fs/telegram/telegram.go +++ b/fs/telegram/telegram.go @@ -55,6 +55,8 @@ type Fs struct { // stopChan is used to signal bot shutdown stopChan chan struct{} + // stopOnce ensures Stop() can be called multiple times safely + stopOnce sync.Once } // File is the afero.File implementation @@ -278,12 +280,15 @@ func (m *Fs) Name() string { } // Stop gracefully shuts down the telegram bot +// This method is safe to call multiple times func (m *Fs) Stop() error { - if m.Bot != nil { - m.Logger.Info("Stopping telegram bot") - m.Bot.Stop() - close(m.stopChan) - } + m.stopOnce.Do(func() { + if m.Bot != nil { + m.Logger.Info("Stopping telegram bot") + m.Bot.Stop() + close(m.stopChan) + } + }) return nil } diff --git a/fs/telegram/telegram_test.go b/fs/telegram/telegram_test.go index cd13892c..2d7a9b76 100644 --- a/fs/telegram/telegram_test.go +++ b/fs/telegram/telegram_test.go @@ -549,6 +549,41 @@ func TestFsStop(t *testing.T) { // as it would require valid Telegram credentials and network access } +// TestFsStopMultipleCalls tests that Stop() can be called multiple times safely +func TestFsStopMultipleCalls(t *testing.T) { + // Test that calling Stop() multiple times doesn't panic + // even when trying to close an already-closed channel + fs := &Fs{ + Bot: nil, // nil bot to keep test simple + Logger: newTestLogger(), + stopChan: make(chan struct{}), + } + + // Call Stop() multiple times + var wg sync.WaitGroup + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + err := fs.Stop() + if err != nil { + t.Errorf("Stop() returned error: %v", err) + } + }() + } + + wg.Wait() + + // All calls should succeed without panic + // The channel should remain unclosed (since Bot is nil) + select { + case <-fs.stopChan: + t.Error("Channel should not be closed when Bot is nil") + default: + // Good, channel is still open + } +} + // TestFileWriteSizeLimit tests that Write enforces the file size limit func TestFileWriteSizeLimit(t *testing.T) { fs := &Fs{ From 09139ba6c291d1eea9138358424f20a5716171bd Mon Sep 17 00:00:00 2001 From: V <57521+slayer@users.noreply.github.com> Date: Wed, 15 Oct 2025 00:35:11 -0700 Subject: [PATCH 07/10] Update fs/telegram/telegram_integration_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- fs/telegram/telegram_integration_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/telegram/telegram_integration_test.go b/fs/telegram/telegram_integration_test.go index 1fa899e9..27ab26be 100644 --- a/fs/telegram/telegram_integration_test.go +++ b/fs/telegram/telegram_integration_test.go @@ -249,8 +249,8 @@ func TestTextSizeLimit(t *testing.T) { Fs: fs, } - if len(smallFile.Content) >= maxTextSize { - t.Error("Small text file should be under maxTextSize") + if len(smallFile.Content) > maxTextSize { + t.Error("Small text file should be under or equal to maxTextSize") } // Test text exceeding limit From b36d71b2f8dc8d1a3ddeb498f5d536fefccf21f0 Mon Sep 17 00:00:00 2001 From: V <57521+slayer@users.noreply.github.com> Date: Wed, 15 Oct 2025 00:35:37 -0700 Subject: [PATCH 08/10] Update fs/telegram/telegram_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- fs/telegram/telegram_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/telegram/telegram_test.go b/fs/telegram/telegram_test.go index 2d7a9b76..f232872b 100644 --- a/fs/telegram/telegram_test.go +++ b/fs/telegram/telegram_test.go @@ -89,7 +89,7 @@ func TestFileInfo(t *testing.T) { if dirInfo.Mode() != 0755 { t.Errorf("Expected mode 0755, got %v", dirInfo.Mode()) } - // Directory size is returned as a constant value (42 in the current implementation) + // Directory size is returned as a constant value (defaultDirSize, currently 4096, in the current implementation) if !dirInfo.IsDir() || dirInfo.Size() <= 0 { t.Errorf("Expected positive dir size, got %d", dirInfo.Size()) } From 7064df4fc536f31d53e288f00b594aacc3419344 Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 15 Oct 2025 11:35:42 +0300 Subject: [PATCH 09/10] Fix UTF-8 character counting for text size limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Change from byte length (len()) to character count (utf8.RuneCountInString()) - Telegram's 4096 limit is characters/runes, not bytes - Add TestTextSizeLimitUTF8 to verify emoji, Cyrillic, ASCII handling - Update maxTextSize documentation to clarify it's character limit This fix allows: - Emoji text up to 16KB in bytes (4096 chars Γ— 4 bytes) - Cyrillic text up to 8KB in bytes (4096 chars Γ— 2 bytes) - ASCII text up to 4KB in bytes (4096 chars Γ— 1 byte) Previously, non-ASCII text was incorrectly rejected when it exceeded 4096 bytes, even if it was under 4096 characters. --- fs/telegram/telegram.go | 5 +- fs/telegram/telegram_integration_test.go | 83 +++++++++++++++++++++++- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/fs/telegram/telegram.go b/fs/telegram/telegram.go index 0b3fcfc2..b84f8c1c 100644 --- a/fs/telegram/telegram.go +++ b/fs/telegram/telegram.go @@ -36,7 +36,8 @@ var ErrFileTooLarge = errors.New("file size exceeds Telegram limit") const ( // maxFileSize is Telegram's file size limit (50MB) maxFileSize = 50 * 1024 * 1024 - // maxTextSize is Telegram's message size limit (4096 characters) + // maxTextSize is Telegram's message size limit (4096 UTF-8 characters/runes, not bytes) + // Note: A 4096-character message can be up to 16KB in bytes (if all emojis) maxTextSize = 4096 ) @@ -154,7 +155,7 @@ func (f *File) Close() error { } else if isExtension(f.Path, audioExtensions) { audio := tele.Audio{File: tele.FromReader(f), Caption: basePath} _, err = f.Fs.Bot.Send(&chat, &audio) - } else if isExtension(f.Path, textExtensions) && len(f.Content) <= maxTextSize { + } else if isExtension(f.Path, textExtensions) && utf8.RuneCountInString(string(f.Content)) <= maxTextSize { // Validate UTF-8 for text files if !utf8.Valid(f.Content) { f.Fs.Logger.Warn("Invalid UTF-8 in text file, sending as document", "path", f.Path) diff --git a/fs/telegram/telegram_integration_test.go b/fs/telegram/telegram_integration_test.go index 27ab26be..9cecb5c6 100644 --- a/fs/telegram/telegram_integration_test.go +++ b/fs/telegram/telegram_integration_test.go @@ -241,7 +241,7 @@ func TestTextSizeLimit(t *testing.T) { stopChan: make(chan struct{}), } - // Test text under limit + // Test text under limit (character count) smallText := strings.Repeat("a", maxTextSize-10) smallFile := &File{ Path: "/small.txt", @@ -253,7 +253,7 @@ func TestTextSizeLimit(t *testing.T) { t.Error("Small text file should be under or equal to maxTextSize") } - // Test text exceeding limit + // Test text exceeding limit (character count) largeText := strings.Repeat("a", maxTextSize+100) largeFile := &File{ Path: "/large.txt", @@ -266,6 +266,85 @@ func TestTextSizeLimit(t *testing.T) { } } +// TestTextSizeLimitUTF8 tests text file size limit with multi-byte UTF-8 characters +func TestTextSizeLimitUTF8(t *testing.T) { + logger := newTestLogger() + + fs := &Fs{ + Bot: nil, + Logger: logger, + ChatID: 789789789, + fakeFs: newFakeFilesystem(), + stopChan: make(chan struct{}), + } + + // Test 1: Emoji (4 bytes per character) + // Create exactly 4096 emoji characters = 16384 bytes + emojiText := strings.Repeat("πŸ˜€", 4096) + emojiFile := &File{ + Path: "/emoji.txt", + Content: []byte(emojiText), + Fs: fs, + } + + // Should be treated as text (exactly at character limit) + byteLen := len(emojiFile.Content) + runeCount := len([]rune(emojiText)) + + t.Logf("Emoji: %d bytes, %d characters", byteLen, runeCount) + + if runeCount != maxTextSize { + t.Errorf("Expected exactly %d characters, got %d", maxTextSize, runeCount) + } + + if byteLen != 16384 { + t.Errorf("Expected 16384 bytes (4096 Γ— 4), got %d", byteLen) + } + + // Test 2: Cyrillic (2 bytes per character) + // Create exactly 4096 cyrillic characters = 8192 bytes + cyrillicText := strings.Repeat("П", 4096) + cyrillicFile := &File{ + Path: "/cyrillic.txt", + Content: []byte(cyrillicText), + Fs: fs, + } + + cyrillicBytes := len(cyrillicFile.Content) + cyrillicRunes := len([]rune(cyrillicText)) + + t.Logf("Cyrillic: %d bytes, %d characters", cyrillicBytes, cyrillicRunes) + + if cyrillicRunes != maxTextSize { + t.Errorf("Expected exactly %d characters, got %d", maxTextSize, cyrillicRunes) + } + + if cyrillicBytes != 8192 { + t.Errorf("Expected 8192 bytes (4096 Γ— 2), got %d", cyrillicBytes) + } + + // Test 3: ASCII (1 byte per character) + asciiText := strings.Repeat("a", 4096) + asciiFile := &File{ + Path: "/ascii.txt", + Content: []byte(asciiText), + Fs: fs, + } + + asciiBytes := len(asciiFile.Content) + asciiRunes := len([]rune(asciiText)) + + t.Logf("ASCII: %d bytes, %d characters", asciiBytes, asciiRunes) + + if asciiRunes != maxTextSize { + t.Errorf("Expected exactly %d characters, got %d", maxTextSize, asciiRunes) + } + + if asciiBytes != 4096 { + t.Errorf("Expected 4096 bytes (4096 Γ— 1), got %d", asciiBytes) + } +} + // TestFsOperations tests basic filesystem operations func TestFsOperations(t *testing.T) { logger := newTestLogger() From 78ad8151c47f6df750fa8b1845e1fdc0bee20f5b Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 15 Oct 2025 11:45:01 +0300 Subject: [PATCH 10/10] Add E2E testing infrastructure for Telegram FTP Add comprehensive end-to-end testing setup: - e2e-test.sh: Automated test script that builds server, creates test files (PNG, TXT, MD, MP4, JSON), uploads via FTP, and verifies delivery - E2E-TESTING.md: Complete documentation with prerequisites, setup instructions, and troubleshooting guide - TESTING_PROPOSAL.md: Testing strategy documentation - .gitignore: Exclude test artifacts and binaries The E2E script includes: - Prerequisites checking (lftp, config validation) - Automatic server lifecycle management - Multiple file type testing - Manual verification workflow - Robust cleanup with trap handlers - Colorful status output Usage: ./fs/telegram/e2e-test.sh --- .gitignore | 1 + fs/telegram/.gitignore | 1 + fs/telegram/E2E-TESTING.md | 282 ++++++++++++++ fs/telegram/TESTING_PROPOSAL.md | 636 ++++++++++++++++++++++++++++++++ fs/telegram/e2e-test.sh | 341 +++++++++++++++++ 5 files changed, 1261 insertions(+) create mode 100644 fs/telegram/.gitignore create mode 100644 fs/telegram/E2E-TESTING.md create mode 100644 fs/telegram/TESTING_PROPOSAL.md create mode 100755 fs/telegram/e2e-test.sh diff --git a/.gitignore b/.gitignore index 9cdef23e..f9b637e5 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ ci-info *.pem gdrive_token_gdrive.json __* +coverage.out diff --git a/fs/telegram/.gitignore b/fs/telegram/.gitignore new file mode 100644 index 00000000..e6152efd --- /dev/null +++ b/fs/telegram/.gitignore @@ -0,0 +1 @@ +e2e-test.conf diff --git a/fs/telegram/E2E-TESTING.md b/fs/telegram/E2E-TESTING.md new file mode 100644 index 00000000..dacc3688 --- /dev/null +++ b/fs/telegram/E2E-TESTING.md @@ -0,0 +1,282 @@ +# End-to-End Testing for Telegram FTP Server + +This directory contains end-to-end testing tools for the Telegram filesystem backend. + +## Overview + +The E2E test suite: +- βœ… Builds and starts the FTP server +- βœ… Creates test files (images, videos, text, documents) +- βœ… Uploads files via FTP client +- βœ… Verifies files are sent to Telegram +- βœ… Automatically cleans up after testing + +## Prerequisites + +### 1. Install FTP Client + +**macOS:** +```bash +brew install lftp +``` + +**Linux (Debian/Ubuntu):** +```bash +sudo apt-get install lftp +``` + +**Linux (RHEL/CentOS):** +```bash +sudo yum install lftp +``` + +### 2. Configure Telegram Bot + +You need a Telegram bot token and chat ID. + +#### Create a Telegram Bot: +1. Open Telegram and search for `@BotFather` +2. Send `/newbot` command +3. Follow instructions to create a bot +4. Copy the bot token (format: `123456789:ABCdefGHIjklMNOpqrsTUVwxyz`) + +#### Get Your Chat ID: +1. Send a message to your bot +2. Visit: `https://api.telegram.org/bot/getUpdates` +3. Look for `"chat":{"id":123456789}` in the response +4. Copy the chat ID number + +#### Update Configuration: +Edit `e2e-test.conf` and replace the token and chat ID: +```json +{ + "version": 1, + "accesses": [ + { + "fs": "telegram", + "shared": true, + "user": "test", + "pass": "test", + "params": { + "Token": "YOUR_BOT_TOKEN_HERE", + "ChatID": "YOUR_CHAT_ID_HERE" + } + } + ], + "passive_transfer_port_range": { + "start": 2122, + "end": 2130 + } +} +``` + +### 3. Optional: Install ImageMagick + +For better test image generation: + +**macOS:** +```bash +brew install imagemagick +``` + +**Linux:** +```bash +sudo apt-get install imagemagick +``` + +**Note:** The script works without ImageMagick but creates a minimal PNG file instead. + +## Running the E2E Test + +### Quick Start + +```bash +cd fs/telegram +./e2e-test.sh +``` + +### What the Script Does + +1. **Prerequisites Check** + - Verifies `lftp` is installed + - Checks configuration file exists + - Validates Telegram token and chat ID are set + +2. **Build Server** + - Compiles the FTP server binary + - Exits if build fails + +3. **Create Test Files** + - `test-image.png` - Test image (sent as Photo) + - `test-text.txt` - Plain text file (sent as Text message) + - `test-readme.md` - Markdown file (sent as formatted Markdown) + - `test-video.mp4` - Video file (sent as Video/Document) + - `test-data.json` - JSON document (sent as Document) + +4. **Start FTP Server** + - Starts server on port 2121 + - Uses configuration from `e2e-test.conf` + - Runs in background + +5. **Upload Files** + - Connects to FTP server + - Uploads all test files + - Reports success/failure + +6. **Verify Uploads** + - Prompts you to check Telegram chat + - Lists expected files + - Asks for manual confirmation + +7. **Cleanup** + - Stops FTP server + - Removes test files + - Runs automatically on script exit + +## Test Files Created + +| File | Type | Sent As | Description | +|------|------|---------|-------------| +| `test-image.png` | Image | Photo | 100x100 blue image with text | +| `test-text.txt` | Text | Text Message | Multi-line text with UTF-8 | +| `test-readme.md` | Markdown | Formatted Text | Markdown with formatting | +| `test-video.mp4` | Video | Video/Document | Small test video file | +| `test-data.json` | JSON | Document | JSON test data | + +## Expected Results + +After running the test, check your Telegram chat. You should see: + +1. **Photo** - Blue square image with "Test Image" text +2. **Text message** - Plain text content from test-text.txt +3. **Formatted message** - Markdown-rendered content with bold/italic +4. **Video or Document** - test-video.mp4 file +5. **Document** - test-data.json file + +## Troubleshooting + +### "lftp: command not found" +**Solution:** Install lftp (see Prerequisites) + +### "Server failed to start" +**Possible causes:** +- Port 2121 is already in use +- Invalid Telegram token +- Invalid chat ID +- Network connectivity issues + +**Debug:** +```bash +# Check if port is in use +lsof -i :2121 + +# Test server manually +go run . --conf fs/telegram/e2e-test.conf +``` + +### "Upload failed" +**Possible causes:** +- Server not running +- Firewall blocking connection +- Invalid credentials in config + +**Debug:** +```bash +# Test FTP connection manually +lftp -u test,test localhost:2121 +``` + +### "No files in Telegram" +**Possible causes:** +- Bot not started (send `/start` to your bot) +- Wrong chat ID +- Invalid bot token +- Bot doesn't have permission to send messages + +**Debug:** +```bash +# Test Telegram API +curl "https://api.telegram.org/bot/getMe" + +# Test sending message +curl -X POST "https://api.telegram.org/bot/sendMessage" \ + -d "chat_id=" \ + -d "text=Test from curl" +``` + +## Manual Testing + +If you want to test manually without the script: + +### 1. Start Server +```bash +cd /path/to/ftpserver +go build +./ftpserver --conf fs/telegram/e2e-test.conf +``` + +### 2. Connect with FTP Client +```bash +lftp -u test,test localhost:2121 +``` + +### 3. Upload Files +```bash +# In lftp prompt: +put /path/to/image.jpg +put /path/to/document.pdf +``` + +### 4. Check Telegram +Open your Telegram chat and verify files were received. + +## Automated Testing in CI/CD + +**Note:** This E2E test requires real Telegram credentials and is **not suitable for CI/CD** by default. + +For CI/CD, consider: +1. Using unit and integration tests (see `telegram_test.go`) +2. Setting up a dedicated test bot with secrets management +3. Running E2E tests in a separate nightly build + +## Cleaning Up + +The script automatically cleans up on exit, but if interrupted: + +```bash +# Kill server manually +pkill ftpserver + +# Remove test files +rm -rf fs/telegram/e2e-test-files +``` + +## Security Notes + +⚠️ **Important:** +- **Never commit** your real bot token to git +- The `e2e-test.conf` file contains placeholder credentials +- Replace with your own token before testing +- Consider using environment variables for tokens: + +```bash +# Set token via environment variable +export TELEGRAM_BOT_TOKEN="your-token-here" +export TELEGRAM_CHAT_ID="your-chat-id-here" + +# Modify script to use env vars if needed +``` + +## Contributing + +When modifying the E2E test: +1. Test with your own Telegram bot first +2. Ensure cleanup happens on all exit paths +3. Add new test files to `create_test_files()` function +4. Update this documentation with new test cases + +## Related Documentation + +- [README.md](README.md) - Telegram filesystem documentation +- [telegram_test.go](telegram_test.go) - Unit tests +- [telegram_integration_test.go](telegram_integration_test.go) - Integration tests diff --git a/fs/telegram/TESTING_PROPOSAL.md b/fs/telegram/TESTING_PROPOSAL.md new file mode 100644 index 00000000..6abe2b84 --- /dev/null +++ b/fs/telegram/TESTING_PROPOSAL.md @@ -0,0 +1,636 @@ +# Telegram Integration Testing Proposal + +## Executive Summary + +This document proposes comprehensive testing strategies for the Telegram filesystem backend. Based on analysis of `gopkg.in/telebot.v3` documentation and testing patterns, we can implement robust testing without requiring actual Telegram credentials or network access. + +--- + +## Current Testing Gap + +**What we have:** +- βœ… Unit tests for fake filesystem +- βœ… Unit tests for File operations (read/write) +- βœ… Unit tests for helper functions +- βœ… 50% code coverage + +**What we're missing:** +- ❌ Bot initialization testing +- ❌ Message sending (Photo, Video, Document, Audio) +- ❌ Command handler testing (/start, /help) +- ❌ Error handling from Telegram API +- ❌ Integration testing with telebot library + +--- + +## Testing Strategies + +### Strategy 1: Offline Bot Testing (Recommended) + +**Key Discovery:** Telebot supports `Offline: true` mode for testing! + +```go +// From telebot documentation +bot, err := tele.NewBot(tele.Settings{ + Token: "test-token", // Can be any string in offline mode + Offline: true, // Skips getMe API call +}) +``` + +#### Benefits: +- βœ… No network required +- βœ… No credentials required +- βœ… Fast execution +- βœ… Deterministic results +- βœ… CI/CD friendly + +#### Implementation: + +```go +// fs/telegram/telegram_integration_test.go + +func TestLoadFs_Offline(t *testing.T) { + // Create a test logger + logger := &testLogger{} + + access := &confpar.Access{ + Params: map[string]string{ + "Token": "test-token-offline", + "ChatID": "123456789", + }, + } + + // This should work in offline mode + fs, err := LoadFs(access, logger) + if err != nil { + t.Fatalf("LoadFs failed: %v", err) + } + + // Verify bot was created + if fs.(*Fs).Bot == nil { + t.Error("Bot should not be nil") + } + + // Verify ChatID was set correctly + if fs.(*Fs).ChatID != 123456789 { + t.Errorf("Expected ChatID 123456789, got %d", fs.(*Fs).ChatID) + } + + // Clean up + fs.(*Fs).Stop() +} +``` + +--- + +### Strategy 2: Mock Bot Interface + +Create a mock bot that implements the minimal interface we need. + +```go +// fs/telegram/telegram_mock_test.go + +type MockBot struct { + sentMessages []sentMessage + sendError error + mu sync.Mutex +} + +type sentMessage struct { + recipient interface{} + what interface{} + options []interface{} +} + +func (m *MockBot) Send(to tele.Recipient, what interface{}, opts ...interface{}) (*tele.Message, error) { + m.mu.Lock() + defer m.mu.Unlock() + + if m.sendError != nil { + return nil, m.sendError + } + + m.sentMessages = append(m.sentMessages, sentMessage{ + recipient: to, + what: what, + options: opts, + }) + + // Return a fake successful message + return &tele.Message{ + ID: len(m.sentMessages), + Chat: &tele.Chat{ID: to.Recipient()}, + }, nil +} + +func (m *MockBot) Start() {} +func (m *MockBot) Stop() {} + +func (m *MockBot) getSentMessages() []sentMessage { + m.mu.Lock() + defer m.mu.Unlock() + return append([]sentMessage{}, m.sentMessages...) +} +``` + +#### Usage: + +```go +func TestFileClose_WithMockBot(t *testing.T) { + mockBot := &MockBot{} + logger := &testLogger{} + + fs := &Fs{ + Bot: mockBot, + Logger: logger, + ChatID: 123456789, + fakeFs: newFakeFilesystem(), + } + + file := &File{ + Path: "/test.jpg", + Content: []byte("fake image data"), + Fs: fs, + } + + // Test Close() which triggers Send() + err := file.Close() + if err != nil { + t.Fatalf("Close failed: %v", err) + } + + // Verify Send was called + messages := mockBot.getSentMessages() + if len(messages) != 1 { + t.Errorf("Expected 1 message sent, got %d", len(messages)) + } + + // Verify it was sent as a photo + msg := messages[0] + if _, ok := msg.what.(*tele.Photo); !ok { + t.Errorf("Expected Photo, got %T", msg.what) + } +} +``` + +--- + +### Strategy 3: Test Poller Pattern + +Use telebot's test poller pattern for command handler testing. + +```go +// fs/telegram/telegram_handlers_test.go + +type testPoller struct { + updates chan tele.Update + done chan struct{} +} + +func newTestPoller() *testPoller { + return &testPoller{ + updates: make(chan tele.Update, 10), + done: make(chan struct{}, 1), + } +} + +func (p *testPoller) Poll(b *tele.Bot, updates chan tele.Update, stop chan struct{}) { + for { + select { + case upd := <-p.updates: + updates <- upd + case <-stop: + return + default: + } + } +} + +func TestBotCommands(t *testing.T) { + logger := &testLogger{} + + bot, err := tele.NewBot(tele.Settings{ + Token: "test-token", + Offline: true, + Synchronous: true, // Process updates synchronously for testing + }) + if err != nil { + t.Fatal(err) + } + + // Register our handlers + bot.Handle("/start", startHandler) + bot.Handle("/help", helpHandler) + + // Create test context + tp := newTestPoller() + bot.Poller = tp + + // Send a /start command + go func() { + tp.updates <- tele.Update{ + Message: &tele.Message{ + Text: "/start", + Chat: &tele.Chat{ID: 123456789}, + Sender: &tele.User{ + ID: 987654321, + FirstName: "TestUser", + }, + }, + } + }() + + // Start bot and wait for processing + go bot.Start() + time.Sleep(100 * time.Millisecond) + bot.Stop() + + // Verify handlers were called (would need to add instrumentation) +} +``` + +--- + +### Strategy 4: Integration Tests with ProcessUpdate + +Use `ProcessUpdate` to simulate Telegram updates directly. + +```go +// fs/telegram/telegram_update_test.go + +func TestProcessUpdate_PhotoMessage(t *testing.T) { + logger := &testLogger{} + + bot, err := tele.NewBot(tele.Settings{ + Token: "test-token", + Offline: true, + Synchronous: true, + }) + if err != nil { + t.Fatal(err) + } + + var received bool + bot.Handle(tele.OnPhoto, func(c tele.Context) error { + received = true + assert.NotNil(t, c.Message().Photo) + return nil + }) + + // Simulate receiving a photo + bot.ProcessUpdate(tele.Update{ + Message: &tele.Message{ + Photo: &tele.Photo{ + File: tele.File{FileID: "test-photo-id"}, + }, + }, + }) + + if !received { + t.Error("Photo handler was not called") + } +} +``` + +--- + +### Strategy 5: Error Simulation + +Test error handling from Telegram API. + +```go +// fs/telegram/telegram_error_test.go + +func TestFileClose_TelegramError(t *testing.T) { + mockBot := &MockBot{ + sendError: tele.ErrTooLarge, // Simulate Telegram error + } + + logger := &testLogger{} + + fs := &Fs{ + Bot: mockBot, + Logger: logger, + ChatID: 123456789, + fakeFs: newFakeFilesystem(), + } + + file := &File{ + Path: "/large.bin", + Content: make([]byte, 60*1024*1024), // 60MB, exceeds limit + Fs: fs, + } + + err := file.Close() + if err == nil { + t.Error("Expected error for oversized file") + } + + // Verify error was logged + if !logger.hasError() { + t.Error("Error should have been logged") + } +} +``` + +--- + +## Proposed Test Structure + +``` +fs/telegram/ +β”œβ”€β”€ telegram.go # Main implementation +β”œβ”€β”€ fake_fs.go # Fake filesystem +β”œβ”€β”€ file_info.go # FileInfo implementation +β”œβ”€β”€ telegram_test.go # Existing unit tests +β”œβ”€β”€ telegram_integration_test.go # NEW: Integration tests with offline bot +β”œβ”€β”€ telegram_mock_test.go # NEW: Mock bot implementation +β”œβ”€β”€ telegram_handlers_test.go # NEW: Command handler tests +β”œβ”€β”€ telegram_error_test.go # NEW: Error handling tests +└── test_helpers.go # NEW: Shared test utilities +``` + +--- + +## Test Helpers to Implement + +```go +// fs/telegram/test_helpers.go + +// testLogger implements log.Logger for testing +type testLogger struct { + logs []logEntry + mu sync.Mutex +} + +type logEntry struct { + level string + message string + fields []interface{} +} + +func (l *testLogger) Info(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"info", msg, fields}) +} + +func (l *testLogger) Error(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"error", msg, fields}) +} + +func (l *testLogger) Warn(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"warn", msg, fields}) +} + +func (l *testLogger) Debug(msg string, fields ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.logs = append(l.logs, logEntry{"debug", msg, fields}) +} + +func (l *testLogger) With(fields ...interface{}) log.Logger { + return l // Simplified for testing +} + +func (l *testLogger) hasError() bool { + l.mu.Lock() + defer l.mu.Unlock() + for _, entry := range l.logs { + if entry.level == "error" { + return true + } + } + return false +} + +func (l *testLogger) getLogs() []logEntry { + l.mu.Lock() + defer l.mu.Unlock() + return append([]logEntry{}, l.logs...) +} + +// createTestBot creates a bot for testing +func createTestBot(t *testing.T) *tele.Bot { + t.Helper() + + bot, err := tele.NewBot(tele.Settings{ + Token: "test-token-" + t.Name(), + Offline: true, + Synchronous: true, + }) + if err != nil { + t.Fatalf("Failed to create test bot: %v", err) + } + + return bot +} + +// createTestAccess creates test access configuration +func createTestAccess(chatID string) *confpar.Access { + return &confpar.Access{ + Params: map[string]string{ + "Token": "test-token", + "ChatID": chatID, + }, + } +} +``` + +--- + +## Coverage Goals + +With these strategies, we can achieve: + +| Component | Current | Target | Strategy | +|-----------|---------|--------|----------| +| LoadFs() | 0% | 90% | Offline bot | +| File.Close() | 0% | 85% | Mock bot | +| Bot handlers | 0% | 80% | ProcessUpdate | +| Error paths | 30% | 90% | Error simulation | +| **Overall** | **50%** | **75-80%** | Combined | + +--- + +## Implementation Priority + +### Phase 1: Foundation (High Priority) +1. βœ… Create `test_helpers.go` with testLogger and utilities +2. βœ… Implement offline bot tests for LoadFs() +3. βœ… Test bot initialization and configuration + +### Phase 2: Core Functionality (High Priority) +4. βœ… Create MockBot implementation +5. βœ… Test File.Close() with different file types +6. βœ… Test UTF-8 validation logic +7. βœ… Test file size limit enforcement + +### Phase 3: Command Handlers (Medium Priority) +8. ⏳ Test /start and /help handlers +9. ⏳ Verify ChatID responses +10. ⏳ Test command handler security + +### Phase 4: Error Handling (Medium Priority) +11. ⏳ Test Telegram API errors +12. ⏳ Test network failures +13. ⏳ Test invalid configurations + +### Phase 5: Advanced (Low Priority) +14. ⏳ Test concurrent file uploads +15. ⏳ Test bot lifecycle (Start/Stop) +16. ⏳ Performance testing + +--- + +## Example: Complete Integration Test + +```go +// fs/telegram/telegram_integration_test.go + +func TestTelegramFs_EndToEnd(t *testing.T) { + // Setup + logger := &testLogger{} + access := createTestAccess("123456789") + + // Create filesystem (uses offline bot) + fs, err := LoadFs(access, logger) + if err != nil { + t.Fatalf("LoadFs failed: %v", err) + } + defer fs.(*Fs).Stop() + + telegramFs := fs.(*Fs) + + // Replace bot with mock for Send() testing + mockBot := &MockBot{} + telegramFs.Bot = mockBot + + // Test 1: Create and write file + file, err := telegramFs.Create("/test.jpg") + if err != nil { + t.Fatalf("Create failed: %v", err) + } + + imageData := []byte("fake jpg data") + n, err := file.Write(imageData) + if err != nil { + t.Fatalf("Write failed: %v", err) + } + if n != len(imageData) { + t.Errorf("Expected to write %d bytes, wrote %d", len(imageData), n) + } + + // Test 2: Close triggers Send to Telegram + err = file.Close() + if err != nil { + t.Fatalf("Close failed: %v", err) + } + + // Test 3: Verify message was sent + messages := mockBot.getSentMessages() + if len(messages) != 1 { + t.Fatalf("Expected 1 message, got %d", len(messages)) + } + + // Test 4: Verify correct message type + photo, ok := messages[0].what.(*tele.Photo) + if !ok { + t.Errorf("Expected Photo, got %T", messages[0].what) + } + + // Test 5: Verify caption + if photo.Caption != "test.jpg" { + t.Errorf("Expected caption 'test.jpg', got '%s'", photo.Caption) + } + + // Test 6: Verify file exists in fake filesystem + info, err := telegramFs.Stat("/test.jpg") + if err != nil { + t.Errorf("Stat failed: %v", err) + } + if info.Size() != int64(len(imageData)) { + t.Errorf("Expected size %d, got %d", len(imageData), info.Size()) + } + + // Test 7: Verify logging + logs := logger.getLogs() + var foundSendLog bool + for _, log := range logs { + if log.level == "info" && log.message == "telegram Bot.Send()" { + foundSendLog = true + break + } + } + if !foundSendLog { + t.Error("Expected Send() to be logged") + } +} +``` + +--- + +## Benefits of This Approach + +1. **No External Dependencies** + - Tests run without internet + - No Telegram credentials needed + - Fast CI/CD pipeline + +2. **Comprehensive Coverage** + - Tests all code paths + - Simulates real scenarios + - Catches edge cases + +3. **Maintainable** + - Clear test structure + - Reusable helpers + - Well-documented patterns + +4. **Reliable** + - Deterministic results + - No flaky tests + - Easy to debug + +--- + +## Limitations & Considerations + +### What We CAN'T Test: +- ❌ Actual message delivery to Telegram +- ❌ Telegram API rate limiting +- ❌ Network latency issues +- ❌ Real file upload behavior +- ❌ Telegram server-side validation + +### What We CAN Test: +- βœ… Bot initialization logic +- βœ… File type detection +- βœ… Size limit enforcement +- βœ… Error handling paths +- βœ… Command handlers +- βœ… UTF-8 validation +- βœ… Concurrency safety +- βœ… Resource cleanup + +### Workarounds: +For actual Telegram integration: +- Manual testing with test bot +- Separate E2E test suite (optional, requires credentials) +- Staging environment testing + +--- + +## Conclusion + +By leveraging telebot's `Offline` mode and mock patterns, we can achieve **75-80% code coverage** for the Telegram filesystem backend without requiring actual Telegram credentials or network access. This approach provides fast, reliable, and maintainable tests suitable for CI/CD environments. + +**Next Steps:** +1. Review and approve this proposal +2. Implement Phase 1 (Foundation) +3. Iteratively add remaining phases +4. Document testing patterns for future contributors diff --git a/fs/telegram/e2e-test.sh b/fs/telegram/e2e-test.sh new file mode 100755 index 00000000..407b7559 --- /dev/null +++ b/fs/telegram/e2e-test.sh @@ -0,0 +1,341 @@ +#!/bin/bash +# +# End-to-End Test Script for Telegram FTP Server +# +# This script: +# 1. Builds the FTP server +# 2. Starts the server with e2e-test.conf +# 3. Creates test files (image, video, text, document) +# 4. Uploads files via FTP client +# 5. Verifies upload success +# 6. Cleans up +# +# Prerequisites: +# - lftp (FTP client) - install with: brew install lftp (macOS) or apt-get install lftp (Linux) +# - Valid Telegram bot token and chat ID in e2e-test.conf +# - ImageMagick (optional, for creating test image) - brew install imagemagick +# +# Usage: +# ./e2e-test.sh +# + +set -e # Exit on error + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +BLUE='\033[0;34m' +NC='\033[0m' # No Color + +# Configuration +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +CONFIG_FILE="$SCRIPT_DIR/e2e-test.conf" +TEST_DIR="$SCRIPT_DIR/e2e-test-files" +FTP_HOST="localhost" +FTP_PORT="2121" +FTP_USER="test" +FTP_PASS="test" +SERVER_PID="" + +# Cleanup function +cleanup() { + echo -e "${YELLOW}Cleaning up...${NC}" + + # Kill server if running + if [ -n "$SERVER_PID" ]; then + echo "Stopping FTP server (PID: $SERVER_PID)..." + kill "$SERVER_PID" 2>/dev/null || true + wait "$SERVER_PID" 2>/dev/null || true + fi + + # Remove test files + if [ -d "$TEST_DIR" ]; then + echo "Removing test files..." + rm -rf "$TEST_DIR" + fi + + echo -e "${GREEN}Cleanup complete${NC}" +} + +# Set up trap to cleanup on exit +trap cleanup EXIT INT TERM + +# Print section header +print_header() { + echo "" + echo -e "${BLUE}========================================${NC}" + echo -e "${BLUE}$1${NC}" + echo -e "${BLUE}========================================${NC}" +} + +# Check prerequisites +check_prerequisites() { + print_header "Checking Prerequisites" + + # Check if lftp is installed + if ! command -v lftp &> /dev/null; then + echo -e "${RED}ERROR: lftp is not installed${NC}" + echo "Install with:" + echo " macOS: brew install lftp" + echo " Linux: sudo apt-get install lftp" + exit 1 + fi + echo -e "${GREEN}βœ“ lftp is installed${NC}" + + # Check if config file exists + if [ ! -f "$CONFIG_FILE" ]; then + echo -e "${RED}ERROR: Config file not found: $CONFIG_FILE${NC}" + exit 1 + fi + echo -e "${GREEN}βœ“ Config file found${NC}" + + # Check if Telegram token is set (not default) + TOKEN=$(grep -o '"Token": "[^"]*"' "$CONFIG_FILE" | cut -d'"' -f4) + if [ -z "$TOKEN" ] || [ ${#TOKEN} -lt 20 ]; then + echo -e "${YELLOW}WARNING: Telegram token may not be configured properly${NC}" + echo "Edit $CONFIG_FILE and set your bot token" + else + echo -e "${GREEN}βœ“ Telegram token configured${NC}" + fi + + # Check if ChatID is set + CHAT_ID=$(grep -o '"ChatID": "[^"]*"' "$CONFIG_FILE" | cut -d'"' -f4) + if [ -z "$CHAT_ID" ]; then + echo -e "${YELLOW}WARNING: ChatID not configured${NC}" + echo "Edit $CONFIG_FILE and set your chat ID" + else + echo -e "${GREEN}βœ“ ChatID configured${NC}" + fi +} + +# Build the FTP server +build_server() { + print_header "Building FTP Server" + + cd "$PROJECT_ROOT" + echo "Building ftpserver..." + go build -o ftpserver . + + if [ ! -f "ftpserver" ]; then + echo -e "${RED}ERROR: Build failed${NC}" + exit 1 + fi + + echo -e "${GREEN}βœ“ Server built successfully${NC}" +} + +# Create test files +create_test_files() { + print_header "Creating Test Files" + + mkdir -p "$TEST_DIR" + + # 1. Create a simple test image (PNG) + echo "Creating test image..." + if command -v convert &> /dev/null; then + # Use ImageMagick if available + convert -size 100x100 xc:blue -pointsize 20 -fill white \ + -gravity center -annotate +0+0 "Test\nImage" \ + "$TEST_DIR/test-image.png" + echo -e "${GREEN}βœ“ Created test-image.png (using ImageMagick)${NC}" + else + # Create a minimal valid PNG file (1x1 red pixel) + printf '\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52' > "$TEST_DIR/test-image.png" + printf '\x00\x00\x00\x01\x00\x00\x00\x01\x08\x02\x00\x00\x00\x90\x77\x53' >> "$TEST_DIR/test-image.png" + printf '\xde\x00\x00\x00\x0c\x49\x44\x41\x54\x08\xd7\x63\xf8\xcf\xc0\x00' >> "$TEST_DIR/test-image.png" + printf '\x00\x03\x01\x01\x00\x18\xdd\x8d\xb4\x00\x00\x00\x00\x49\x45\x4e' >> "$TEST_DIR/test-image.png" + printf '\x44\xae\x42\x60\x82' >> "$TEST_DIR/test-image.png" + echo -e "${GREEN}βœ“ Created test-image.png (minimal PNG)${NC}" + fi + + # 2. Create a text file + cat > "$TEST_DIR/test-text.txt" << 'EOF' +This is a test text file for E2E testing. + +It contains: +- Multiple lines +- UTF-8 characters: Γ±, ΓΌ, δΈ­ζ–‡ +- Numbers: 123456 +- Symbols: !@#$%^&*() + +This file will be sent to Telegram via FTP. +EOF + echo -e "${GREEN}βœ“ Created test-text.txt${NC}" + + # 3. Create a markdown file + cat > "$TEST_DIR/test-readme.md" << 'EOF' +# E2E Test Readme + +This is a **markdown** file for testing. + +## Features +- *Italic text* +- **Bold text** +- `Code blocks` + +## Testing +This file tests markdown rendering in Telegram. +EOF + echo -e "${GREEN}βœ“ Created test-readme.md${NC}" + + # 4. Create a small "video" file (actually just a text file with .mp4 extension for testing) + echo "Fake video file for E2E testing" > "$TEST_DIR/test-video.mp4" + echo -e "${GREEN}βœ“ Created test-video.mp4${NC}" + + # 5. Create a JSON document + cat > "$TEST_DIR/test-data.json" << 'EOF' +{ + "test": "e2e", + "timestamp": "2025-10-14", + "files": ["image", "video", "text", "json"], + "success": true +} +EOF + echo -e "${GREEN}βœ“ Created test-data.json${NC}" + + echo "" + echo "Test files created in: $TEST_DIR" + ls -lh "$TEST_DIR" +} + +# Start FTP server +start_server() { + print_header "Starting FTP Server" + + cd "$PROJECT_ROOT" + echo "Starting server on port $FTP_PORT..." + echo "Using config: $CONFIG_FILE" + + # Start server in background + ./ftpserver --conf "$CONFIG_FILE" & + SERVER_PID=$! + + echo "Server PID: $SERVER_PID" + + # Wait for server to start + echo "Waiting for server to start..." + sleep 3 + + # Check if server is running + if ! kill -0 "$SERVER_PID" 2>/dev/null; then + echo -e "${RED}ERROR: Server failed to start${NC}" + exit 1 + fi + + echo -e "${GREEN}βœ“ Server started successfully${NC}" +} + +# Upload files via FTP +upload_files() { + print_header "Uploading Files via FTP" + + echo "Connecting to FTP server..." + echo "Host: $FTP_HOST:$FTP_PORT" + echo "User: $FTP_USER" + + # Use lftp to upload files + lftp -c " + set ftp:ssl-allow no + set net:timeout 10 + set net:max-retries 2 + open -u $FTP_USER,$FTP_PASS $FTP_HOST:$FTP_PORT + echo 'Connected to FTP server' + echo 'Uploading files...' + lcd $TEST_DIR + mput *.png + mput *.txt + mput *.md + mput *.mp4 + mput *.json + echo 'Upload complete' + quit + " + + UPLOAD_STATUS=$? + + if [ $UPLOAD_STATUS -eq 0 ]; then + echo -e "${GREEN}βœ“ Files uploaded successfully${NC}" + return 0 + else + echo -e "${RED}βœ— Upload failed with status: $UPLOAD_STATUS${NC}" + return 1 + fi +} + +# Verify uploads +verify_uploads() { + print_header "Verifying Uploads" + + echo "Files sent to Telegram:" + ls -1 "$TEST_DIR" + + echo "" + echo -e "${YELLOW}Please check your Telegram chat for the uploaded files.${NC}" + echo "" + echo "Expected files in Telegram:" + echo " 1. test-image.png (as Photo)" + echo " 2. test-text.txt (as Text message)" + echo " 3. test-readme.md (as Markdown message)" + echo " 4. test-video.mp4 (as Video or Document)" + echo " 5. test-data.json (as Document)" + + echo "" + read -p "Did all files appear in Telegram? (y/n): " -n 1 -r + echo + + if [[ $REPLY =~ ^[Yy]$ ]]; then + echo -e "${GREEN}βœ“ Manual verification: SUCCESS${NC}" + return 0 + else + echo -e "${RED}βœ— Manual verification: FAILED${NC}" + return 1 + fi +} + +# Main execution +main() { + echo -e "${BLUE}" + echo "╔════════════════════════════════════════╗" + echo "β•‘ Telegram FTP Server E2E Test β•‘" + echo "β•šβ•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•" + echo -e "${NC}" + + check_prerequisites + build_server + create_test_files + start_server + + # Give server a moment to fully initialize + sleep 2 + + upload_files + UPLOAD_RESULT=$? + + if [ $UPLOAD_RESULT -eq 0 ]; then + verify_uploads + VERIFY_RESULT=$? + else + echo -e "${RED}Skipping verification due to upload failure${NC}" + VERIFY_RESULT=1 + fi + + # Final report + print_header "Test Summary" + + if [ $UPLOAD_RESULT -eq 0 ] && [ $VERIFY_RESULT -eq 0 ]; then + echo -e "${GREEN}╔════════════════════════════════════════╗${NC}" + echo -e "${GREEN}β•‘ E2E TEST PASSED βœ“ β•‘${NC}" + echo -e "${GREEN}β•šβ•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•${NC}" + exit 0 + else + echo -e "${RED}╔════════════════════════════════════════╗${NC}" + echo -e "${RED}β•‘ E2E TEST FAILED βœ— β•‘${NC}" + echo -e "${RED}β•šβ•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•β•${NC}" + exit 1 + fi +} + +# Run main function +main