From 7a08cd75473bd2cb7e592a0fa84db5e8aa0c818c Mon Sep 17 00:00:00 2001
From: David Disseldorp <ddiss@samba.org>
Date: Thu, 19 Sep 2013 20:31:37 -0700
Subject: [PATCH 1/2] spoolss: return the spoolss job ID in notifications
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Print job notifications currently carry the system print job identifier
from the queue structure. Instead, the spoolss job identifier should be
resolved and returned.

Print clients can use notification job-ids in subsequent spoolss SetJob
requests. Returning an incorrect identifier can result in the failure of
such requests, e.g. spoolss_SetJob(SPOOLSS_JOB_CONTROL_DELETE).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=10271

Signed-off-by: David Disseldorp <ddiss@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Günther Deschner <gd@samba.org>
(cherry picked from commit 24d025f85d6eea272bff5e1040d4fd2ba0e6b8f3)
---
 source3/include/printing.h                  |  1 +
 source3/printing/printing.c                 |  2 +-
 source3/rpc_server/spoolss/srv_spoolss_nt.c | 32 +++++++++++++++++++++++------
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/source3/include/printing.h b/source3/include/printing.h
index 391fb7a..ec5a53b 100644
--- a/source3/include/printing.h
+++ b/source3/include/printing.h
@@ -193,6 +193,7 @@ uint16_t print_spool_rap_jobid(struct print_file_data *print_file);
 
 /* The following definitions come from printing/printing.c  */
 
+uint32 sysjob_to_jobid_pdb(struct tdb_print_db *pdb, int sysjob);
 uint32 sysjob_to_jobid(int unix_jobid);
 bool print_notify_register_pid(int snum);
 bool print_notify_deregister_pid(int snum);
diff --git a/source3/printing/printing.c b/source3/printing/printing.c
index 23b143b..86f8d25 100644
--- a/source3/printing/printing.c
+++ b/source3/printing/printing.c
@@ -514,7 +514,7 @@ static int unixjob_traverse_fn(TDB_CONTEXT *the_tdb, TDB_DATA key,
 	return 0;
 }
 
-static uint32 sysjob_to_jobid_pdb(struct tdb_print_db *pdb, int sysjob)
+uint32 sysjob_to_jobid_pdb(struct tdb_print_db *pdb, int sysjob)
 {
 	struct unixjob_traverse_state state;
 
diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index 9ecf191..6af738c 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -3601,6 +3601,7 @@ static WERROR printer_notify_info(struct pipes_struct *p,
 	print_status_struct status;
 	struct spoolss_PrinterInfo2 *pinfo2 = NULL;
 	WERROR result;
+	struct tdb_print_db *pdb;
 
 	DEBUG(4,("printer_notify_info\n"));
 
@@ -3624,13 +3625,19 @@ static WERROR printer_notify_info(struct pipes_struct *p,
 		return WERR_BADFID;
 	}
 
+	pdb = get_print_db_byname(Printer->sharename);
+	if (pdb == NULL) {
+		return WERR_BADFID;
+	}
+
 	/* Maybe we should use the SYSTEM session_info here... */
 	result = winreg_get_printer_internal(mem_ctx,
 				    get_session_info_system(),
 				    p->msg_ctx,
 				    lp_servicename(talloc_tos(), snum), &pinfo2);
 	if (!W_ERROR_IS_OK(result)) {
-		return WERR_BADFID;
+		result = WERR_BADFID;
+		goto err_pdb_drop;
 	}
 
 	/*
@@ -3639,10 +3646,11 @@ static WERROR printer_notify_info(struct pipes_struct *p,
 	 */
 	pinfo2->servername = talloc_strdup(pinfo2, Printer->servername);
 	if (pinfo2->servername == NULL) {
-		return WERR_NOMEM;
+		result = WERR_NOMEM;
+		goto err_pdb_drop;
 	}
 
-	for (i=0; i<option->count; i++) {
+	for (i = 0; i < option->count; i++) {
 		option_type = option->types[i];
 
 		switch (option_type.type) {
@@ -3661,12 +3669,21 @@ static WERROR printer_notify_info(struct pipes_struct *p,
 			count = print_queue_status(p->msg_ctx, snum, &queue,
 						   &status);
 
-			for (j=0; j<count; j++) {
+			for (j = 0; j < count; j++) {
+				uint32_t jobid;
+				jobid = sysjob_to_jobid_pdb(pdb,
+							    queue[j].sysjob);
+				if (jobid == (uint32_t)-1) {
+					DEBUG(2, ("ignoring untracked job %d\n",
+						  queue[j].sysjob));
+					continue;
+				}
+				/* FIXME check return value */
 				construct_notify_jobs_info(p->msg_ctx,
 							   &queue[j], info,
 							   pinfo2, snum,
 							   &option_type,
-							   queue[j].sysjob,
+							   jobid,
 							   mem_ctx);
 			}
 
@@ -3691,7 +3708,10 @@ static WERROR printer_notify_info(struct pipes_struct *p,
 	*/
 
 	talloc_free(pinfo2);
-	return WERR_OK;
+	result = WERR_OK;
+err_pdb_drop:
+	release_print_db(pdb);
+	return result;
 }
 
 /****************************************************************
-- 
1.8.1.4


From b2d979ab575a8f6b3850d0edf9fbe669c66e4bca Mon Sep 17 00:00:00 2001
From: David Disseldorp <ddiss@samba.org>
Date: Fri, 18 Oct 2013 13:09:23 +0200
Subject: [PATCH 2/2] printing: always store sytem job-ID in queue state
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Print jobs have multiple identifiers: the regular spoolss jobid, which
is allocated by spoolss on job submission, and the system jobid, which
is assigned by the printing back-end.
Currently these identifiers are incorrectly mixed in print job queue
tracking. Fix this by ensuring that only the system jobid is stored in
the print queue state structure.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=10271

Signed-off-by: David Disseldorp <ddiss@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Günther Deschner <gd@samba.org>

Autobuild-User(master): David Disseldorp <ddiss@samba.org>
Autobuild-Date(master): Mon Nov 18 18:03:41 CET 2013 on sn-devel-104
(cherry picked from commit b7da5a5b00f6c78e41279415e33c091dcc0a773b)
---
 source3/printing/printing.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/source3/printing/printing.c b/source3/printing/printing.c
index 86f8d25..5a35ad0 100644
--- a/source3/printing/printing.c
+++ b/source3/printing/printing.c
@@ -3043,7 +3043,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 	size_t len = 0;
 	uint32 i;
 	int max_reported_jobs = lp_max_reported_jobs(snum);
-	bool ret = False;
+	bool ret = false;
 	const char* sharename = lp_servicename(talloc_tos(), snum);
 	TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx);
 	if (tmp_ctx == NULL) {
@@ -3087,7 +3087,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 
 	/* Retrieve the linearised queue data. */
 
-	for( i  = 0; i < qcount; i++) {
+	for(i = 0; i < qcount; i++) {
 		uint32 qjob, qsize, qpage_count, qstatus, qpriority, qtime;
 		len += tdb_unpack(data.dptr + len, data.dsize - len, "ddddddff",
 				&qjob,
@@ -3109,7 +3109,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 	total_count = qcount;
 
 	/* Add new jobids to the queue. */
-	for( i  = 0; i < extra_count; i++) {
+	for (i = 0; i < extra_count; i++) {
 		uint32 jobid;
 		struct printjob *pjob;
 
@@ -3122,7 +3122,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 			continue;
 		}
 
-		queue[total_count].sysjob = jobid;
+		queue[total_count].sysjob = pjob->sysjob;
 		queue[total_count].size = pjob->size;
 		queue[total_count].page_count = pjob->page_count;
 		queue[total_count].status = pjob->status;
@@ -3137,32 +3137,31 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 	/* Update the changed jobids. */
 	for (i = 0; i < changed_count; i++) {
 		uint32_t jobid = IVAL(jcdata.dptr, i * 4);
+		struct printjob *pjob;
 		uint32_t j;
 		bool found = false;
 
+		pjob = print_job_find(tmp_ctx, sharename, jobid);
+		if (pjob == NULL) {
+			DEBUG(5,("get_stored_queue_info: failed to find "
+				 "changed job = %u\n",
+				 (unsigned int)jobid));
+			remove_from_jobs_changed(sharename, jobid);
+			continue;
+		}
+
 		for (j = 0; j < total_count; j++) {
-			if (queue[j].sysjob == jobid) {
+			if (queue[j].sysjob == pjob->sysjob) {
 				found = true;
 				break;
 			}
 		}
 
 		if (found) {
-			struct printjob *pjob;
-
 			DEBUG(5,("get_stored_queue_info: changed job: %u\n",
-				 (unsigned int) jobid));
-
-			pjob = print_job_find(tmp_ctx, sharename, jobid);
-			if (pjob == NULL) {
-				DEBUG(5,("get_stored_queue_info: failed to find "
-					 "changed job = %u\n",
-					 (unsigned int) jobid));
-				remove_from_jobs_changed(sharename, jobid);
-				continue;
-			}
+				 (unsigned int)jobid));
 
-			queue[j].sysjob = jobid;
+			queue[j].sysjob = pjob->sysjob;
 			queue[j].size = pjob->size;
 			queue[j].page_count = pjob->page_count;
 			queue[j].status = pjob->status;
@@ -3172,8 +3171,10 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 			fstrcpy(queue[j].fs_file, pjob->jobname);
 			talloc_free(pjob);
 
-			DEBUG(5,("get_stored_queue_info: updated queue[%u], jobid: %u, jobname: %s\n",
-				 (unsigned int) j, (unsigned int) jobid, pjob->jobname));
+			DEBUG(5,("updated queue[%u], jobid: %u, sysjob: %u, "
+				 "jobname: %s\n",
+				 (unsigned int)j, (unsigned int)jobid,
+				 (unsigned int)queue[j].sysjob, pjob->jobname));
 		}
 
 		remove_from_jobs_changed(sharename, jobid);
@@ -3192,7 +3193,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 	*ppqueue = queue;
 	*pcount = total_count;
 
-	ret = True;
+	ret = true;
 
   out:
 
-- 
1.8.1.4