[PATCH] Fix printing regression.
Jeremy Allison
jra at samba.org
Fri Jun 1 16:39:40 UTC 2018
On Fri, Jun 01, 2018 at 10:32:05AM +0200, Andreas Schneider wrote:
> On Thursday, 31 May 2018 21:03:23 CEST Jeremy Allison wrote:
> > Hi all,
> >
> > Here is a fix for a regression in the printing code
> > that's been there since we moved from 3.0.x -> 3.5.0 :-).
> >
> > https://bugzilla.samba.org/show_bug.cgi?id=13457
> >
> > Setting delete-on-close on an open printing file
> > should cancel the job.
> >
> > The fix was easy, writing the regression test
> > took the time :-).
> >
> > Please review and push if happy !
>
> This doesn't pass with --picky-developer :-)
Passed with picky-developer locally. I must have
an older compiler.
> ignoring return value of 'ftruncate', declared with attribute
> warn_unused_result [-Werror=unused-result]
>
>
> I've merged the test and added a return value check for ftruncate to print an
> error if this happens.
Thanks for catching this !
> Please check the attached patch.
LGTM. Pushed and thanks !
> Thanks,
>
>
> Andreas
>
>
> --
> Andreas Schneider GPG-ID: CC014E3D
> Samba Team asn at samba.org
> www.samba.org
> From 69c1ce81b243831ed00f369976a4c6320e261f1f Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Thu, 31 May 2018 10:18:21 -0700
> Subject: [PATCH 1/2] s3: torture: Add DELETE-PRINT test.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13457
>
> Signed-off-by: Jeremy Allison <jra at samba.org>
> Reviewed-by: Andreas Schneider <asn at samba.org>
> ---
> selftest/knownfail.d/smbspool | 1 +
> source3/script/tests/test_smbspool.sh | 63 +++++++++++++++++++++++
> source3/torture/torture.c | 73 +++++++++++++++++++++++++++
> 3 files changed, 137 insertions(+)
> create mode 100644 selftest/knownfail.d/smbspool
>
> diff --git a/selftest/knownfail.d/smbspool b/selftest/knownfail.d/smbspool
> new file mode 100644
> index 00000000000..a61fc4dca5d
> --- /dev/null
> +++ b/selftest/knownfail.d/smbspool
> @@ -0,0 +1 @@
> +^samba3.blackbox.smbspool.delete.on.close
> diff --git a/source3/script/tests/test_smbspool.sh b/source3/script/tests/test_smbspool.sh
> index 899b34d0e40..d95ed064634 100755
> --- a/source3/script/tests/test_smbspool.sh
> +++ b/source3/script/tests/test_smbspool.sh
> @@ -22,6 +22,7 @@ incdir=`dirname $0`/../../../testprogs/blackbox
> samba_bindir="$BINDIR"
> samba_vlp="$samba_bindir/vlp"
> samba_smbspool="$samba_bindir/smbspool"
> +samba_smbtorture3="$samba_bindir/smbtorture3"
> samba_smbspool_krb5="$samba_bindir/smbspool_krb5_wrapper"
>
> test_smbspool_noargs()
> @@ -119,6 +120,64 @@ test_vlp_verify()
> fi
> }
>
> +test_delete_on_close()
> +{
> + tdbfile="$PREFIX_ABS/$TARGET_ENV/lockdir/vlp.tdb"
> + if [ ! -w $tdbfile ]; then
> + echo "vlp tdbfile $tdbfile doesn't exist or is not writeable!"
> + return 1
> + fi
> +
> + cmd='$samba_vlp tdbfile=$tdbfile lpq print1 2>&1'
> + eval echo "$cmd"
> + out=$(eval $cmd)
> + ret=$?
> + if [ $ret != 0 ]; then
> + echo "failed to lpq jobs on print1 with $samba_vlp"
> + echo "$out"
> + return 1
> + fi
> +
> + num_jobs=$(echo "$out" | wc -l)
> + #
> + # Now run the test DELETE-PRINT from smbtorture3
> + #
> + cmd='$samba_smbtorture3 //$SERVER_IP/print1 -U$USERNAME%$PASSWORD DELETE-PRINT 2>&1'
> + eval echo "$cmd"
> + out_t=$(eval $cmd)
> + ret=$?
> + if [ $ret != 0 ]; then
> + echo "failed to run DELETE-PRINT on print1"
> + echo "$out_t"
> + return 1
> + fi
> +
> + cmd='$samba_vlp tdbfile=$tdbfile lpq print1 2>&1'
> + eval echo "$cmd"
> + out1=$(eval $cmd)
> + ret=$?
> + if [ $ret != 0 ]; then
> + echo "(2) failed to lpq jobs on print1 with $samba_vlp"
> + echo "$out1"
> + return 1
> + fi
> + num_jobs1=$(echo "$out1" | wc -l)
> +
> + #
> + # Number of jobs should not change. Job
> + # should not have made it to backend.
> + #
> + if [ "$num_jobs1" -ne "$num_jobs" ]; then
> + echo "delete-on-close fail $num_jobs1 -ne $num_jobs"
> + echo "$out"
> + echo "$out_t"
> + echo "$out1"
> + return 1
> + fi
> +
> + return 0
> +}
> +
> testit "smbspool no args" \
> test_smbspool_noargs $samba_smbspool || \
> failed=$(expr $failed + 1)
> @@ -180,4 +239,8 @@ testit "vlp verify example.ps" \
> failed=$(expr $failed + 1)
> unset AUTH_INFO_REQUIRED
>
> +testit "delete on close" \
> + test_delete_on_close \
> + || failed=$(expr $failed + 1)
> +
> exit $failed
> diff --git a/source3/torture/torture.c b/source3/torture/torture.c
> index 23671a706cd..412e9b413fa 100644
> --- a/source3/torture/torture.c
> +++ b/source3/torture/torture.c
> @@ -4561,6 +4561,78 @@ static bool run_deletetest(int dummy)
> return correct;
> }
>
> +/*
> + Exercise delete on close semantics - use on the PRINT1 share in torture
> + testing.
> + */
> +static bool run_delete_print_test(int dummy)
> +{
> + struct cli_state *cli1 = NULL;
> + const char *fname = "print_delete.file";
> + uint16_t fnum1 = (uint16_t)-1;
> + bool correct = false;
> + const char *buf = "print file data\n";
> + NTSTATUS status;
> +
> + printf("starting print delete test\n");
> +
> + if (!torture_open_connection(&cli1, 0)) {
> + return false;
> + }
> +
> + smbXcli_conn_set_sockopt(cli1->conn, sockops);
> +
> + status = cli_ntcreate(cli1, fname, 0, GENERIC_ALL_ACCESS|DELETE_ACCESS,
> + FILE_ATTRIBUTE_NORMAL, 0, FILE_OVERWRITE_IF,
> + 0, 0, &fnum1, NULL);
> + if (!NT_STATUS_IS_OK(status)) {
> + printf("open of %s failed (%s)\n",
> + fname,
> + nt_errstr(status));
> + goto fail;
> + }
> +
> + status = cli_writeall(cli1,
> + fnum1,
> + 0,
> + (const uint8_t *)buf,
> + 0, /* offset */
> + strlen(buf), /* size */
> + NULL);
> + if (!NT_STATUS_IS_OK(status)) {
> + printf("writing print file data failed (%s)\n",
> + nt_errstr(status));
> + goto fail;
> + }
> +
> + status = cli_nt_delete_on_close(cli1, fnum1, true);
> + if (!NT_STATUS_IS_OK(status)) {
> + printf("setting delete_on_close failed (%s)\n",
> + nt_errstr(status));
> + goto fail;
> + }
> +
> + status = cli_close(cli1, fnum1);
> + if (!NT_STATUS_IS_OK(status)) {
> + printf("close failed (%s)\n", nt_errstr(status));
> + goto fail;
> + }
> +
> + printf("finished print delete test\n");
> +
> + correct = true;
> +
> + fail:
> +
> + if (fnum1 != (uint16_t)-1) {
> + cli_close(cli1, fnum1);
> + }
> +
> + if (cli1 && !torture_close_connection(cli1)) {
> + correct = false;
> + }
> + return correct;
> +}
>
> /*
> Test wildcard delete.
> @@ -11551,6 +11623,7 @@ static struct {
> {"RENAME-ACCESS", run_rename_access, 0},
> {"OWNER-RIGHTS", run_owner_rights, 0},
> {"DELETE", run_deletetest, 0},
> + {"DELETE-PRINT", run_delete_print_test, 0},
> {"WILDDELETE", run_wild_deletetest, 0},
> {"DELETE-LN", run_deletetest_ln, 0},
> {"PROPERTIES", run_properties, 0},
> --
> 2.17.0
>
>
> From 9aeeb24b36c0c596361cec1eda262e9e17578735 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Thu, 31 May 2018 10:35:48 -0700
> Subject: [PATCH 2/2] s3: smbd: printing: Re-implement delete-on-close
> semantics for print files missing since 3.5.x.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13457
>
> Signed-off-by: Jeremy Allison <jra at samba.org>
> Reviewed-by: Andreas Schneider <asn at samba.org>
> ---
> selftest/knownfail.d/smbspool | 1 -
> source3/printing/printspoolss.c | 17 +++++++++++++++++
> 2 files changed, 17 insertions(+), 1 deletion(-)
> delete mode 100644 selftest/knownfail.d/smbspool
>
> diff --git a/selftest/knownfail.d/smbspool b/selftest/knownfail.d/smbspool
> deleted file mode 100644
> index a61fc4dca5d..00000000000
> --- a/selftest/knownfail.d/smbspool
> +++ /dev/null
> @@ -1 +0,0 @@
> -^samba3.blackbox.smbspool.delete.on.close
> diff --git a/source3/printing/printspoolss.c b/source3/printing/printspoolss.c
> index 60002020351..9d565de120a 100644
> --- a/source3/printing/printspoolss.c
> +++ b/source3/printing/printspoolss.c
> @@ -309,6 +309,23 @@ void print_spool_end(files_struct *fsp, enum file_close_type close_type)
> WERROR werr;
> struct dcerpc_binding_handle *b = NULL;
>
> + if (fsp->fh->private_options &
> + NTCREATEX_OPTIONS_PRIVATE_DELETE_ON_CLOSE) {
> + int ret;
> +
> + /*
> + * Job was requested to be cancelled by setting
> + * delete on close so truncate the job file.
> + * print_job_end() which is called from
> + * _spoolss_EndDocPrinter() will take
> + * care of deleting it for us.
> + */
> + ret = ftruncate(fsp->fh->fd, 0);
> + if (ret == -1) {
> + DBG_ERR("ftruncate failed: %s\n", strerror(errno));
> + }
> + }
> +
> b = fsp->conn->spoolss_pipe->binding_handle;
>
> switch (close_type) {
> --
> 2.17.0
>
More information about the samba-technical
mailing list