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(-) 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