Skip to content

Fix recursive downloads of unmounted shared folders #401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AndreyVMarkelov merged 1 commit into from
Nov 12, 2026
Merged

Fix recursive downloads of unmounted shared folders #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 10 additions & 29 deletions cmd/share_link_download.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func shareLinkDownload(cmd *cobra.Command, args []string) error {
if err != nil {
return withJSONErrorDetails(err, urlErrorDetails(url), operationErrorDetails("share_link_download"))
}
if err := downloadSharedLinkFolder(filesNewFunc(config), dbx, arg, folder.Name, dst, cmd.ErrOrStderr()); err != nil {
if err := downloadSharedLinkFolder(filesNewFunc(config), dbx, arg, dst, cmd.ErrOrStderr()); err != nil {
return withJSONErrorDetails(err, urlErrorDetails(url), operationErrorDetails("share_link_download"))
}
commandVerboseStatus(cmd, "Downloaded shared link folder to %s", dst)
Expand Down Expand Up @@ -209,7 +209,7 @@ func sharedLinkFolderDownloadTarget(target string, link *sharing.FolderLinkMetad
return target, nil
}

func downloadSharedLinkFolder(filesDbx filesClient, dbx sharedLinkClient, arg *sharing.GetSharedLinkMetadataArg, rootName, dst string, errOut io.Writer) error {
func downloadSharedLinkFolder(filesDbx filesClient, dbx sharedLinkClient, arg *sharing.GetSharedLinkMetadataArg, dst string, errOut io.Writer) error {
if errOut == nil {
errOut = io.Discard
}
Expand Down Expand Up @@ -238,14 +238,11 @@ func downloadSharedLinkFolder(filesDbx filesClient, dbx sharedLinkClient, arg *s
for _, entry := range entries {
switch f := entry.(type) {
case *files.FolderMetadata:
relPath, err := sharedLinkEntryRelativePath(f.PathDisplay, rootName)
relPath, err := sharedLinkEntryRelativePath(relFolder, f.Name)
if err != nil {
downloadErrors = append(downloadErrors, err)
continue
}
if relPath == "" {
continue
}
localDir, err := sharedLinkLocalPath(dst, relPath)
if err != nil {
downloadErrors = append(downloadErrors, err)
Expand All @@ -257,7 +254,7 @@ func downloadSharedLinkFolder(filesDbx filesClient, dbx sharedLinkClient, arg *s
}
queue = append(queue, relPath)
case *files.FileMetadata:
relPath, err := sharedLinkEntryRelativePath(f.PathDisplay, rootName)
relPath, err := sharedLinkEntryRelativePath(relFolder, f.Name)
if err != nil {
downloadErrors = append(downloadErrors, err)
continue
Expand Down Expand Up @@ -334,29 +331,13 @@ func downloadSharedLinkRelativeFile(dbx sharedLinkClient, baseArg *sharing.GetSh
})
}

func sharedLinkEntryRelativePath(pathDisplay string, rootName string) (string, error) {
rel := strings.TrimPrefix(pathDisplay, "/")
if rootName != "" {
rootName = strings.Trim(rootName, "/")
first, rest, ok := strings.Cut(rel, "/")
if strings.EqualFold(first, rootName) {
if !ok {
return "", nil
}
rel = rest
}
}
parts := strings.Split(rel, "/")
for _, part := range parts {
if part == "" || part == "." || part == ".." {
return "", fmt.Errorf("invalid shared link entry path %q", pathDisplay)
}
}
rel = path.Clean(rel)
if rel == "" || rel == "." || rel == ".." || strings.HasPrefix(rel, "../") {
return "", fmt.Errorf("invalid shared link entry path %q", pathDisplay)
// Shared-link listings may omit path_display and path_lower when the folder is
// not mounted in the caller's Dropbox. Names are relative to the folder listed.
func sharedLinkEntryRelativePath(parentRel, name string) (string, error) {
if name == "." || !filepath.IsLocal(name) || strings.ContainsAny(name, `/\`) {
return "", fmt.Errorf("invalid shared link entry name %q", name)
}
return rel, nil
return path.Join(parentRel, name), nil
}

func sharedLinkAPIPath(rel string) string {
Expand Down
248 changes: 129 additions & 119 deletions cmd/share_link_download_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,86 +462,98 @@ func TestShareLinkDownloadFolderRejectsStdoutTarget(t *testing.T) {
}

func TestShareLinkDownloadFolderRecursiveDownloadsNestedFiles(t *testing.T) {
tmp := t.TempDir()
target := filepath.Join(tmp, "out")
var listed []string
var listedPassword string
var downloaded []string
for _, mounted := range []bool{false, true} {
t.Run(fmt.Sprintf("mounted=%t", mounted), func(t *testing.T) {
metadata := func(name, relativePath string) files.Metadata {
m := files.Metadata{Name: name}
if mounted {
m.PathDisplay = "/parent/docs/" + relativePath
m.PathLower = strings.ToLower(m.PathDisplay)
}
return m
}
tmp := t.TempDir()
target := filepath.Join(tmp, "out")
var listed []string
var listedPassword string
var downloaded []string

stubSharedLinkClient(t, &mockSharedLinkClient{
getSharedLinkMetadataFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, error) {
if arg.LinkPassword != "secret" {
t.Fatalf("metadata password = %q, want secret", arg.LinkPassword)
stubSharedLinkClient(t, &mockSharedLinkClient{
getSharedLinkMetadataFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, error) {
if arg.LinkPassword != "secret" {
t.Fatalf("metadata password = %q, want secret", arg.LinkPassword)
}
return sharedLinkFolder("/docs", "https://example.com/folder"), nil
},
getSharedLinkFileFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, io.ReadCloser, error) {
downloaded = append(downloaded, arg.Path)
if arg.LinkPassword != "secret" {
t.Fatalf("download password = %q, want secret", arg.LinkPassword)
}
contents := strings.TrimPrefix(arg.Path, "/")
return downloadableSharedLinkFile(filepath.Base(arg.Path), arg.Path, "https://example.com/folder", uint64(len(contents))),
io.NopCloser(strings.NewReader(contents)), nil
},
})
stubFilesClient(t, &mockFilesClient{
listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) {
listed = append(listed, arg.Path)
if arg.SharedLink == nil {
t.Fatal("SharedLink = nil, want shared-link listing")
}
if arg.SharedLink.Url != "https://example.com/folder" {
t.Fatalf("shared link URL = %q, want https://example.com/folder", arg.SharedLink.Url)
}
listedPassword = arg.SharedLink.Password
if arg.Recursive {
t.Fatal("Recursive = true, want manual recursion for shared links")
}
switch arg.Path {
case "":
return &files.ListFolderResult{
Entries: []files.IsMetadata{
&files.FileMetadata{Metadata: metadata("root.txt", "root.txt"), Size: 8},
&files.FolderMetadata{Metadata: metadata("sub", "sub")},
},
}, nil
case "/sub":
return &files.ListFolderResult{
Entries: []files.IsMetadata{
&files.FileMetadata{Metadata: metadata("deep.txt", "sub/deep.txt"), Size: 12},
},
}, nil
default:
t.Fatalf("unexpected list path %q", arg.Path)
}
return nil, nil
},
})

cmd := newShareLinkDownloadTestCommand(nil, nil)
if err := cmd.Flags().Set("recursive", "true"); err != nil {
t.Fatalf("set recursive: %v", err)
}
return sharedLinkFolder("/docs", "https://example.com/folder"), nil
},
getSharedLinkFileFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, io.ReadCloser, error) {
downloaded = append(downloaded, arg.Path)
if arg.LinkPassword != "secret" {
t.Fatalf("download password = %q, want secret", arg.LinkPassword)
if err := cmd.Flags().Set("password", "secret"); err != nil {
t.Fatalf("set password: %v", err)
}
contents := strings.TrimPrefix(arg.Path, "/")
return downloadableSharedLinkFile(filepath.Base(arg.Path), arg.Path, "https://example.com/folder", uint64(len(contents))),
io.NopCloser(strings.NewReader(contents)), nil
},
})
stubFilesClient(t, &mockFilesClient{
listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) {
listed = append(listed, arg.Path)
if arg.SharedLink == nil {
t.Fatal("SharedLink = nil, want shared-link listing")

if err := shareLinkDownload(cmd, []string{"https://example.com/folder", target}); err != nil {
t.Fatalf("shareLinkDownload error: %v", err)
}
if arg.SharedLink.Url != "https://example.com/folder" {
t.Fatalf("shared link URL = %q, want https://example.com/folder", arg.SharedLink.Url)

if strings.Join(listed, ",") != ",/sub" {
t.Fatalf("listed paths = %q, want root then sub", strings.Join(listed, ","))
}
listedPassword = arg.SharedLink.Password
if arg.Recursive {
t.Fatal("Recursive = true, want manual recursion for shared links")
if listedPassword != "secret" {
t.Fatalf("listed password = %q, want secret", listedPassword)
}
switch arg.Path {
case "":
return &files.ListFolderResult{
Entries: []files.IsMetadata{
&files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/root.txt"}, Size: 8},
&files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/docs/sub"}},
},
}, nil
case "/sub":
return &files.ListFolderResult{
Entries: []files.IsMetadata{
&files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/sub/deep.txt"}, Size: 12},
},
}, nil
default:
t.Fatalf("unexpected list path %q", arg.Path)
if strings.Join(downloaded, ",") != "/root.txt,/sub/deep.txt" {
t.Fatalf("downloaded paths = %q, want root and nested file", strings.Join(downloaded, ","))
}
return nil, nil
},
})

cmd := newShareLinkDownloadTestCommand(nil, nil)
if err := cmd.Flags().Set("recursive", "true"); err != nil {
t.Fatalf("set recursive: %v", err)
}
if err := cmd.Flags().Set("password", "secret"); err != nil {
t.Fatalf("set password: %v", err)
}

if err := shareLinkDownload(cmd, []string{"https://example.com/folder", target}); err != nil {
t.Fatalf("shareLinkDownload error: %v", err)
}

if strings.Join(listed, ",") != ",/sub" {
t.Fatalf("listed paths = %q, want root then sub", strings.Join(listed, ","))
}
if listedPassword != "secret" {
t.Fatalf("listed password = %q, want secret", listedPassword)
}
if strings.Join(downloaded, ",") != "/root.txt,/sub/deep.txt" {
t.Fatalf("downloaded paths = %q, want root and nested file", strings.Join(downloaded, ","))
assertFileContent(t, filepath.Join(target, "root.txt"), "root.txt")
assertFileContent(t, filepath.Join(target, "sub", "deep.txt"), "sub/deep.txt")
})
}
assertFileContent(t, filepath.Join(target, "root.txt"), "root.txt")
assertFileContent(t, filepath.Join(target, "sub", "deep.txt"), "sub/deep.txt")
}

func TestShareLinkDownloadFolderUsesExistingTargetDirectory(t *testing.T) {
Expand All @@ -564,7 +576,7 @@ func TestShareLinkDownloadFolderUsesExistingTargetDirectory(t *testing.T) {
listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) {
return &files.ListFolderResult{
Entries: []files.IsMetadata{
&files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/root.txt"}, Size: 4},
&files.FileMetadata{Metadata: files.Metadata{Name: "root.txt"}, Size: 4},
},
}, nil
},
Expand Down Expand Up @@ -599,7 +611,7 @@ func TestShareLinkDownloadFolderFollowsPagination(t *testing.T) {
listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) {
return &files.ListFolderResult{
Entries: []files.IsMetadata{
&files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/page-one.txt"}, Size: 4},
&files.FileMetadata{Metadata: files.Metadata{Name: "page-one.txt"}, Size: 4},
},
HasMore: true,
Cursor: "cursor-2",
Expand All @@ -609,7 +621,7 @@ func TestShareLinkDownloadFolderFollowsPagination(t *testing.T) {
continued = arg.Cursor
return &files.ListFolderResult{
Entries: []files.IsMetadata{
&files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/page-two.txt"}, Size: 4},
&files.FileMetadata{Metadata: files.Metadata{Name: "page-two.txt"}, Size: 4},
},
}, nil
},
Expand Down Expand Up @@ -661,53 +673,51 @@ func TestShareLinkDownloadFolderDoesNotCreateTargetWhenRootListFails(t *testing.
}
}

func TestSharedLinkEntryRelativePathStripsRootBySegment(t *testing.T) {
tests := []struct {
name string
pathDisplay string
rootName string
want string
}{
{
name: "root itself",
pathDisplay: "/docs",
rootName: "docs",
want: "",
},
{
name: "child",
pathDisplay: "/docs/root.txt",
rootName: "docs",
want: "root.txt",
},
{
name: "case-insensitive root segment",
pathDisplay: "/DOCS/root.txt",
rootName: "docs",
want: "root.txt",
},
{
name: "partial prefix is not stripped",
pathDisplay: "/docs-other/root.txt",
rootName: "docs",
want: "docs-other/root.txt",
},
{
name: "unicode root segment",
pathDisplay: "/\u0130/root.txt",
rootName: "\u0130",
want: "root.txt",
},
func TestSharedLinkEntryRelativePath(t *testing.T) {
tests := []struct{ parent, name, want string }{
{"", "root.txt", "root.txt"},
{"sub/inner", "deep.txt", "sub/inner/deep.txt"},
{"docs", "docs", "docs/docs"},
{"", "İ.txt", "İ.txt"},
{"sub", "a..b", "sub/a..b"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := sharedLinkEntryRelativePath(tt.pathDisplay, tt.rootName)
if err != nil {
t.Fatalf("sharedLinkEntryRelativePath error: %v", err)
t.Run(tt.want, func(t *testing.T) {
got, err := sharedLinkEntryRelativePath(tt.parent, tt.name)
if err != nil || got != tt.want {
t.Fatalf("relative path = %q, %v, want %q", got, err, tt.want)
}
})
}
}

func TestShareLinkDownloadFolderRejectsUnsafeEntryNames(t *testing.T) {
for _, name := range []string{"", ".", "..", "../escape.txt", "a/b", `a\b`, `..\escape.txt`, "/absolute", `C:\escape.txt`} {
t.Run(name, func(t *testing.T) {
target := filepath.Join(t.TempDir(), "out")
listed := 0
filesDbx := &mockFilesClient{listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) {
listed++
if listed > 1 {
t.Fatal("unsafe folder was queued for listing")
}
return &files.ListFolderResult{Entries: []files.IsMetadata{
&files.FileMetadata{Metadata: files.Metadata{Name: name}},
&files.FolderMetadata{Metadata: files.Metadata{Name: name}},
}}, nil
}}
dbx := &mockSharedLinkClient{getSharedLinkFileFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, io.ReadCloser, error) {
t.Fatal("unsafe file was downloaded")
return nil, nil, nil
}}
var stderr bytes.Buffer
err := downloadSharedLinkFolder(filesDbx, dbx, sharing.NewGetSharedLinkMetadataArg("https://example.com/folder"), target, &stderr)
if err == nil || !strings.Contains(err.Error(), "2 error(s)") {
t.Fatalf("error = %v, want 2 errors", err)
}
if got != tt.want {
t.Fatalf("relative path = %q, want %q", got, tt.want)
entries, err := os.ReadDir(target)
if err != nil || len(entries) != 0 {
t.Fatalf("target entries = %v, %v, want empty directory", entries, err)
}
})
}
Expand Down
Loading