Merge pull request #295 from dropbox/put/chunk-size-validation

Add upper-bound chunk-size validation to put

Author: AndreyVMarkelov

README.md

@@ -406,12 +406,21 @@ The `--verbose` option will turn on verbose logging and is useful for debugging.
 ```sh
 $ dbxcli put file.txt /destination/file.txt        # upload a single file
 $ dbxcli put -r ./project /backup/project          # recursively upload a directory
-$ dbxcli put -r -w 8 ./large-folder /backup/large  # use 8 workers per large file
+$ dbxcli put -w 1 -c 134217728 large.zip /backup/large.zip
 $ dbxcli put --if-exists skip file.txt /dest.txt   # skip if the file already exists
 ```
 
 By default, `put` overwrites existing destination files. Use `--if-exists overwrite|skip|fail` to choose whether existing files are overwritten, skipped, or treated as an error.
 
+For files larger than 32MiB, `put` uses Dropbox upload sessions. By default, it
+uses 4 workers and 16MiB chunks. Each chunk is one upload-session request; chunk
+size must be a multiple of 4MiB and no more than 128MiB. On unstable networks,
+fewer workers and larger chunks may be more reliable:
+
+```sh
+$ dbxcli put -w 1 -c 134217728 large.zip /backup/large.zip
+```
+
 ### Downloading files and directories
 
 ```sh

cmd/put.go

@@ -38,6 +38,11 @@ import (
 const singleShotUploadSizeCutoff int64 = 32 * (1 << 20)
 
 const (
+	putChunkSizeUnit int64 = 1 << 22
+	// Dropbox upload-session requests should stay below 150 MB. Use a
+	// conservative 128 MiB max that is also a multiple of 4 MiB.
+	putMaxChunkSize int64 = 128 * (1 << 20)
+
 	putIfExistsOverwrite = "overwrite"
 	putIfExistsSkip      = "skip"
 	putIfExistsFail      = "fail"
@@ -445,8 +450,14 @@ func parsePutOptions(cmd *cobra.Command) (putOptions, error) {
 	if err != nil {
 		return putOptions{}, err
 	}
-	if chunkSize%(1<<22) != 0 {
-		return putOptions{}, invalidArgumentsErrorWithDetails("`put` requires chunk size to be multiple of 4MiB", flagErrorDetails("chunksize"))
+	if chunkSize < putChunkSizeUnit {
+		return putOptions{}, invalidArgumentsErrorWithDetails("`put` requires chunk size to be at least 4MiB", flagErrorDetails("chunksize"))
+	}
+	if chunkSize%putChunkSizeUnit != 0 {
+		return putOptions{}, invalidArgumentsErrorWithDetails("`put` requires chunk size to be a multiple of 4MiB", flagErrorDetails("chunksize"))
+	}
+	if chunkSize > putMaxChunkSize {
+		return putOptions{}, invalidArgumentsErrorWithDetails("`put` requires chunk size to be no more than 128MiB", flagErrorDetails("chunksize"))
 	}
 	workers, err := cmd.Flags().GetInt("workers")
 	if err != nil {
@@ -889,9 +900,13 @@ var putCmd = &cobra.Command{
   - 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.
+  - Files larger than 32MiB use Dropbox upload sessions. Each chunk is one
+    upload-session request; chunk size must be a multiple of 4MiB and no more
+    than 128MiB.
 `,
 	Example: `  dbxcli put file.txt /destination/file.txt
   dbxcli put -r ./project /backup/project
+  dbxcli put -w 1 -c 134217728 large.zip /backup/large.zip
   printf 'hello' | dbxcli put - /hello.txt
   tar cz ./src | dbxcli put - /backups/src.tgz`,
 	RunE: put,
@@ -901,8 +916,8 @@ func init() {
 	RootCmd.AddCommand(putCmd)
 	enableStructuredOutput(putCmd)
 	putCmd.Flags().BoolP("recursive", "r", false, "Recursively upload directories")
-	putCmd.Flags().IntP("workers", "w", 4, "Number of concurrent upload workers to use")
-	putCmd.Flags().Int64P("chunksize", "c", 1<<24, "Chunk size to use (should be multiple of 4MiB)")
+	putCmd.Flags().IntP("workers", "w", 4, "Number of concurrent upload workers for chunked large-file uploads")
+	putCmd.Flags().Int64P("chunksize", "c", 1<<24, "Chunk size in bytes for chunked large-file uploads; must be a multiple of 4MiB and no more than 128MiB")
 	putCmd.Flags().BoolP("debug", "d", false, "Print debug timing")
 	putCmd.Flags().String("if-exists", putIfExistsOverwrite, "What to do when the destination file exists: overwrite, skip, or fail")
 }

cmd/put_test.go

@@ -667,16 +667,37 @@ func TestPutRecursive_SkipsSymlinks(t *testing.T) {
 }
 
 func TestPutChunkSizeValidation(t *testing.T) {
-	tmpFile := filepath.Join(t.TempDir(), "test.txt")
-	if err := os.WriteFile(tmpFile, []byte("test"), 0644); err != nil {
-		t.Fatal(err)
+	tests := []struct {
+		name      string
+		chunkSize string
+		want      string
+	}{
+		{
+			name:      "below 4MiB",
+			chunkSize: "100",
+			want:      "`put` requires chunk size to be at least 4MiB",
+		},
+		{
+			name:      "not multiple of 4MiB",
+			chunkSize: "6291456",
+			want:      "`put` requires chunk size to be a multiple of 4MiB",
+		},
+		{
+			name:      "above Dropbox request limit",
+			chunkSize: "268435456",
+			want:      "`put` requires chunk size to be no more than 128MiB",
+		},
 	}
 
-	cmd := testPutCmd()
-	_ = cmd.Flags().Set("chunksize", "100")
-	err := put(cmd, []string{tmpFile, "/test.txt"})
-	if err == nil || err.Error() != "`put` requires chunk size to be multiple of 4MiB" {
-		t.Errorf("expected chunk size validation error, got %v", err)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := testPutCmd()
+			_ = cmd.Flags().Set("chunksize", tt.chunkSize)
+			_, err := parsePutOptions(cmd)
+			if err == nil || err.Error() != tt.want {
+				t.Errorf("expected chunk size validation error %q, got %v", tt.want, err)
+			}
+		})
 	}
 }
 

docs/commands/dbxcli_put.md

@@ -12,6 +12,9 @@ Upload files or directories to Dropbox.
   - 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.
+  - Files larger than 32MiB use Dropbox upload sessions. Each chunk is one
+    upload-session request; chunk size must be a multiple of 4MiB and no more
+    than 128MiB.
 
 
 ```
@@ -23,19 +26,20 @@ dbxcli put [flags] <source> [<target>]
 ```
   dbxcli put file.txt /destination/file.txt
   dbxcli put -r ./project /backup/project
+  dbxcli put -w 1 -c 134217728 large.zip /backup/large.zip
   printf 'hello' | dbxcli put - /hello.txt
   tar cz ./src | dbxcli put - /backups/src.tgz
 ```
 
 ### Options
 
 ```
-  -c, --chunksize int      Chunk size to use (should be multiple of 4MiB) (default 16777216)
+  -c, --chunksize int      Chunk size in bytes for chunked large-file uploads; must be a multiple of 4MiB and no more than 128MiB (default 16777216)
   -d, --debug              Print debug timing
   -h, --help               help for put
       --if-exists string   What to do when the destination file exists: overwrite, skip, or fail (default "overwrite")
   -r, --recursive          Recursively upload directories
-  -w, --workers int        Number of concurrent upload workers to use (default 4)
+  -w, --workers int        Number of concurrent upload workers for chunked large-file uploads (default 4)
 ```
 
 ### Options inherited from parent commands