getquota/setquota quoting problem

Jeremy Allison jra at samba.org
Wed Feb 1 01:18:14 UTC 2017


On Fri, Jan 27, 2017 at 11:17:34AM -0500, Emily Maier wrote:
> Hi,
> 
> We found that the "get quota command" and "set quota command" hooks
> don't work properly for filenames with whitespace in them. They get
> passed as multiple arguments to the quota script due to quoting
> issues. I've attached a patch that correctly calls the script and
> adds a test for it.

> Emily Maier

Thanks a *LOT* for this Emily ! I'll try and take a look
at this (travelling to FOSDEM this week so it might not
get done immediately though).

I just wanted to thank you for your contribution, and
assure you I'll try not to drop if on the floor :-).

Cheers,

	Jeremy.

> From 0e90438bfb58283eec52bf6ea8751fda44e17e60 Mon Sep 17 00:00:00 2001
> From: Emily Maier <emily.maier at editshare.com>
> Date: Thu, 15 Sep 2016 11:50:35 -0400
> Subject: [PATCH] s3: Fix get quota and set quota arguments
> 
> Change invocation of the 'get quota command' and 'set quota command'
> scripts so that paths with spaces in them can be passed properly.
> 
> Signed-off-by: Emily Maier <emily.maier at editshare.com>
> ---
>  selftest/target/Samba3.pm                  | 11 +++++++
>  source3/lib/sysquotas.c                    | 30 +++++++++++++----
>  source3/script/tests/test_quota_command.sh | 52 ++++++++++++++++++++++++++++++
>  source3/selftest/tests.py                  |  1 +
>  testprogs/blackbox/get_quota.sh            |  8 +++++
>  5 files changed, 96 insertions(+), 6 deletions(-)
>  create mode 100755 source3/script/tests/test_quota_command.sh
>  create mode 100755 testprogs/blackbox/get_quota.sh
> 
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index 32f0c6f5a01..55b381962c3 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -641,6 +641,11 @@ sub setup_fileserver($$)
>  	push(@dirs, "$dfree_share_dir/subdir2");
>  	push(@dirs, "$dfree_share_dir/subdir3");
>  
> +	my $quota_share_dir="$share_dir/quota";
> +	push(@dirs, "$quota_share_dir");
> +	push(@dirs, "$quota_share_dir/subdir1");
> +	push(@dirs, "$quota_share_dir/subdir 2");
> +
>  	my $valid_users_sharedir="$share_dir/valid_users";
>  	push(@dirs,$valid_users_sharedir);
>  
> @@ -672,6 +677,9 @@ sub setup_fileserver($$)
>  	path = $dfree_share_dir
>  	comment = smb username is [%U]
>  	dfree command = $srcdir_abs/testprogs/blackbox/dfree.sh
> +[quota]
> +	path = $quota_share_dir
> +	comment = smb username is [%U]
>  [valid-users-access]
>  	path = $valid_users_sharedir
>  	valid users = +userdup
> @@ -1169,6 +1177,7 @@ sub provision($$$$$$$$)
>  	my @unix_gids = split(" ", $unix_gids_str);
>  
>  	my $prefix_abs = abs_path($prefix);
> +	my $srcdir_abs = abs_path($self->{srcdir});
>  	my $bindir_abs = abs_path($self->{bindir});
>  
>  	my @dirs = ();
> @@ -1518,6 +1527,8 @@ sub provision($$$$$$$$)
>  	#it just means we ALLOW one to be configured.
>  	allow insecure wide links = yes
>  
> +	get quota command = $srcdir_abs/testprogs/blackbox/get_quota.sh
> +
>  	# Begin extra options
>  	$extra_options
>  	# End extra options
> diff --git a/source3/lib/sysquotas.c b/source3/lib/sysquotas.c
> index eef87beafe0..fe4ad6ae087 100644
> --- a/source3/lib/sysquotas.c
> +++ b/source3/lib/sysquotas.c
> @@ -250,6 +250,9 @@ static int command_get_quota(const char *path, enum SMB_QUOTA_TYPE qtype, unid_t
>  		char *p2;
>  		char *syscmd = NULL;
>  		int _id = -1;
> +		int ret;
> +		int outfd;
> +		int numlines;
>  
>  		switch(qtype) {
>  			case SMB_USER_QUOTA_TYPE:
> @@ -265,17 +268,23 @@ static int command_get_quota(const char *path, enum SMB_QUOTA_TYPE qtype, unid_t
>  				return -1;
>  		}
>  
> -		if (asprintf(&syscmd, "%s %s %d %d",
> +		if (asprintf(&syscmd, "%s \"%s\" %d %d",
>  			get_quota_command, path, qtype, _id) < 0) {
>  			return -1;
>  		}
>  
>  		DEBUG (3, ("get_quota: Running command %s\n", syscmd));
>  
> -		lines = file_lines_pload(talloc_tos(), syscmd, NULL);
> +		ret = smbrun(syscmd, &outfd, NULL);
> +		if (ret || outfd == -1) {
> +			DEBUG (0, ("get_quota_command failed!\n"));
> +			return -1;
> +		}
> +		lines = fd_lines_load(outfd, &numlines, 0, talloc_tos());
> +		close(outfd);
>  		SAFE_FREE(syscmd);
>  
> -		if (lines) {
> +		if (lines && numlines) {
>  			char *line = lines[0];
>  
>  			DEBUG (3, ("Read output from get_quota, \"%s\"\n", line));
> @@ -393,6 +402,9 @@ static int command_set_quota(const char *path, enum SMB_QUOTA_TYPE qtype, unid_t
>  		char **lines = NULL;
>  		char *syscmd = NULL;
>  		int _id = -1;
> +		int ret;
> +		int outfd;
> +		int numlines;
>  
>  		switch(qtype) {
>  			case SMB_USER_QUOTA_TYPE:
> @@ -408,7 +420,7 @@ static int command_set_quota(const char *path, enum SMB_QUOTA_TYPE qtype, unid_t
>  		}
>  
>  		if (asprintf(&syscmd,
> -			"%s %s %d %d "
> +			"%s \"%s\" %d %d "
>  			"%u %llu %llu "
>  			"%llu %llu %llu ",
>  			set_quota_command, path, qtype, _id, dp->qflags,
> @@ -420,9 +432,15 @@ static int command_set_quota(const char *path, enum SMB_QUOTA_TYPE qtype, unid_t
>  
>  		DEBUG (3, ("get_quota: Running command %s\n", syscmd));
>  
> -		lines = file_lines_pload(talloc_tos(), syscmd, NULL);
> +		ret = smbrun(syscmd, &outfd, NULL);
> +		if (ret || outfd == -1) {
> +			DEBUG (0, ("set_quota_command failed!\n"));
> +			return -1;
> +		}
> +		lines = fd_lines_load(outfd, &numlines, 0, talloc_tos());
> +		close(outfd);
>  		SAFE_FREE(syscmd);
> -		if (lines) {
> +		if (lines && numlines) {
>  			char *line = lines[0];
>  
>  			DEBUG (3, ("Read output from set_quota, \"%s\"\n", line));
> diff --git a/source3/script/tests/test_quota_command.sh b/source3/script/tests/test_quota_command.sh
> new file mode 100755
> index 00000000000..dadb20438b7
> --- /dev/null
> +++ b/source3/script/tests/test_quota_command.sh
> @@ -0,0 +1,52 @@
> +#!/bin/sh
> +#
> +# Blackbox test for 'get quota command'.
> +#
> +
> +if [ $# -lt 6 ]; then
> +cat <<EOF
> +Usage: test_quota_command.sh SERVER DOMAIN USERNAME PASSWORD PREFIX SMBCLIENT
> +EOF
> +exit 1;
> +fi
> +
> +SERVER=$1
> +DOMAIN=$2
> +USERNAME=$3
> +PASSWORD=$4
> +PREFIX=$5
> +smbclient=$6
> +shift 6
> +failed=0
> +
> +incdir=`dirname $0`/../../../testprogs/blackbox
> +. $incdir/subunit.sh
> +
> +test_smbclient_dfree() {
> +	name="$1"
> +	share="$2"
> +	cmd="$3"
> +    expected="$4"
> +	shift
> +	shift
> +    shift
> +	subunit_start_test "$name"
> +	output=$($VALGRIND $smbclient //$SERVER/$share -c "$cmd" $@ 2>&1)
> +	status=$?
> +	if [ x$status = x0 ]; then
> +		received=$(echo "$output" | awk '/blocks of size/ {print $1, $5, $6}')
> +		if [ "$expected" = "$received" ]; then
> +			subunit_pass_test "$name"
> +		else
> +			echo "$output" | subunit_fail_test "$name"
> +		fi
> +	else
> +		echo "$output" | subunit_fail_test "$name"
> +	fi
> +	return $status
> +}
> +
> +test_smbclient_dfree "Test dfree command subdir1 SMB3" quota "cd subdir1; l" "1000 1024. 0" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 || failed=`expr $failed + 1`
> +test_smbclient_dfree "Test dfree command subdir 2 SMB3" quota "cd \"subdir 2\"; l" "1000 1024. 800" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 || failed=`expr $failed + 1`
> +
> +exit $failed
> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
> index 4231e1db42c..9a01a785a94 100755
> --- a/source3/selftest/tests.py
> +++ b/source3/selftest/tests.py
> @@ -195,6 +195,7 @@ for env in ["fileserver"]:
>      plantestsuite("samba3.blackbox.preserve_case (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_preserve_case.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3])
>      plantestsuite("samba3.blackbox.dfree_command (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_dfree_command.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3])
>      plantestsuite("samba3.blackbox.dfree_quota (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_dfree_quota.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', smbclient3, smbcquotas, smbcacls])
> +    plantestsuite("samba3.blackbox.quota_command (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_quota_command.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3])
>      plantestsuite("samba3.blackbox.valid_users (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_valid_users.sh"), '$SERVER', '$SERVER_IP', '$DOMAIN', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3])
>      plantestsuite("samba3.blackbox.offline (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_offline.sh"), '$SERVER', '$SERVER_IP', '$DOMAIN', '$USERNAME', '$PASSWORD', '$LOCAL_PATH/offline', smbclient3])
>      plantestsuite("samba3.blackbox.shadow_copy2 NT1 (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_shadow_copy.sh"), '$SERVER', '$SERVER_IP', '$DOMAIN', '$USERNAME', '$PASSWORD', '$LOCAL_PATH/shadow', smbclient3, '-m', 'NT1'])
> diff --git a/testprogs/blackbox/get_quota.sh b/testprogs/blackbox/get_quota.sh
> new file mode 100755
> index 00000000000..2906383610e
> --- /dev/null
> +++ b/testprogs/blackbox/get_quota.sh
> @@ -0,0 +1,8 @@
> +#!/bin/sh
> +if [ "$1" = "subdir1" ] ; then
> +	echo "2 1000 1000 1000 100 1000 1000 1024"
> +elif [ "$1" = "subdir 2" ] ; then
> +	echo "2 200 1000 1000 100 1000 1000 1024"
> +else
> +	echo "0 0 0 0 0 0 0 1024"
> +fi
> -- 
> 2.11.0
> 




More information about the samba-technical mailing list