Browse code

Fix opq whiteouts problems for files with dot prefix

Fixes #17766

Previously, opaque directory whiteouts on non-native
graphdrivers depended on the file order, meaning
files added with the same layer before the whiteout
file `.wh..wh..opq` were also removed.

If that file happened to have subdirs, then calling
chtimes on those dirs after unpack would fail the pull.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

Tonis Tiigi authored on 2015/11/09 16:00:01
Showing 2 changed files
... ...
@@ -25,6 +25,7 @@ func UnpackLayer(dest string, layer Reader, options *TarOptions) (size int64, er
25 25
 	defer pools.BufioReader32KPool.Put(trBuf)
26 26
 
27 27
 	var dirs []*tar.Header
28
+	unpackedPaths := make(map[string]struct{})
28 29
 
29 30
 	if options == nil {
30 31
 		options = &TarOptions{}
... ...
@@ -134,14 +135,27 @@ func UnpackLayer(dest string, layer Reader, options *TarOptions) (size int64, er
134 134
 		if strings.HasPrefix(base, WhiteoutPrefix) {
135 135
 			dir := filepath.Dir(path)
136 136
 			if base == WhiteoutOpaqueDir {
137
-				fi, err := os.Lstat(dir)
138
-				if err != nil && !os.IsNotExist(err) {
139
-					return 0, err
140
-				}
141
-				if err := os.RemoveAll(dir); err != nil {
137
+				_, err := os.Lstat(dir)
138
+				if err != nil {
142 139
 					return 0, err
143 140
 				}
144
-				if err := os.Mkdir(dir, fi.Mode()&os.ModePerm); err != nil {
141
+				err = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
142
+					if err != nil {
143
+						if os.IsNotExist(err) {
144
+							err = nil // parent was deleted
145
+						}
146
+						return err
147
+					}
148
+					if path == dir {
149
+						return nil
150
+					}
151
+					if _, exists := unpackedPaths[path]; !exists {
152
+						err := os.RemoveAll(path)
153
+						return err
154
+					}
155
+					return nil
156
+				})
157
+				if err != nil {
145 158
 					return 0, err
146 159
 				}
147 160
 			} else {
... ...
@@ -214,6 +228,7 @@ func UnpackLayer(dest string, layer Reader, options *TarOptions) (size int64, er
214 214
 			if hdr.Typeflag == tar.TypeDir {
215 215
 				dirs = append(dirs, hdr)
216 216
 			}
217
+			unpackedPaths[path] = struct{}{}
217 218
 		}
218 219
 	}
219 220
 
... ...
@@ -2,7 +2,14 @@ package archive
2 2
 
3 3
 import (
4 4
 	"archive/tar"
5
+	"io"
6
+	"io/ioutil"
7
+	"os"
8
+	"path/filepath"
9
+	"reflect"
5 10
 	"testing"
11
+
12
+	"github.com/docker/docker/pkg/ioutils"
6 13
 )
7 14
 
8 15
 func TestApplyLayerInvalidFilenames(t *testing.T) {
... ...
@@ -188,3 +195,176 @@ func TestApplyLayerInvalidSymlink(t *testing.T) {
188 188
 		}
189 189
 	}
190 190
 }
191
+
192
+func TestApplyLayerWhiteouts(t *testing.T) {
193
+	wd, err := ioutil.TempDir("", "graphdriver-test-whiteouts")
194
+	if err != nil {
195
+		return
196
+	}
197
+	defer os.RemoveAll(wd)
198
+
199
+	base := []string{
200
+		".baz",
201
+		"bar/",
202
+		"bar/bax",
203
+		"bar/bay/",
204
+		"baz",
205
+		"foo/",
206
+		"foo/.abc",
207
+		"foo/.bcd/",
208
+		"foo/.bcd/a",
209
+		"foo/cde/",
210
+		"foo/cde/def",
211
+		"foo/cde/efg",
212
+		"foo/fgh",
213
+		"foobar",
214
+	}
215
+
216
+	type tcase struct {
217
+		change, expected []string
218
+	}
219
+
220
+	tcases := []tcase{
221
+		{
222
+			base,
223
+			base,
224
+		},
225
+		{
226
+			[]string{
227
+				".bay",
228
+				".wh.baz",
229
+				"foo/",
230
+				"foo/.bce",
231
+				"foo/.wh..wh..opq",
232
+				"foo/cde/",
233
+				"foo/cde/efg",
234
+			},
235
+			[]string{
236
+				".bay",
237
+				".baz",
238
+				"bar/",
239
+				"bar/bax",
240
+				"bar/bay/",
241
+				"foo/",
242
+				"foo/.bce",
243
+				"foo/cde/",
244
+				"foo/cde/efg",
245
+				"foobar",
246
+			},
247
+		},
248
+		{
249
+			[]string{
250
+				".bay",
251
+				".wh..baz",
252
+				".wh.foobar",
253
+				"foo/",
254
+				"foo/.abc",
255
+				"foo/.wh.cde",
256
+				"bar/",
257
+			},
258
+			[]string{
259
+				".bay",
260
+				"bar/",
261
+				"bar/bax",
262
+				"bar/bay/",
263
+				"foo/",
264
+				"foo/.abc",
265
+				"foo/.bce",
266
+			},
267
+		},
268
+		{
269
+			[]string{
270
+				".abc",
271
+				".wh..wh..opq",
272
+				"foobar",
273
+			},
274
+			[]string{
275
+				".abc",
276
+				"foobar",
277
+			},
278
+		},
279
+	}
280
+
281
+	for i, tc := range tcases {
282
+		l, err := makeTestLayer(tc.change)
283
+		if err != nil {
284
+			t.Fatal(err)
285
+		}
286
+
287
+		_, err = UnpackLayer(wd, l, nil)
288
+		if err != nil {
289
+			t.Fatal(err)
290
+		}
291
+		err = l.Close()
292
+		if err != nil {
293
+			t.Fatal(err)
294
+		}
295
+
296
+		paths, err := readDirContents(wd)
297
+		if err != nil {
298
+			t.Fatal(err)
299
+		}
300
+
301
+		if !reflect.DeepEqual(tc.expected, paths) {
302
+			t.Fatalf("invalid files for layer %d: expected %q, got %q", i, tc.expected, paths)
303
+		}
304
+	}
305
+
306
+}
307
+
308
+func makeTestLayer(paths []string) (rc io.ReadCloser, err error) {
309
+	tmpDir, err := ioutil.TempDir("", "graphdriver-test-mklayer")
310
+	if err != nil {
311
+		return
312
+	}
313
+	defer func() {
314
+		if err != nil {
315
+			os.RemoveAll(tmpDir)
316
+		}
317
+	}()
318
+	for _, p := range paths {
319
+		if p[len(p)-1] == filepath.Separator {
320
+			if err = os.MkdirAll(filepath.Join(tmpDir, p), 0700); err != nil {
321
+				return
322
+			}
323
+		} else {
324
+			if err = ioutil.WriteFile(filepath.Join(tmpDir, p), nil, 0600); err != nil {
325
+				return
326
+			}
327
+		}
328
+	}
329
+	archive, err := Tar(tmpDir, Uncompressed)
330
+	if err != nil {
331
+		return
332
+	}
333
+	return ioutils.NewReadCloserWrapper(archive, func() error {
334
+		err := archive.Close()
335
+		os.RemoveAll(tmpDir)
336
+		return err
337
+	}), nil
338
+}
339
+
340
+func readDirContents(root string) ([]string, error) {
341
+	var files []string
342
+	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
343
+		if err != nil {
344
+			return err
345
+		}
346
+		if path == root {
347
+			return nil
348
+		}
349
+		rel, err := filepath.Rel(root, path)
350
+		if err != nil {
351
+			return err
352
+		}
353
+		if info.IsDir() {
354
+			rel = rel + "/"
355
+		}
356
+		files = append(files, rel)
357
+		return nil
358
+	})
359
+	if err != nil {
360
+		return nil, err
361
+	}
362
+	return files, nil
363
+}