From aac354b323126c72062d963b40ff3d0848923b3b Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Thu, 18 Jun 2026 21:25:48 -0700 Subject: [PATCH] Add Unix pipe support for put and get commands Support stdin upload (`put - `) and stdout download (`get -`). Stdin is spooled to a temp file before upload; stdout downloads are byte-clean with retry-before-first-byte semantics and graceful EPIPE handling. Closes #194 --- README.md | 15 ++ cmd/get.go | 34 ++- cmd/pipe_test.go | 489 +++++++++++++++++++++++++++++++++++++++++++ cmd/put.go | 61 ++++++ cmd/sigpipe_other.go | 5 + cmd/sigpipe_unix.go | 12 ++ cmd/stdin.go | 48 +++++ cmd/stdin_test.go | 132 ++++++++++++ cmd/stdout.go | 72 +++++++ cmd/stdout_test.go | 162 ++++++++++++++ 10 files changed, 1028 insertions(+), 2 deletions(-) create mode 100644 cmd/pipe_test.go create mode 100644 cmd/sigpipe_other.go create mode 100644 cmd/sigpipe_unix.go create mode 100644 cmd/stdin.go create mode 100644 cmd/stdin_test.go create mode 100644 cmd/stdout.go create mode 100644 cmd/stdout_test.go diff --git a/README.md b/README.md index cb74553..e1ae637 100644 --- a/README.md +++ b/README.md @@ -265,6 +265,21 @@ $ dbxcli get /remote/file.txt ./local-file.txt # download a single file $ dbxcli get -r /remote/folder ./local-folder # recursively download a folder ``` +### Piping with stdin/stdout + +Use `-` as the local operand to stream through pipes: + +```sh +$ printf 'hello' | dbxcli put - /hello.txt # upload from stdin +$ tar cz ./src | dbxcli put - /backups/src.tgz # pipe archive to Dropbox +$ dbxcli get /backups/src.tgz - | tar tz # download to stdout and list +$ dbxcli get /file.txt - > local-copy.txt # download to stdout, redirect to file +``` + +Stdin uploads are spooled to a temp file before uploading, so disk space up to the full input size is required. Stdout downloads are byte-clean: all progress and diagnostic output goes to stderr. + +A bare `-` means stream only when it is the local operand. Dropbox paths named `-` are valid, for example `dbxcli put - /-` and `dbxcli get /- -`. To upload a local file literally named `-`, use `./-`. + ### Removing files and folders ```sh diff --git a/cmd/get.go b/cmd/get.go index 4f0e696..a4b8168 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -47,6 +47,10 @@ func get(cmd *cobra.Command, args []string) (err error) { recursive, _ := cmd.Flags().GetBool("recursive") + if dst == "-" { + return getStdout(cmd, src, recursive) + } + dbx := filesNewFunc(config) meta, err := dbx.GetMetadata(files.NewGetMetadataArg(src)) @@ -78,6 +82,23 @@ func get(cmd *cobra.Command, args []string) (err error) { return downloadFile(dbx, src, dst) } +func getStdout(cmd *cobra.Command, src string, recursive bool) error { + if recursive { + return errors.New("`get -` cannot be used with --recursive") + } + + dbx := filesNewFunc(config) + + meta, err := dbx.GetMetadata(files.NewGetMetadataArg(src)) + if err == nil { + if _, ok := meta.(*files.FolderMetadata); ok { + return fmt.Errorf("%s is a folder; cannot download folder to stdout", src) + } + } + + return downloadToStdout(dbx, src, cmd.OutOrStdout()) +} + func getWithClient(dbx files.Client, args []string) (err error) { if len(args) == 0 || len(args) > 2 { return errors.New("`get` requires `src` and/or `dst` arguments") @@ -229,7 +250,7 @@ func downloadFileOnce(dbx files.Client, arg *files.DownloadArg, dst string) erro if err != nil { return err } - defer contents.Close() + defer func() { _ = contents.Close() }() finalDst, err := downloadDestinationPath(dst) if err != nil { @@ -275,7 +296,16 @@ func downloadFileOnce(dbx files.Client, arg *files.DownloadArg, dst string) erro var getCmd = &cobra.Command{ Use: "get [flags] []", Short: "Download a file or folder", - RunE: get, + Long: `Download a file or folder from Dropbox. + - Use --recursive (-r) to download entire directories. + - Use - as target to write file bytes to stdout. + Stdout is byte-clean: all progress and errors go to stderr. +`, + Example: ` dbxcli get /remote/file.txt ./local-file.txt + dbxcli get -r /remote/folder ./local-folder + dbxcli get /backups/src.tgz - | tar tz + dbxcli get /file.txt - > local-copy.txt`, + RunE: get, } func init() { diff --git a/cmd/pipe_test.go b/cmd/pipe_test.go new file mode 100644 index 0000000..1f99367 --- /dev/null +++ b/cmd/pipe_test.go @@ -0,0 +1,489 @@ +package cmd + +import ( + "bytes" + "errors" + "io" + "os" + "strings" + "testing" + + "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox" + "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" + "github.com/spf13/cobra" +) + +func testPutCmdWithStdin(stdin io.Reader) *cobra.Command { + cmd := testPutCmd() + cmd.SetIn(stdin) + return cmd +} + +func TestPutStdin_RequiresTarget(t *testing.T) { + cmd := testPutCmdWithStdin(strings.NewReader("data")) + err := put(cmd, []string{"-"}) + if err == nil { + t.Fatal("expected error for put - without target") + } + if !strings.Contains(err.Error(), "explicit target") { + t.Errorf("error = %q, want mention of explicit target", err.Error()) + } +} + +func TestPutStdin_RejectsRecursive(t *testing.T) { + cmd := testPutCmdWithStdin(strings.NewReader("data")) + _ = cmd.Flags().Set("recursive", "true") + err := put(cmd, []string{"-", "/file.txt"}) + if err == nil { + t.Fatal("expected error for put - --recursive") + } + if !strings.Contains(err.Error(), "--recursive") { + t.Errorf("error = %q, want mention of --recursive", err.Error()) + } +} + +func TestPutStdin_RejectsDirectoryStyleTarget(t *testing.T) { + cmd := testPutCmdWithStdin(strings.NewReader("data")) + err := put(cmd, []string{"-", "/folder/"}) + if err == nil { + t.Fatal("expected error for directory-style target") + } + if !strings.Contains(err.Error(), "directory target") { + t.Errorf("error = %q, want mention of directory target", err.Error()) + } +} + +func TestPutStdin_RejectsExistingFolder(t *testing.T) { + cmd := testPutCmdWithStdin(strings.NewReader("data")) + + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: arg.Path}}, nil + }, + } + stubFilesClient(t, mock) + + err := put(cmd, []string{"-", "/existing-folder"}) + if err == nil { + t.Fatal("expected error for existing folder target") + } + if !strings.Contains(err.Error(), "folder") { + t.Errorf("error = %q, want mention of folder", err.Error()) + } +} + +func TestPutStdin_UploadsContent(t *testing.T) { + content := "hello from stdin" + cmd := testPutCmdWithStdin(strings.NewReader(content)) + + var uploadedPath string + var uploadedContent []byte + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return nil, &files.GetMetadataAPIError{} + }, + uploadFn: func(arg *files.UploadArg, r io.Reader) (*files.FileMetadata, error) { + uploadedPath = arg.Path + data, _ := io.ReadAll(r) + uploadedContent = data + return &files.FileMetadata{}, nil + }, + } + stubFilesClient(t, mock) + + err := put(cmd, []string{"-", "/dest.txt"}) + if err != nil { + t.Fatalf("put stdin error: %v", err) + } + if uploadedPath != "/dest.txt" { + t.Errorf("uploaded path = %q, want /dest.txt", uploadedPath) + } + if !strings.Contains(string(uploadedContent), content) { + t.Errorf("uploaded content = %q, want to contain %q", uploadedContent, content) + } +} + +func TestPutStdin_UploadsToDashPath(t *testing.T) { + content := "dash path" + cmd := testPutCmdWithStdin(strings.NewReader(content)) + + var uploadedPath string + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return nil, &files.GetMetadataAPIError{} + }, + uploadFn: func(arg *files.UploadArg, r io.Reader) (*files.FileMetadata, error) { + uploadedPath = arg.Path + _, _ = io.ReadAll(r) + return &files.FileMetadata{}, nil + }, + } + stubFilesClient(t, mock) + + err := put(cmd, []string{"-", "/-"}) + if err != nil { + t.Fatalf("put stdin error: %v", err) + } + if uploadedPath != "/-" { + t.Errorf("uploaded path = %q, want /-", uploadedPath) + } +} + +func TestPutStdin_EmptyUploadsZeroBytes(t *testing.T) { + cmd := testPutCmdWithStdin(strings.NewReader("")) + + var uploadedSize int + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return nil, &files.GetMetadataAPIError{} + }, + uploadFn: func(arg *files.UploadArg, r io.Reader) (*files.FileMetadata, error) { + data, _ := io.ReadAll(r) + uploadedSize = len(data) + return &files.FileMetadata{}, nil + }, + } + stubFilesClient(t, mock) + + err := put(cmd, []string{"-", "/empty.txt"}) + if err != nil { + t.Fatalf("put stdin error: %v", err) + } + if uploadedSize != 0 { + t.Errorf("uploaded size = %d, want 0", uploadedSize) + } +} + +func TestPutStdin_CleanupFailureAfterSuccessfulUpload(t *testing.T) { + cmd := testPutCmdWithStdin(strings.NewReader("data")) + + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return nil, &files.GetMetadataAPIError{} + }, + uploadFn: func(arg *files.UploadArg, r io.Reader) (*files.FileMetadata, error) { + _, _ = io.ReadAll(r) + return &files.FileMetadata{}, nil + }, + } + stubFilesClient(t, mock) + + cleanupErr := errors.New("remove failed") + var tempPath string + origRemoveFile := removeFile + removeFile = func(path string) error { + tempPath = path + return cleanupErr + } + t.Cleanup(func() { + removeFile = origRemoveFile + if tempPath != "" { + _ = origRemoveFile(tempPath) + } + }) + + origStderr := os.Stderr + stderrReader, stderrWriter, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stderr = stderrWriter + t.Cleanup(func() { + os.Stderr = origStderr + _ = stderrReader.Close() + _ = stderrWriter.Close() + }) + + err = put(cmd, []string{"-", "/dest.txt"}) + _ = stderrWriter.Close() + stderr, _ := io.ReadAll(stderrReader) + os.Stderr = origStderr + + if err == nil { + t.Fatal("expected cleanup failure") + } + if !errors.Is(err, cleanupErr) { + t.Fatalf("error = %v, want wrapped cleanup error", err) + } + if !strings.Contains(err.Error(), "sensitive stdin data may remain on disk") { + t.Errorf("error = %q, want sensitive-data warning", err.Error()) + } + if tempPath == "" { + t.Fatal("expected temp path to be captured") + } + if !strings.Contains(string(stderr), "failed to remove temp file") || !strings.Contains(string(stderr), cleanupErr.Error()) { + t.Errorf("stderr = %q, want cleanup failure with underlying error", string(stderr)) + } +} + +func TestPutStdin_ReportsCleanupFailureAfterUploadFailure(t *testing.T) { + cmd := testPutCmdWithStdin(strings.NewReader("data")) + uploadErr := errors.New("upload failed") + + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return nil, &files.GetMetadataAPIError{} + }, + uploadFn: func(arg *files.UploadArg, r io.Reader) (*files.FileMetadata, error) { + _, _ = io.ReadAll(r) + return nil, uploadErr + }, + } + stubFilesClient(t, mock) + + cleanupErr := errors.New("remove failed") + var tempPath string + origRemoveFile := removeFile + removeFile = func(path string) error { + tempPath = path + return cleanupErr + } + t.Cleanup(func() { + removeFile = origRemoveFile + if tempPath != "" { + _ = origRemoveFile(tempPath) + } + }) + + origStderr := os.Stderr + stderrReader, stderrWriter, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stderr = stderrWriter + t.Cleanup(func() { + os.Stderr = origStderr + _ = stderrReader.Close() + _ = stderrWriter.Close() + }) + + err = put(cmd, []string{"-", "/dest.txt"}) + _ = stderrWriter.Close() + stderr, _ := io.ReadAll(stderrReader) + os.Stderr = origStderr + + if !errors.Is(err, uploadErr) { + t.Fatalf("error = %v, want upload error", err) + } + if errors.Is(err, cleanupErr) { + t.Fatalf("error = %v, should preserve upload error as return value", err) + } + if tempPath == "" { + t.Fatal("expected temp path to be captured") + } + if !strings.Contains(string(stderr), "failed to remove temp file") || + !strings.Contains(string(stderr), cleanupErr.Error()) || + !strings.Contains(string(stderr), "sensitive stdin data may remain on disk") { + t.Errorf("stderr = %q, want cleanup failure warning", string(stderr)) + } +} + +func TestPutLocalDashFile(t *testing.T) { + // put ./- /remote uploads a local file named "-" + tmpDir := t.TempDir() + dashFile := tmpDir + "/-" + if err := writeTestFile(dashFile, "local dash file"); err != nil { + t.Fatal(err) + } + + var uploadedPath string + origConfig := config + defer func() { config = origConfig }() + config = testConfig() + + testClient := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return nil, &files.GetMetadataAPIError{} + }, + uploadFn: func(arg *files.UploadArg, r io.Reader) (*files.FileMetadata, error) { + uploadedPath = arg.Path + _, _ = io.ReadAll(r) + return &files.FileMetadata{}, nil + }, + } + origNew := filesNewFunc + filesNewFunc = func(_ dropbox.Config) files.Client { return testClient } + defer func() { filesNewFunc = origNew }() + + cmd := testPutCmd() + err := put(cmd, []string{dashFile, "/remote"}) + if err != nil { + t.Fatalf("put ./ - error: %v", err) + } + if uploadedPath != "/remote" { + t.Errorf("uploaded path = %q, want /remote", uploadedPath) + } +} + +// --- get stdout tests --- + +func testGetCmd() *cobra.Command { + cmd := &cobra.Command{} + cmd.Flags().BoolP("recursive", "r", false, "") + return cmd +} + +func TestGetStdout_WritesContent(t *testing.T) { + content := "file content here" + var stdout bytes.Buffer + + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return &files.FileMetadata{ + Metadata: files.Metadata{PathDisplay: arg.Path}, + Size: uint64(len(content)), + }, nil + }, + downloadFn: func(arg *files.DownloadArg) (*files.FileMetadata, io.ReadCloser, error) { + meta := &files.FileMetadata{ + Metadata: files.Metadata{PathDisplay: arg.Path}, + Size: uint64(len(content)), + } + return meta, io.NopCloser(strings.NewReader(content)), nil + }, + } + stubFilesClient(t, mock) + + cmd := testGetCmd() + cmd.SetOut(&stdout) + + err := get(cmd, []string{"/file.txt", "-"}) + if err != nil { + t.Fatalf("get stdout error: %v", err) + } + if stdout.String() != content { + t.Errorf("stdout = %q, want %q", stdout.String(), content) + } +} + +func TestGetStdout_DashPathDownloads(t *testing.T) { + content := "dash content" + var stdout bytes.Buffer + + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return &files.FileMetadata{ + Metadata: files.Metadata{PathDisplay: arg.Path}, + Size: uint64(len(content)), + }, nil + }, + downloadFn: func(arg *files.DownloadArg) (*files.FileMetadata, io.ReadCloser, error) { + if arg.Path != "/-" { + t.Errorf("download path = %q, want /-", arg.Path) + } + meta := &files.FileMetadata{ + Metadata: files.Metadata{PathDisplay: arg.Path}, + Size: uint64(len(content)), + } + return meta, io.NopCloser(strings.NewReader(content)), nil + }, + } + stubFilesClient(t, mock) + + cmd := testGetCmd() + cmd.SetOut(&stdout) + + err := get(cmd, []string{"/-", "-"}) + if err != nil { + t.Fatalf("get /- - error: %v", err) + } + if stdout.String() != content { + t.Errorf("stdout = %q, want %q", stdout.String(), content) + } +} + +func TestGetStdout_RejectsFolder(t *testing.T) { + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: arg.Path}}, nil + }, + } + stubFilesClient(t, mock) + + cmd := testGetCmd() + var stdout bytes.Buffer + cmd.SetOut(&stdout) + + err := get(cmd, []string{"/folder", "-"}) + if err == nil { + t.Fatal("expected error for folder download to stdout") + } + if !strings.Contains(err.Error(), "folder") { + t.Errorf("error = %q, want mention of folder", err.Error()) + } + if stdout.Len() != 0 { + t.Errorf("stdout should be empty, got %q", stdout.String()) + } +} + +func TestGetStdout_RejectsRecursive(t *testing.T) { + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: arg.Path}}, nil + }, + } + stubFilesClient(t, mock) + + cmd := testGetCmd() + _ = cmd.Flags().Set("recursive", "true") + var stdout bytes.Buffer + cmd.SetOut(&stdout) + + err := get(cmd, []string{"/folder", "-"}) + if err == nil { + t.Fatal("expected error for recursive download to stdout") + } + if !strings.Contains(err.Error(), "--recursive") { + t.Errorf("error = %q, want mention of --recursive", err.Error()) + } +} + +func TestGetStdout_NoLocalFilesCreated(t *testing.T) { + tmpDir := t.TempDir() + origDir, _ := os.Getwd() + _ = os.Chdir(tmpDir) + defer func() { _ = os.Chdir(origDir) }() + + content := "file data" + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + return &files.FileMetadata{ + Metadata: files.Metadata{PathDisplay: arg.Path}, + Size: uint64(len(content)), + }, nil + }, + downloadFn: func(arg *files.DownloadArg) (*files.FileMetadata, io.ReadCloser, error) { + meta := &files.FileMetadata{ + Metadata: files.Metadata{PathDisplay: arg.Path}, + Size: uint64(len(content)), + } + return meta, io.NopCloser(strings.NewReader(content)), nil + }, + } + stubFilesClient(t, mock) + + cmd := testGetCmd() + var stdout bytes.Buffer + cmd.SetOut(&stdout) + + err := get(cmd, []string{"/file.txt", "-"}) + if err != nil { + t.Fatalf("get stdout error: %v", err) + } + + entries, err := os.ReadDir(tmpDir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + var names []string + for _, e := range entries { + names = append(names, e.Name()) + } + t.Errorf("expected no local files created, found: %v", names) + } +} + +func writeTestFile(path, content string) error { + return os.WriteFile(path, []byte(content), 0644) +} diff --git a/cmd/put.go b/cmd/put.go index 0db4dab..54ab767 100644 --- a/cmd/put.go +++ b/cmd/put.go @@ -203,6 +203,10 @@ func put(cmd *cobra.Command, args []string) (err error) { src := args[0] + if src == "-" { + return putStdin(cmd, args, opts, recursive) + } + srcInfo, err := os.Stat(src) if err != nil { return @@ -234,6 +238,56 @@ func put(cmd *cobra.Command, args []string) (err error) { return putFile(src, dst, opts) } +func putStdin(cmd *cobra.Command, args []string, opts putOptions, recursive bool) error { + if len(args) < 2 { + return errors.New("`put -` requires an explicit target path") + } + if recursive { + return errors.New("`put -` cannot be used with --recursive") + } + + dst := args[1] + if strings.HasSuffix(dst, "/") { + return fmt.Errorf("cannot upload stdin to directory target %q; provide a full Dropbox file path", dst) + } + + dstPath, err := validatePath(dst) + if err != nil { + return err + } + + dbx := filesNewFunc(config) + if isRemoteFolder(dbx, dstPath) { + return fmt.Errorf("cannot upload stdin to folder %q; provide a full Dropbox file path", dstPath) + } + + tmpPath, _, cleanup, err := spoolStdinToTemp(cmd.InOrStdin()) + if err != nil { + return err + } + + uploadErr := putFile(tmpPath, dstPath, opts) + cleanupErr := cleanup() + + if uploadErr != nil { + if cleanupErr != nil { + reportStdinCleanupFailure(tmpPath, cleanupErr) + } + return uploadErr + } + + if cleanupErr != nil { + reportStdinCleanupFailure(tmpPath, cleanupErr) + return fmt.Errorf("failed to remove temp file %s after upload; sensitive stdin data may remain on disk: %w", tmpPath, cleanupErr) + } + + return nil +} + +func reportStdinCleanupFailure(tmpPath string, err error) { + fmt.Fprintf(os.Stderr, "error: failed to remove temp file %s: %v; sensitive stdin data may remain on disk\n", tmpPath, err) +} + func resolveDestination(dbx files.Client, src, dst string, dstIsDir bool) string { if dstIsDir { return path.Join("/", dst, filepath.Base(src)) @@ -386,7 +440,14 @@ var putCmd = &cobra.Command{ Long: `Upload files or directories to Dropbox. - If target is not provided, uploads to the root of your Dropbox. - Use --recursive (-r) to upload entire directories. + - Use - as source to read from stdin (target is required). + Stdin is spooled to a temp file before upload and may use disk + space up to the full input size. `, + Example: ` dbxcli put file.txt /destination/file.txt + dbxcli put -r ./project /backup/project + printf 'hello' | dbxcli put - /hello.txt + tar cz ./src | dbxcli put - /backups/src.tgz`, RunE: put, } diff --git a/cmd/sigpipe_other.go b/cmd/sigpipe_other.go new file mode 100644 index 0000000..c58f5c9 --- /dev/null +++ b/cmd/sigpipe_other.go @@ -0,0 +1,5 @@ +//go:build !(darwin || dragonfly || freebsd || illumos || linux || netbsd || openbsd || solaris) + +package cmd + +func ignoreBrokenPipeSignal() {} diff --git a/cmd/sigpipe_unix.go b/cmd/sigpipe_unix.go new file mode 100644 index 0000000..eeac72d --- /dev/null +++ b/cmd/sigpipe_unix.go @@ -0,0 +1,12 @@ +//go:build darwin || dragonfly || freebsd || illumos || linux || netbsd || openbsd || solaris + +package cmd + +import ( + "os/signal" + "syscall" +) + +func ignoreBrokenPipeSignal() { + signal.Ignore(syscall.SIGPIPE) +} diff --git a/cmd/stdin.go b/cmd/stdin.go new file mode 100644 index 0000000..bd19375 --- /dev/null +++ b/cmd/stdin.go @@ -0,0 +1,48 @@ +package cmd + +import ( + "errors" + "fmt" + "io" + "os" +) + +var removeFile = os.Remove + +func spoolStdinToTemp(r io.Reader) (path string, size int64, cleanup func() error, err error) { + f, err := os.CreateTemp("", "dbxcli-stdin-*") + if err != nil { + return "", 0, nil, fmt.Errorf("create temp file: %w", err) + } + if err := os.Chmod(f.Name(), 0600); err != nil { + _ = f.Close() + _ = os.Remove(f.Name()) + return "", 0, nil, fmt.Errorf("chmod temp file: %w", err) + } + + tmpPath := f.Name() + removeTmp := func() error { return removeFile(tmpPath) } + + n, copyErr := io.Copy(f, r) + if closeErr := f.Close(); closeErr != nil { + if copyErr != nil { + return "", 0, nil, stdinSpoolFailureError(tmpPath, copyErr, removeTmp()) + } + return "", 0, nil, stdinSpoolFailureError(tmpPath, closeErr, removeTmp()) + } + if copyErr != nil { + return "", 0, nil, stdinSpoolFailureError(tmpPath, copyErr, removeTmp()) + } + + return tmpPath, n, removeTmp, nil +} + +func stdinSpoolFailureError(tmpPath string, cause error, cleanupErr error) error { + if cleanupErr == nil { + return cause + } + return errors.Join( + cause, + fmt.Errorf("failed to remove temp file %s after stdin spool failure; sensitive stdin data may remain on disk: %w", tmpPath, cleanupErr), + ) +} diff --git a/cmd/stdin_test.go b/cmd/stdin_test.go new file mode 100644 index 0000000..0e692f4 --- /dev/null +++ b/cmd/stdin_test.go @@ -0,0 +1,132 @@ +package cmd + +import ( + "bytes" + "errors" + "io" + "os" + "runtime" + "strings" + "testing" +) + +func TestSpoolStdinToTemp_Success(t *testing.T) { + data := []byte("hello from stdin") + r := bytes.NewReader(data) + + path, size, cleanup, err := spoolStdinToTemp(r) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer func() { _ = cleanup() }() + + if size != int64(len(data)) { + t.Errorf("size = %d, want %d", size, len(data)) + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read temp file: %v", err) + } + if !bytes.Equal(got, data) { + t.Errorf("temp file content = %q, want %q", got, data) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("failed to stat temp file: %v", err) + } + if runtime.GOOS != "windows" && info.Mode().Perm() != 0600 { + t.Errorf("temp file permissions = %o, want 0600", info.Mode().Perm()) + } +} + +func TestSpoolStdinToTemp_Empty(t *testing.T) { + r := bytes.NewReader(nil) + + path, size, cleanup, err := spoolStdinToTemp(r) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer func() { _ = cleanup() }() + + if size != 0 { + t.Errorf("size = %d, want 0", size) + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read temp file: %v", err) + } + if len(got) != 0 { + t.Errorf("temp file content = %q, want empty", got) + } +} + +func TestSpoolStdinToTemp_ReadError(t *testing.T) { + r := &failingReader{err: io.ErrUnexpectedEOF} + + _, _, _, err := spoolStdinToTemp(r) + if err == nil { + t.Fatal("expected error for failing reader") + } +} + +func TestSpoolStdinToTemp_ReadErrorReportsCleanupFailure(t *testing.T) { + readErr := io.ErrUnexpectedEOF + cleanupErr := errors.New("remove failed") + var tempPath string + origRemoveFile := removeFile + removeFile = func(path string) error { + tempPath = path + return cleanupErr + } + t.Cleanup(func() { + removeFile = origRemoveFile + if tempPath != "" { + _ = origRemoveFile(tempPath) + } + }) + + _, _, _, err := spoolStdinToTemp(&failingReader{err: readErr}) + if err == nil { + t.Fatal("expected error for failing reader") + } + if !errors.Is(err, readErr) { + t.Fatalf("error = %v, want read error", err) + } + if !errors.Is(err, cleanupErr) { + t.Fatalf("error = %v, want cleanup error", err) + } + if tempPath == "" { + t.Fatal("expected temp path to be captured") + } + if !strings.Contains(err.Error(), "sensitive stdin data may remain on disk") { + t.Errorf("error = %q, want sensitive-data warning", err.Error()) + } +} + +func TestSpoolStdinToTemp_CleanupRemovesFile(t *testing.T) { + r := bytes.NewReader([]byte("data")) + + path, _, cleanup, err := spoolStdinToTemp(r) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if err := cleanup(); err != nil { + t.Fatalf("cleanup error: %v", err) + } + + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Errorf("expected temp file to be removed after cleanup, stat err = %v", err) + } +} + +type failingReader struct { + err error +} + +func (r *failingReader) Read(p []byte) (int, error) { + return 0, r.err +} diff --git a/cmd/stdout.go b/cmd/stdout.go new file mode 100644 index 0000000..56c9193 --- /dev/null +++ b/cmd/stdout.go @@ -0,0 +1,72 @@ +package cmd + +import ( + "errors" + "fmt" + "io" + "os" + "syscall" + + "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" +) + +var errStdoutBrokenPipe = errors.New("stdout broken pipe") + +func downloadToStdout(dbx files.Client, src string, w io.Writer) error { + ignoreBrokenPipeSignal() + + arg := files.NewDownloadArg(src) + var bytesWritten int64 + + err := retryWithBackoff(func() error { + if bytesWritten > 0 { + return partialStdoutError(bytesWritten) + } + + _, contents, err := dbx.Download(arg) + if err != nil { + return err + } + defer func() { _ = contents.Close() }() + + n, copyErr := io.Copy(stdoutBrokenPipeWriter{w: w}, contents) + bytesWritten += n + + if errors.Is(copyErr, errStdoutBrokenPipe) { + return nil + } + if copyErr != nil && bytesWritten > 0 { + return partialStdoutError(bytesWritten) + } + return copyErr + }) + + return err +} + +func partialStdoutError(bytesWritten int64) error { + return fmt.Errorf("download failed after writing %d bytes to stdout; cannot retry partial output", bytesWritten) +} + +type stdoutBrokenPipeWriter struct { + w io.Writer +} + +func (w stdoutBrokenPipeWriter) Write(p []byte) (int, error) { + n, err := w.w.Write(p) + if err != nil && isEPIPE(err) { + return n, errStdoutBrokenPipe + } + return n, err +} + +func isEPIPE(err error) bool { + if errors.Is(err, syscall.EPIPE) { + return true + } + var pathErr *os.PathError + if errors.As(err, &pathErr) { + return errors.Is(pathErr.Err, syscall.EPIPE) + } + return false +} diff --git a/cmd/stdout_test.go b/cmd/stdout_test.go new file mode 100644 index 0000000..482ef86 --- /dev/null +++ b/cmd/stdout_test.go @@ -0,0 +1,162 @@ +package cmd + +import ( + "bytes" + "errors" + "io" + "strings" + "syscall" + "testing" + + "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox" + dbxauth "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/auth" + "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" +) + +func TestDownloadToStdout_Success(t *testing.T) { + content := "file content" + mock := &mockFilesClient{ + downloadFn: func(arg *files.DownloadArg) (*files.FileMetadata, io.ReadCloser, error) { + meta := &files.FileMetadata{Size: uint64(len(content))} + return meta, io.NopCloser(strings.NewReader(content)), nil + }, + } + + var buf bytes.Buffer + err := downloadToStdout(mock, "/file.txt", &buf) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if buf.String() != content { + t.Errorf("output = %q, want %q", buf.String(), content) + } +} + +func TestDownloadToStdout_RetriesBeforeFirstByte(t *testing.T) { + stubRetrySleep(t) + content := "file content" + calls := 0 + + mock := &mockFilesClient{ + downloadFn: func(arg *files.DownloadArg) (*files.FileMetadata, io.ReadCloser, error) { + calls++ + if calls < 3 { + return nil, nil, dbxauth.ServerError{APIError: dropbox.APIError{ErrorSummary: "500"}} + } + meta := &files.FileMetadata{Size: uint64(len(content))} + return meta, io.NopCloser(strings.NewReader(content)), nil + }, + } + + var buf bytes.Buffer + err := downloadToStdout(mock, "/file.txt", &buf) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if calls != 3 { + t.Errorf("calls = %d, want 3", calls) + } + if buf.String() != content { + t.Errorf("output = %q, want %q", buf.String(), content) + } +} + +func TestDownloadToStdout_NoRetryAfterPartialOutput(t *testing.T) { + retryDelays := stubRetrySleep(t) + downloadCalls := 0 + + mock := &mockFilesClient{ + downloadFn: func(arg *files.DownloadArg) (*files.FileMetadata, io.ReadCloser, error) { + downloadCalls++ + meta := &files.FileMetadata{Size: 100} + return meta, &failingReadCloser{data: []byte("partial")}, nil + }, + } + + var buf bytes.Buffer + err := downloadToStdout(mock, "/file.txt", &buf) + if err == nil { + t.Fatal("expected error for partial output failure") + } + if !strings.Contains(err.Error(), "cannot retry") { + t.Errorf("error = %q, want mention of cannot retry", err.Error()) + } + if downloadCalls != 1 { + t.Errorf("download calls = %d, want 1 (no second Download call after partial write)", downloadCalls) + } + if len(*retryDelays) != 0 { + t.Errorf("retry delays = %v, want no retry sleep after partial output", *retryDelays) + } +} + +func TestDownloadToStdout_WriterEPIPEReturnsNil(t *testing.T) { + mock := &mockFilesClient{ + downloadFn: func(arg *files.DownloadArg) (*files.FileMetadata, io.ReadCloser, error) { + meta := &files.FileMetadata{Size: 100} + return meta, io.NopCloser(strings.NewReader("some data")), nil + }, + } + + err := downloadToStdout(mock, "/file.txt", epipeWriter{}) + if err != nil { + t.Fatalf("expected nil error for writer EPIPE, got %v", err) + } +} + +func TestDownloadToStdout_ReaderEPIPEReturnsError(t *testing.T) { + downloadCalls := 0 + mock := &mockFilesClient{ + downloadFn: func(arg *files.DownloadArg) (*files.FileMetadata, io.ReadCloser, error) { + downloadCalls++ + meta := &files.FileMetadata{Size: 100} + return meta, io.NopCloser(&epipeReader{data: []byte("some")}), nil + }, + } + + var buf bytes.Buffer + err := downloadToStdout(mock, "/file.txt", &buf) + if err == nil { + t.Fatal("expected error for reader EPIPE") + } + if !strings.Contains(err.Error(), "cannot retry") { + t.Fatalf("error = %q, want partial-output no-retry error", err.Error()) + } + if downloadCalls != 1 { + t.Errorf("download calls = %d, want 1", downloadCalls) + } +} + +func TestIsEPIPE(t *testing.T) { + if !isEPIPE(syscall.EPIPE) { + t.Error("expected EPIPE to be recognized") + } + if isEPIPE(io.EOF) { + t.Error("expected EOF to not be EPIPE") + } + if isEPIPE(nil) { + t.Error("expected nil to not be EPIPE") + } + if isEPIPE(errors.New("random error")) { + t.Error("expected random error to not be EPIPE") + } +} + +type epipeReader struct { + data []byte + sent bool +} + +func (r *epipeReader) Read(p []byte) (int, error) { + if !r.sent { + r.sent = true + n := copy(p, r.data) + return n, nil + } + return 0, syscall.EPIPE +} + +type epipeWriter struct{} + +func (epipeWriter) Write(p []byte) (int, error) { + return 0, syscall.EPIPE +}