[PATCH] Fix two bugs in smbget

Christian Ambach ambi at samba.org
Thu Oct 25 12:14:05 UTC 2018


Am 25.10.18 um 13:51 schrieb Andreas Schneider:

> The tree has 1 new uncommitted files!!!
> git clean -n
> Would remove testfile
I am sorry for that.
Not sure how it ended up there and why git clean would have wanted to
remove it. I found one on my machine under st/tmp/, but that's excluded
via .gitignore.
I have expanded the test to remove all testfiles once all tests
have been performed, hopefully that'll work better this time.

Cheers,
Christian



-------------- next part --------------
From 9e5817be303a2c6742c40a0374b8c8d6a474bf15 Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Mon, 22 Oct 2018 16:22:00 +0200
Subject: [PATCH 1/3] s3:script/tests reduce code duplication

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 source3/script/tests/test_smbget.sh | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/source3/script/tests/test_smbget.sh b/source3/script/tests/test_smbget.sh
index f21a131..05925f3 100755
--- a/source3/script/tests/test_smbget.sh
+++ b/source3/script/tests/test_smbget.sh
@@ -37,15 +37,18 @@ create_test_data()
 
 remove_test_data()
 {
-	rm -rf dir1 dir2 testfile
 	pushd $WORKDIR
 	rm -rf dir1 dir2 testfile
 	popd
 }
 
+clear_download_area() {
+	rm -rf dir1 dir2 testfile
+}
+
 test_singlefile_guest()
 {
-	[ -e testfile ] && rm testfile
+	clear_download_area
 	echo "$SMBGET -v -a smb://$SERVER_IP/smbget/testfile"
 	$SMBGET -v -a smb://$SERVER_IP/smbget/testfile
 	if [ $? -ne 0 ]; then
@@ -62,7 +65,7 @@ test_singlefile_guest()
 
 test_singlefile_U()
 {
-	[ -e testfile ] && rm testfile
+	clear_download_area
 	$SMBGET -v -U$USERNAME%$PASSWORD smb://$SERVER_IP/smbget/testfile
 	if [ $? -ne 0 ]; then
 		echo 'ERROR: RC does not match, expected: 0'
@@ -78,7 +81,7 @@ test_singlefile_U()
 
 test_singlefile_smburl()
 {
-	[ -e testfile ] && rm testfile
+	clear_download_area
 	$SMBGET -w $DOMAIN smb://$USERNAME:$PASSWORD@$SERVER_IP/smbget/testfile
 	if [ $? -ne 0 ]; then
 		echo 'ERROR: RC does not match, expected: 0'
@@ -94,7 +97,7 @@ test_singlefile_smburl()
 
 test_singlefile_rcfile()
 {
-	[ -e testfile ] && rm testfile
+	clear_download_area
 	echo "user $USERNAME%$PASSWORD" > $TMPDIR/rcfile
 	$SMBGET -vn -f $TMPDIR/rcfile smb://$SERVER_IP/smbget/testfile
 	rc=$?
@@ -113,9 +116,7 @@ test_singlefile_rcfile()
 
 test_recursive_U()
 {
-	[ -e testfile ] && rm testfile
-	[ -d dir1 ] && rm -rf dir1
-	[ -d dir2 ] && rm -rf dir2
+	clear_download_area
 	$SMBGET -v -R -U$USERNAME%$PASSWORD smb://$SERVER_IP/smbget/
 	if [ $? -ne 0 ]; then
 		echo 'ERROR: RC does not match, expected: 0'
@@ -135,7 +136,7 @@ test_recursive_U()
 
 test_resume()
 {
-	[ -e testfile ] && rm testfile
+	clear_download_area
 	cp $WORKDIR/testfile .
 	truncate -s 1024 testfile
 	$SMBGET -v -r -U$USERNAME%$PASSWORD smb://$SERVER_IP/smbget/testfile
@@ -155,6 +156,7 @@ test_resume()
 
 test_resume_modified()
 {
+	clear_download_area
 	dd if=/dev/urandom bs=1024 count=2 of=testfile
 	$SMBGET -v -r -U$USERNAME%$PASSWORD smb://$SERVER_IP/smbget/testfile
 	if [ $? -ne 1 ]; then
@@ -167,7 +169,7 @@ test_resume_modified()
 
 test_update()
 {
-	[ -e testfile ] && rm testfile
+	clear_download_area
 	$SMBGET -v -U$USERNAME%$PASSWORD smb://$SERVER_IP/smbget/testfile
 	if [ $? -ne 0 ]; then
 		echo 'ERROR: RC does not match, expected: 0'
@@ -229,7 +231,9 @@ testit "resume download (modified file)" test_resume_modified \
 testit "update" test_update \
 	|| failed=`expr $failed + 1`
 
-popd
+clear_download_area
+
+popd # TMPDIR
 
 remove_test_data
 
-- 
1.9.1


From b85fbea4a95c2477bcf3bdaa8ca72dcde4a70bbb Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Mon, 22 Oct 2018 16:28:21 +0200
Subject: [PATCH 2/3] s3:utils/smbget add error handling for mkdir() calls

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 source3/script/tests/test_smbget.sh | 24 ++++++++++++++++++++++++
 source3/utils/smbget.c              | 10 +++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/source3/script/tests/test_smbget.sh b/source3/script/tests/test_smbget.sh
index 05925f3..3f0aac5 100755
--- a/source3/script/tests/test_smbget.sh
+++ b/source3/script/tests/test_smbget.sh
@@ -134,6 +134,27 @@ test_recursive_U()
 	return 0
 }
 
+test_recursive_existing_dir()
+{
+	clear_download_area
+	mkdir dir1
+	$SMBGET -v -R -U$USERNAME%$PASSWORD smb://$SERVER_IP/smbget/
+	if [ $? -ne 0 ]; then
+		echo 'ERROR: RC does not match, expected: 0'
+		return 1
+	fi
+
+	cmp --silent $WORKDIR/testfile ./testfile && \
+	cmp --silent $WORKDIR/dir1/testfile1 ./dir1/testfile1 && \
+	cmp --silent $WORKDIR/dir2/testfile2 ./dir2/testfile2
+	if [ $? -ne 0 ]; then
+		echo 'ERROR: file content does not match'
+		return 1
+	fi
+
+	return 0
+}
+
 test_resume()
 {
 	clear_download_area
@@ -222,6 +243,9 @@ testit "download single file with rcfile" test_singlefile_rcfile \
 testit "recursive download" test_recursive_U \
 	|| failed=`expr $failed + 1`
 
+testit "recursive download (existing target dir)" test_recursive_existing_dir \
+	|| failed=`expr $failed + 1`
+
 testit "resume download" test_resume \
 	|| failed=`expr $failed + 1`
 
diff --git a/source3/utils/smbget.c b/source3/utils/smbget.c
index 4653c68..ca4bf31 100644
--- a/source3/utils/smbget.c
+++ b/source3/utils/smbget.c
@@ -190,7 +190,15 @@ static bool smb_download_dir(const char *base, const char *name, int resume)
 	while (*relname == '/') {
 		relname++;
 	}
-	mkdir(relname, 0755);
+
+	if (strlen(relname) > 0) {
+		int rc = mkdir(relname, 0755);
+		if (rc == -1 && errno != EEXIST) {
+			fprintf(stderr, "Can't create directory %s: %s\n",
+				relname, strerror(errno));
+			return false;
+		}
+	}
 
 	tmpname = SMB_STRDUP(name);
 
-- 
1.9.1


From 7daf361c006bb449bd6188e8147aa8638c871402 Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Tue, 23 Oct 2018 20:05:04 +0200
Subject: [PATCH 3/3] s3:utils/smbget fix recursive download with empty source
 directories

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13199
Signed-off-by: Christian Ambach <ambi at samba.org>
---
 source3/script/tests/test_smbget.sh | 38 ++++++++++++++++++++++++++++++++++++-
 source3/utils/smbget.c              |  1 +
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/source3/script/tests/test_smbget.sh b/source3/script/tests/test_smbget.sh
index 3f0aac5..b0ff75f 100755
--- a/source3/script/tests/test_smbget.sh
+++ b/source3/script/tests/test_smbget.sh
@@ -43,7 +43,7 @@ remove_test_data()
 }
 
 clear_download_area() {
-	rm -rf dir1 dir2 testfile
+	rm -rf dir1 dir2 testfile dir001 dir004
 }
 
 test_singlefile_guest()
@@ -155,6 +155,39 @@ test_recursive_existing_dir()
 	return 0
 }
 
+
+test_recursive_with_empty() # see Bug 13199
+{
+	clear_download_area
+	# create some additional empty directories
+	mkdir -p $WORKDIR/dir001/dir002/dir003
+	mkdir -p $WORKDIR/dir004/dir005/dir006
+	$SMBGET -v -R -U$USERNAME%$PASSWORD smb://$SERVER_IP/smbget/
+	rc=$?
+	rm -rf $WORKDIR/dir001
+	rm -rf $WORKDIR/dir004
+	if [ $rc -ne 0 ]; then
+		echo 'ERROR: RC does not match, expected: 0'
+		return 1
+	fi
+
+	cmp --silent $WORKDIR/testfile ./testfile && \
+	cmp --silent $WORKDIR/dir1/testfile1 ./dir1/testfile1 && \
+	cmp --silent $WORKDIR/dir2/testfile2 ./dir2/testfile2
+	if [ $? -ne 0 ]; then
+		echo 'ERROR: file content does not match'
+		return 1
+	fi
+
+	if [ ! -d dir001/dir002/dir003 ] || [ ! -d dir004/dir005/dir006 ]; then
+		echo 'ERROR: empty directories are not present'
+		return 1
+	fi
+
+	return 0
+}
+
+
 test_resume()
 {
 	clear_download_area
@@ -246,6 +279,9 @@ testit "recursive download" test_recursive_U \
 testit "recursive download (existing target dir)" test_recursive_existing_dir \
 	|| failed=`expr $failed + 1`
 
+testit "recursive download with empty directories" test_recursive_with_empty \
+	|| failed=`expr $failed + 1`
+
 testit "resume download" test_resume \
 	|| failed=`expr $failed + 1`
 
diff --git a/source3/utils/smbget.c b/source3/utils/smbget.c
index ca4bf31..49cca4e 100644
--- a/source3/utils/smbget.c
+++ b/source3/utils/smbget.c
@@ -205,6 +205,7 @@ static bool smb_download_dir(const char *base, const char *name, int resume)
 	while ((dirent = smbc_readdir(dirhandle))) {
 		char *newname;
 		if (!strcmp(dirent->name, ".") || !strcmp(dirent->name, "..")) {
+			ok = true;
 			continue;
 		}
 		if (asprintf(&newname, "%s/%s", tmpname, dirent->name) == -1) {
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181025/3d93685a/signature.sig>


More information about the samba-technical mailing list