Skip to content

Commit 7274705

Browse files
flouthocflouthoc
committed
add,copy: if multiple sources then dest must end with a slash
When using `ADD` or `COPY` enforce the condition that destination must look like a directory by making sure that destination ends with a slash `/`. This ensures that we don't hit undefined behaviour for example with the use case `COPY some/* /oncontainer` it is not clear that `oncontainer` will be a directory or a file, users expect it to be a directory if `*` wildcard resolves to multiple files and logically if wildcard resolves to single file then it should be a directory, since this condition is undefined and not in parity with docker so lets drop support for it. Docker's defined rule: * If multiple <src> resources are specified, either directly or due to the use of a wildcard, then <dest> must be a directory, and it must end with a slash /. Reference: https://docs.docker.com/engine/reference/builder/#add Closes: containers#4167 Signed-off-by: Aditya R <arajan@redhat.com>
1 parent 18cd2d5 commit 7274705

File tree

3 files changed

+71
-8
lines changed

3 files changed

+71
-8
lines changed

‎add.go

+29
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,39 @@ func includeDirectoryAnyway(path string, pm *fileutils.PatternMatcher) bool {
188188
return false
189189
}
190190

191+
// validateSourceAndDest performs a sanity check on the case that if more
192+
// than one source is provided then destination must end with a slash (`/`),
193+
// which signifies that destination is a directory and similarly function
194+
// returns error if destination does not ends with a slash (`/`).
195+
// Following function can contain more validation cases in future.
196+
func validateSourceAndDest(destination string, sources []string) error {
197+
multipleSources := false
198+
if len(sources) > 1 {
199+
multipleSources = true
200+
} else {
201+
// Check length before accessing `0`
202+
// to avoid `index out of range`.
203+
if len(sources) == 1 {
204+
if strings.HasSuffix(sources[0], "*") {
205+
multipleSources = true
206+
}
207+
}
208+
}
209+
if multipleSources {
210+
if !strings.HasSuffix(destination, "/") {
211+
return errors.New("adding multiple sources to non-directory destination")
212+
}
213+
}
214+
return nil
215+
}
216+
191217
// Add copies the contents of the specified sources into the container's root
192218
// filesystem, optionally extracting contents of local files that look like
193219
// non-empty archives.
194220
func (b *Builder) Add(destination string, extract bool, options AddAndCopyOptions, sources ...string) error {
221+
if err := validateSourceAndDest(destination, sources); err != nil {
222+
return err
223+
}
195224
mountPoint, err := b.Mount(b.MountLabel)
196225
if err != nil {
197226
return err

‎tests/add.bats

+21-3
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@ load helpers
2828
# Copy a file to a specific subdirectory
2929
run_buildah add $cid ${TEST_SCRATCH_DIR}/randomfile /subdir
3030
# Copy two files to a specific subdirectory
31-
run_buildah add $cid ${TEST_SCRATCH_DIR}/randomfile ${TEST_SCRATCH_DIR}/other-randomfile /other-subdir
31+
run_buildah add $cid ${TEST_SCRATCH_DIR}/randomfile ${TEST_SCRATCH_DIR}/other-randomfile /other-subdir/
3232
# Copy two files to a specific location, which succeeds because we can create it as a directory.
33-
run_buildah add $cid ${TEST_SCRATCH_DIR}/randomfile ${TEST_SCRATCH_DIR}/other-randomfile /notthereyet-subdir
33+
run_buildah add $cid ${TEST_SCRATCH_DIR}/randomfile ${TEST_SCRATCH_DIR}/other-randomfile /notthereyet-subdir/
34+
# Add with a wildcard and destination dir does not exists
35+
run_buildah add $cid ${TEST_SCRATCH_DIR}/* /notthereyet2-subdir/
3436
# Copy two files to a specific location, which fails because it's not a directory.
35-
run_buildah 125 add $cid ${TEST_SCRATCH_DIR}/randomfile ${TEST_SCRATCH_DIR}/other-randomfile /randomfile
37+
run_buildah 125 add $cid ${TEST_SCRATCH_DIR}/randomfile ${TEST_SCRATCH_DIR}/other-randomfile /randomfile/
3638
# Copy a file to a different working directory
3739
run_buildah config --workingdir=/cwd $cid
3840
run_buildah add $cid ${TEST_SCRATCH_DIR}/randomfile
@@ -58,6 +60,22 @@ load helpers
5860
run_buildah rm $newcid
5961
}
6062

63+
@test "add-multiple-files to destination not ending with slash" {
64+
createrandom ${TEST_SCRATCH_DIR}/randomfile
65+
createrandom ${TEST_SCRATCH_DIR}/other-randomfile
66+
createrandom ${TEST_SCRATCH_DIR}/third-randomfile
67+
68+
run_buildah from $WITH_POLICY_JSON scratch
69+
cid=$output
70+
run_buildah mount $cid
71+
root=$output
72+
run_buildah config --workingdir / $cid
73+
run_buildah 125 add $cid ${TEST_SCRATCH_DIR}/* /test
74+
expect_output --substring "adding multiple sources to non-directory destination"
75+
run_buildah 125 add $cid ${TEST_SCRATCH_DIR}/randomefile ${TEST_SCRATCH_DIR}/other-randomfile /test
76+
expect_output --substring "adding multiple sources to non-directory destination"
77+
}
78+
6179
@test "add-local-archive" {
6280
createrandom ${TEST_SCRATCH_DIR}/randomfile
6381
createrandom ${TEST_SCRATCH_DIR}/other-randomfile

‎tests/copy.bats

+21-5
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ load helpers
2626
# copy ${TEST_SCRATCH_DIR}/randomfile to a file of the same name in the container's working directory
2727
run_buildah copy $cid ${TEST_SCRATCH_DIR}/randomfile
2828
# copy ${TEST_SCRATCH_DIR}/other-randomfile and ${TEST_SCRATCH_DIR}/third-randomfile to a new directory named ${TEST_SCRATCH_DIR}/randomfile in the container
29-
run_buildah copy $cid ${TEST_SCRATCH_DIR}/other-randomfile ${TEST_SCRATCH_DIR}/third-randomfile ${TEST_SCRATCH_DIR}/randomfile
29+
run_buildah copy $cid ${TEST_SCRATCH_DIR}/other-randomfile ${TEST_SCRATCH_DIR}/third-randomfile ${TEST_SCRATCH_DIR}/randomfile/
3030
# try to copy ${TEST_SCRATCH_DIR}/other-randomfile and ${TEST_SCRATCH_DIR}/third-randomfile to a /randomfile, which already exists and is a file
31-
run_buildah 125 copy $cid ${TEST_SCRATCH_DIR}/other-randomfile ${TEST_SCRATCH_DIR}/third-randomfile /randomfile
31+
run_buildah 125 copy $cid ${TEST_SCRATCH_DIR}/other-randomfile ${TEST_SCRATCH_DIR}/third-randomfile /randomfile/
3232
# copy ${TEST_SCRATCH_DIR}/other-randomfile and ${TEST_SCRATCH_DIR}/third-randomfile to previously-created directory named ${TEST_SCRATCH_DIR}/randomfile in the container
33-
run_buildah copy $cid ${TEST_SCRATCH_DIR}/other-randomfile ${TEST_SCRATCH_DIR}/third-randomfile ${TEST_SCRATCH_DIR}/randomfile
33+
run_buildah copy $cid ${TEST_SCRATCH_DIR}/other-randomfile ${TEST_SCRATCH_DIR}/third-randomfile ${TEST_SCRATCH_DIR}/randomfile/
3434
run_buildah rm $cid
3535

3636
_prefetch alpine
@@ -40,15 +40,15 @@ load helpers
4040
root=$output
4141
run_buildah config --workingdir / $cid
4242
run_buildah copy $cid ${TEST_SCRATCH_DIR}/randomfile
43-
run_buildah copy $cid ${TEST_SCRATCH_DIR}/other-randomfile ${TEST_SCRATCH_DIR}/third-randomfile ${TEST_SCRATCH_DIR}/randomfile /etc
43+
run_buildah copy $cid ${TEST_SCRATCH_DIR}/other-randomfile ${TEST_SCRATCH_DIR}/third-randomfile ${TEST_SCRATCH_DIR}/randomfile /etc/
4444
run_buildah rm $cid
4545

4646
run_buildah from --quiet --pull=false $WITH_POLICY_JSON alpine
4747
cid=$output
4848
run_buildah mount $cid
4949
root=$output
5050
run_buildah config --workingdir / $cid
51-
run_buildah copy $cid "${TEST_SCRATCH_DIR}/*randomfile" /etc
51+
run_buildah copy $cid "${TEST_SCRATCH_DIR}/*randomfile" /etc/
5252
(cd ${TEST_SCRATCH_DIR}; for i in *randomfile; do cmp $i ${root}/etc/$i; done)
5353
}
5454

@@ -78,6 +78,22 @@ load helpers
7878
cmp ${TEST_SCRATCH_DIR}/other-randomfile $newroot/other-randomfile
7979
}
8080

81+
@test "copy-multiple-files to destination not ending with slash" {
82+
createrandom ${TEST_SCRATCH_DIR}/randomfile
83+
createrandom ${TEST_SCRATCH_DIR}/other-randomfile
84+
createrandom ${TEST_SCRATCH_DIR}/third-randomfile
85+
86+
run_buildah from $WITH_POLICY_JSON scratch
87+
cid=$output
88+
run_buildah mount $cid
89+
root=$output
90+
run_buildah config --workingdir / $cid
91+
run_buildah 125 copy $cid ${TEST_SCRATCH_DIR}/* /test
92+
expect_output --substring "adding multiple sources to non-directory destination"
93+
run_buildah 125 copy $cid ${TEST_SCRATCH_DIR}/randomefile ${TEST_SCRATCH_DIR}/other-randomfile /test
94+
expect_output --substring "adding multiple sources to non-directory destination"
95+
}
96+
8197
@test "copy-local-subdirectory" {
8298
mkdir -p ${TEST_SCRATCH_DIR}/subdir
8399
createrandom ${TEST_SCRATCH_DIR}/subdir/randomfile

0 commit comments

Comments
 (0)