Skip to content

Commit 16d68af

Browse files
committed
fix(desktop): properly de-dupe and validate paths
Signed-off-by: Milas Bowman <[email protected]>
1 parent 343f384 commit 16d68af

File tree

6 files changed

+70
-59
lines changed

6 files changed

+70
-59
lines changed

Diff for: internal/desktop/file_shares.go

+2-9
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23-
"path/filepath"
2423
"strings"
2524
"sync"
2625

@@ -124,19 +123,13 @@ type FileShareManager struct {
124123
state map[string]*FileShareSession
125124
}
126125

127-
func NewFileShareManager(cli *Client, projectName string, hostPaths []string) (*FileShareManager, error) {
128-
for _, p := range hostPaths {
129-
if !filepath.IsAbs(p) {
130-
return nil, fmt.Errorf("file share path is not absolute: %s", p)
131-
}
132-
}
133-
126+
func NewFileShareManager(cli *Client, projectName string, hostPaths []string) *FileShareManager {
134127
return &FileShareManager{
135128
cli: cli,
136129
projectName: projectName,
137130
hostPaths: hostPaths,
138131
state: make(map[string]*FileShareSession),
139-
}, nil
132+
}
140133
}
141134

142135
// EnsureExists looks for existing File Shares or creates new ones for the

Diff for: internal/paths/paths.go

+42
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,45 @@ func IsChild(dir string, file string) bool {
7676
current = cDir
7777
}
7878
}
79+
80+
// EncompassingPaths returns the minimal set of paths that root all paths
81+
// from the original collection.
82+
//
83+
// For example, ["/foo", "/foo/bar", "/foo", "/baz"] -> ["/foo", "/baz].
84+
func EncompassingPaths(paths []string) []string {
85+
result := []string{}
86+
for _, current := range paths {
87+
isCovered := false
88+
hasRemovals := false
89+
90+
for i, existing := range result {
91+
if IsChild(existing, current) {
92+
// The path is already covered, so there's no need to include it
93+
isCovered = true
94+
break
95+
}
96+
97+
if IsChild(current, existing) {
98+
// Mark the element empty for removal.
99+
result[i] = ""
100+
hasRemovals = true
101+
}
102+
}
103+
104+
if !isCovered {
105+
result = append(result, current)
106+
}
107+
108+
if hasRemovals {
109+
// Remove all the empties
110+
newResult := []string{}
111+
for _, r := range result {
112+
if r != "" {
113+
newResult = append(newResult, r)
114+
}
115+
}
116+
result = newResult
117+
}
118+
}
119+
return result
120+
}

Diff for: pkg/compose/create.go

+22-5
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,16 @@ import (
2222
"encoding/json"
2323
"errors"
2424
"fmt"
25+
"io/fs"
2526
"os"
2627
"path"
2728
"path/filepath"
29+
"sort"
2830
"strconv"
2931
"strings"
3032

3133
"github.com/docker/compose/v2/internal/desktop"
34+
pathutil "github.com/docker/compose/v2/internal/paths"
3235
moby "github.com/docker/docker/api/types"
3336
"github.com/docker/docker/api/types/blkiodev"
3437
"github.com/docker/docker/api/types/container"
@@ -161,13 +164,27 @@ func (s *composeService) ensureProjectVolumes(ctx context.Context, project *type
161164
if vol.Type != string(mount.TypeBind) {
162165
continue
163166
}
164-
paths = append(paths, vol.Source)
167+
p := filepath.Clean(vol.Source)
168+
if !filepath.IsAbs(p) {
169+
return fmt.Errorf("file share path is not absolute: %s", p)
170+
}
171+
if _, err := os.Stat(p); errors.Is(err, fs.ErrNotExist) {
172+
if vol.Bind != nil && !vol.Bind.CreateHostPath {
173+
return fmt.Errorf("service %s: host path %q does not exist and `create_host_path` is false", svcName, vol.Source)
174+
}
175+
if err := os.MkdirAll(p, 0o755); err != nil {
176+
return fmt.Errorf("creating host path: %w", err)
177+
}
178+
}
179+
paths = append(paths, p)
165180
}
166181
}
167-
fileShareManager, err := desktop.NewFileShareManager(s.desktopCli, project.Name, paths)
168-
if err != nil {
169-
return fmt.Errorf("creating file share manager: %w", err)
170-
}
182+
183+
// remove duplicate/unnecessary child paths and sort them for predictability
184+
paths = pathutil.EncompassingPaths(paths)
185+
sort.Strings(paths)
186+
187+
fileShareManager := desktop.NewFileShareManager(s.desktopCli, project.Name, paths)
171188
if err := fileShareManager.EnsureExists(ctx); err != nil {
172189
return fmt.Errorf("initializing file shares: %w", err)
173190
}

Diff for: pkg/watch/paths.go

-42
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"fmt"
2121
"os"
2222
"path/filepath"
23-
24-
pathutil "github.com/docker/compose/v2/internal/paths"
2523
)
2624

2725
func greatestExistingAncestor(path string) (string, error) {
@@ -41,43 +39,3 @@ func greatestExistingAncestor(path string) (string, error) {
4139

4240
return path, nil
4341
}
44-
45-
// If we're recursively watching a path, it doesn't
46-
// make sense to watch any of its descendants.
47-
func dedupePathsForRecursiveWatcher(paths []string) []string {
48-
result := []string{}
49-
for _, current := range paths {
50-
isCovered := false
51-
hasRemovals := false
52-
53-
for i, existing := range result {
54-
if pathutil.IsChild(existing, current) {
55-
// The path is already covered, so there's no need to include it
56-
isCovered = true
57-
break
58-
}
59-
60-
if pathutil.IsChild(current, existing) {
61-
// Mark the element empty for removal.
62-
result[i] = ""
63-
hasRemovals = true
64-
}
65-
}
66-
67-
if !isCovered {
68-
result = append(result, current)
69-
}
70-
71-
if hasRemovals {
72-
// Remove all the empties
73-
newResult := []string{}
74-
for _, r := range result {
75-
if r != "" {
76-
newResult = append(newResult, r)
77-
}
78-
}
79-
result = newResult
80-
}
81-
}
82-
return result
83-
}

Diff for: pkg/watch/watcher_darwin.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"path/filepath"
2626
"time"
2727

28+
pathutil "github.com/docker/compose/v2/internal/paths"
2829
"github.com/fsnotify/fsevents"
2930
"github.com/sirupsen/logrus"
3031
)
@@ -132,7 +133,7 @@ func newWatcher(paths []string, ignore PathMatcher) (Notify, error) {
132133
stop: make(chan struct{}),
133134
}
134135

135-
paths = dedupePathsForRecursiveWatcher(paths)
136+
paths = pathutil.EncompassingPaths(paths)
136137
for _, path := range paths {
137138
path, err := filepath.Abs(path)
138139
if err != nil {

Diff for: pkg/watch/watcher_naive.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func (d *naiveNotify) Start() error {
7171
return err
7272
}
7373
if d.isWatcherRecursive {
74-
pathsToWatch = dedupePathsForRecursiveWatcher(pathsToWatch)
74+
pathsToWatch = pathutil.EncompassingPaths(pathsToWatch)
7575
}
7676

7777
for _, name := range pathsToWatch {
@@ -320,7 +320,7 @@ func newWatcher(paths []string, ignore PathMatcher) (Notify, error) {
320320
wrappedEvents := make(chan FileEvent)
321321
notifyList := make(map[string]bool, len(paths))
322322
if isWatcherRecursive {
323-
paths = dedupePathsForRecursiveWatcher(paths)
323+
paths = pathutil.EncompassingPaths(paths)
324324
}
325325
for _, path := range paths {
326326
path, err := filepath.Abs(path)

0 commit comments

Comments
 (0)