[lxc-devel] [lxd/master] file push: fix off by one error

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 761 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20161206/84a4ff18/attachment.bin>
-------------- next part --------------
From 08b4fae51de09491e030ad9e0f36071d6b48a80c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 6 Dec 2016 14:11:43 +0100
Subject: [PATCH] file push: fix off by one error

If filepath.Dir(someDir) returns "." we need to substract minus one to include
the first letter of the directory we want to create. This seems to only affect
recursive mode.

Also this adds two tests: One for when we are in the same working directory as
the directory we want to push (This will cover filepath.Dir(someDir) returning
".".). And one for when we give a directory via "../".

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 client.go                |  7 ++++++-
 lxc/file.go              |  6 +++++-
 test/suites/filemanip.sh | 23 +++++++++++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/client.go b/client.go
index 339f6cf..67ce9c2 100644
--- a/client.go
+++ b/client.go
@@ -1903,7 +1903,12 @@ func (c *Client) RecursivePushFile(container string, source string, target strin
 			return fmt.Errorf("got error sending path %s: %s", p, err)
 		}
 
-		targetPath := path.Join(target, p[len(sourceDir):])
+		appendLen := len(sourceDir)
+		if sourceDir == "." {
+			appendLen--
+		}
+
+		targetPath := path.Join(target, p[appendLen:])
 		if fInfo.IsDir() {
 			return c.Mkdir(container, targetPath, fInfo.Mode())
 		}
diff --git a/lxc/file.go b/lxc/file.go
index 7413e1b..56765ac 100644
--- a/lxc/file.go
+++ b/lxc/file.go
@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"syscall"
@@ -79,7 +80,10 @@ func (c *fileCmd) push(config *lxd.Config, send_file_perms bool, args []string)
 	// re-add leading / that got stripped by the SplitN
 	targetPath := "/" + pathSpec[1]
 	// clean various /./, /../, /////, etc. that users add (#2557)
-	targetPath = path.Clean(targetPath)
+	targetPath, err := filepath.Abs(targetPath)
+	if err != nil {
+		return fmt.Errorf(i18n.G("Could not sanitize path %s"), targetPath)
+	}
 	// normalization may reveal that path is still a dir, e.g. /.
 	if strings.HasSuffix(targetPath, "/") {
 		targetIsDir = true
diff --git a/test/suites/filemanip.sh b/test/suites/filemanip.sh
index 39fc8cd..97ad73e 100644
--- a/test/suites/filemanip.sh
+++ b/test/suites/filemanip.sh
@@ -19,6 +19,7 @@ test_filemanip() {
 
   # lxc {push|pull} -r
   mkdir "${TEST_DIR}"/source
+  mkdir "${TEST_DIR}"/source/another_level
   echo "foo" > "${TEST_DIR}"/source/foo
   echo "bar" > "${TEST_DIR}"/source/bar
 
@@ -28,6 +29,28 @@ test_filemanip() {
   [ "$(lxc exec filemanip -- stat -c "%g" /tmp/ptest/source)" = "$(id -g)" ]
   [ "$(lxc exec filemanip -- stat -c "%a" /tmp/ptest/source)" = "755" ]
 
+  # Special case where we are in the same directory as the one we are currently
+  # created.
+  oldcwd=$(pwd)
+  cd "${TEST_DIR}"
+
+  lxc file push -r source filemanip/tmp/ptest
+  [ "$(lxc exec filemanip -- stat -c "%u" /tmp/ptest/source)" = "$(id -u)" ]
+  [ "$(lxc exec filemanip -- stat -c "%g" /tmp/ptest/source)" = "$(id -g)" ]
+  [ "$(lxc exec filemanip -- stat -c "%a" /tmp/ptest/source)" = "755" ]
+
+  # Special case where we are in the same directory as the one we are currently
+  # created.
+  cd source
+
+  lxc file push -r ../source filemanip/tmp/ptest
+  [ "$(lxc exec filemanip -- stat -c "%u" /tmp/ptest/source)" = "$(id -u)" ]
+  [ "$(lxc exec filemanip -- stat -c "%g" /tmp/ptest/source)" = "$(id -g)" ]
+  [ "$(lxc exec filemanip -- stat -c "%a" /tmp/ptest/source)" = "755" ]
+
+  # Switch back to old working directory.
+  cd "${oldcwd}"
+
   mkdir "${TEST_DIR}"/dest
   lxc file pull -r filemanip/tmp/ptest/source "${TEST_DIR}"/dest