[LOCALSPL] Fix parameter handling in LocalSetJob and add tests for the few ways we...

Yes, it checks the input handle and doesn't fail if an invalid level is given, because someone may still send a Command.
This also fixes CORE-12794. Thanks for reporting!

diff --git a/modules/rostests/apitests/localspl/dll/CMakeLists.txt b/modules/rostests/apitests/localspl/dll/CMakeLists.txt
index 2ce4287..4c518ee 100644 (file)
--- a/modules/rostests/apitests/localspl/dll/CMakeLists.txt
+++ b/modules/rostests/apitests/localspl/dll/CMakeLists.txt
@@ -4,11 +4,12 @@ include_directories(${REACTOS_SOURCE_DIR}/win32ss/printing/include)
 list(APPEND SOURCE
     fpEnumPrinters.c
     fpGetPrintProcessorDirectory.c
+    fpSetJob.c
     main.c)
 
 add_library(localspl_apitest.dll SHARED ${SOURCE})
 target_link_libraries(localspl_apitest.dll wine ${PSEH_LIB})
 set_module_type(localspl_apitest.dll win32dll)
-add_importlibs(localspl_apitest.dll spoolss msvcrt kernel32 ntdll)
+add_importlibs(localspl_apitest.dll spoolss advapi32 msvcrt kernel32 ntdll)
 set_target_properties(localspl_apitest.dll PROPERTIES SUFFIX "")
 add_rostests_file(TARGET localspl_apitest.dll)

diff --git a/modules/rostests/apitests/localspl/dll/fpSetJob.c b/modules/rostests/apitests/localspl/dll/fpSetJob.c
new file mode 100644 (file)
index 0000000..810dc1a
--- /dev/null
+++ b/modules/rostests/apitests/localspl/dll/fpSetJob.c
@@ -0,0 +1,64 @@
+/*
+ * PROJECT:     ReactOS Local Spooler API Tests Injected DLL
+ * LICENSE:     GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+)
+ * PURPOSE:     Tests for fpSetJob
+ * COPYRIGHT:   Copyright 2017 Colin Finck (colin@reactos.org)
+ */
+
+#include <apitest.h>
+
+#define WIN32_NO_STATUS
+#include <windef.h>
+#include <winbase.h>
+#include <wingdi.h>
+#include <winreg.h>
+#include <winspool.h>
+#include <winsplp.h>
+
+#include "../localspl_apitest.h"
+#include <spoolss.h>
+
+extern PWSTR GetDefaultPrinterFromRegistry(VOID);
+extern BOOL GetLocalsplFuncs(LPPRINTPROVIDOR pp);
+
+/* From printing/include/spoolss.h */
+#define MAX_PRINTER_NAME        220
+
+START_TEST(fpSetJob)
+{
+    HANDLE hPrinter = NULL;
+    PRINTPROVIDOR pp;
+    PWSTR pwszDefaultPrinter = NULL;
+
+    if (!GetLocalsplFuncs(&pp))
+        goto Cleanup;
+
+    // Verify that fpSetJob returns ERROR_INVALID_HANDLE when nothing is provided.
+    ok(!pp.fpSetJob(NULL, 0, 0, NULL, 0), "fpSetJob returns TRUE\n");
+    ok(GetLastError() == ERROR_INVALID_HANDLE, "fpSetJob returns error %lu!\n", GetLastError());
+
+    // Get the default printer.
+    pwszDefaultPrinter = GetDefaultPrinterFromRegistry();
+    if (!pwszDefaultPrinter)
+    {
+        skip("Could not determine the default printer!\n");
+        goto Cleanup;
+    }
+
+    if (!pp.fpOpenPrinter(pwszDefaultPrinter, &hPrinter, NULL))
+    {
+        skip("Could not open a handle to the default printer, last error is %lu!\n", GetLastError());
+        goto Cleanup;
+    }
+
+    // Verify that fpSetJob returns ERROR_INVALID_PARAMETER if only a printer handle is provided.
+    ok(!pp.fpSetJob(hPrinter, 0, 0, NULL, 0), "fpSetJob returns TRUE\n");
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "fpSetJob returns error %lu!\n", GetLastError());
+
+Cleanup:
+    if (pwszDefaultPrinter)
+        HeapFree(GetProcessHeap(), 0, pwszDefaultPrinter);
+
+    if (hPrinter)
+        pp.fpClosePrinter(hPrinter);
+}

diff --git a/modules/rostests/apitests/localspl/dll/main.c b/modules/rostests/apitests/localspl/dll/main.c
index 6914dc0..260baa3 100644 (file)
--- a/modules/rostests/apitests/localspl/dll/main.c
+++ b/modules/rostests/apitests/localspl/dll/main.c
@@ -2,7 +2,7 @@
  * PROJECT:     ReactOS Local Spooler API Tests Injected DLL
  * LICENSE:     GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+)
  * PURPOSE:     Main functions
- * COPYRIGHT:   Copyright 2015-2016 Colin Finck (colin@reactos.org)
+ * COPYRIGHT:   Copyright 2015-2017 Colin Finck (colin@reactos.org)
  */
 
 #define __ROS_LONG64__
@@ -27,14 +27,85 @@
 // Test list
 extern void func_fpEnumPrinters(void);
 extern void func_fpGetPrintProcessorDirectory(void);
+extern void func_fpSetJob(void);
 
 const struct test winetest_testlist[] =
 {
     { "fpEnumPrinters", func_fpEnumPrinters },
     { "fpGetPrintProcessorDirectory", func_fpGetPrintProcessorDirectory },
+    { "fpSetJob", func_fpSetJob },
     { 0, 0 }
 };
 
+/**
+ * We don't link against winspool, so we don't have GetDefaultPrinterW.
+ * We can easily implement a similar function ourselves though.
+ */
+PWSTR
+GetDefaultPrinterFromRegistry(VOID)
+{
+    static const WCHAR wszWindowsKey[] = L"Software\\Microsoft\\Windows NT\\CurrentVersion\\Windows";
+    static const WCHAR wszDeviceValue[] = L"Device";
+
+    DWORD cbNeeded;
+    DWORD dwErrorCode;
+    HKEY hWindowsKey = NULL;
+    PWSTR pwszDevice;
+    PWSTR pwszComma;
+    PWSTR pwszReturnValue = NULL;
+
+    // Open the registry key where the default printer for the current user is stored.
+    dwErrorCode = (DWORD)RegOpenKeyExW(HKEY_CURRENT_USER, wszWindowsKey, 0, KEY_READ, &hWindowsKey);
+    if (dwErrorCode != ERROR_SUCCESS)
+    {
+        skip("RegOpenKeyExW failed with status %u!\n", dwErrorCode);
+        goto Cleanup;
+    }
+
+    // Determine the size of the required buffer.
+    dwErrorCode = (DWORD)RegQueryValueExW(hWindowsKey, wszDeviceValue, NULL, NULL, NULL, &cbNeeded);
+    if (dwErrorCode != ERROR_SUCCESS)
+    {
+        skip("RegQueryValueExW failed with status %u!\n", dwErrorCode);
+        goto Cleanup;
+    }
+
+    // Allocate it.
+    pwszDevice = HeapAlloc(GetProcessHeap(), 0, cbNeeded);
+    if (!pwszDevice)
+    {
+        skip("HeapAlloc failed!\n");
+        goto Cleanup;
+    }
+
+    // Now get the actual value.
+    dwErrorCode = RegQueryValueExW(hWindowsKey, wszDeviceValue, NULL, NULL, (PBYTE)pwszDevice, &cbNeeded);
+    if (dwErrorCode != ERROR_SUCCESS)
+    {
+        skip("RegQueryValueExW failed with status %u!\n", dwErrorCode);
+        goto Cleanup;
+    }
+
+    // We get a string "<Printer Name>,winspool,<Port>:".
+    // Extract the printer name from it.
+    pwszComma = wcschr(pwszDevice, L',');
+    if (!pwszComma)
+    {
+        skip("Found no or invalid default printer: %S!\n", pwszDevice);
+        goto Cleanup;
+    }
+
+    // Return the default printer.
+    *pwszComma = 0;
+    pwszReturnValue = pwszDevice;
+
+Cleanup:
+    if (hWindowsKey)
+        RegCloseKey(hWindowsKey);
+
+    return pwszReturnValue;
+}
+
 BOOL
 GetLocalsplFuncs(LPPRINTPROVIDOR pp)
 {

diff --git a/modules/rostests/apitests/localspl/testlist.c b/modules/rostests/apitests/localspl/testlist.c
index efd9f73..b1a5d9e 100644 (file)
--- a/modules/rostests/apitests/localspl/testlist.c
+++ b/modules/rostests/apitests/localspl/testlist.c
@@ -2,7 +2,7 @@
  * PROJECT:     ReactOS Local Spooler API Tests
  * LICENSE:     GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+)
  * PURPOSE:     Test list
- * COPYRIGHT:   Copyright 2015-2016 Colin Finck (colin@reactos.org)
+ * COPYRIGHT:   Copyright 2015-2017 Colin Finck (colin@reactos.org)
  */
 
 #define __ROS_LONG64__
@@ -12,12 +12,14 @@
 
 extern void func_fpEnumPrinters(void);
 extern void func_fpGetPrintProcessorDirectory(void);
+extern void func_fpSetJob(void);
 extern void func_service(void);
 
 const struct test winetest_testlist[] =
 {
     { "fpEnumPrinters", func_fpEnumPrinters },
     { "fpGetPrintProcessorDirectory", func_fpGetPrintProcessorDirectory },
+    { "fpSetJob", func_fpSetJob },
     { "service", func_service },
 
     { 0, 0 }

diff --git a/modules/rostests/apitests/localspl/tests.c b/modules/rostests/apitests/localspl/tests.c
index 85d8023..e5c1cc0 100644 (file)
--- a/modules/rostests/apitests/localspl/tests.c
+++ b/modules/rostests/apitests/localspl/tests.c
@@ -217,3 +217,8 @@ START_TEST(fpGetPrintProcessorDirectory)
 {
     _RunRemoteTest("fpGetPrintProcessorDirectory");
 }
+
+START_TEST(fpSetJob)
+{
+    _RunRemoteTest("fpSetJob");
+}

diff --git a/win32ss/printing/providers/localspl/jobs.c b/win32ss/printing/providers/localspl/jobs.c
index 0c9e040..b52b25e 100644 (file)
--- a/win32ss/printing/providers/localspl/jobs.c
+++ b/win32ss/printing/providers/localspl/jobs.c
@@ -963,7 +963,7 @@ Cleanup:
 BOOL WINAPI
 LocalSetJob(HANDLE hPrinter, DWORD JobId, DWORD Level, PBYTE pJobInfo, DWORD Command)
 {
-    DWORD dwErrorCode;
+    DWORD dwErrorCode = ERROR_SUCCESS;
     PLOCAL_HANDLE pHandle;
     PLOCAL_JOB pJob;
     PLOCAL_PRINTER_HANDLE pPrinterHandle;
@@ -973,7 +973,7 @@ LocalSetJob(HANDLE hPrinter, DWORD JobId, DWORD Level, PBYTE pJobInfo, DWORD Com
 
     // Check if this is a printer handle.
     pHandle = (PLOCAL_HANDLE)hPrinter;
-    if (pHandle->HandleType != HandleType_Printer)
+    if (!pHandle || pHandle->HandleType != HandleType_Printer)
     {
         dwErrorCode = ERROR_INVALID_HANDLE;
         goto Cleanup;
@@ -989,16 +989,11 @@ LocalSetJob(HANDLE hPrinter, DWORD JobId, DWORD Level, PBYTE pJobInfo, DWORD Com
         goto Cleanup;
     }
 
-    // Setting job information is handled differently for each level.
-    if (Level)
-    {
-        if (Level == 1)
-            dwErrorCode = _LocalSetJobLevel1(pPrinterHandle, pJob, (PJOB_INFO_1W)pJobInfo);
-        else if (Level == 2)
-            dwErrorCode = _LocalSetJobLevel2(pPrinterHandle, pJob, (PJOB_INFO_2W)pJobInfo);
-        else
-            dwErrorCode = ERROR_INVALID_LEVEL;
-    }
+    // Set new job information if a valid level was given.
+    if (Level == 1)
+        dwErrorCode = _LocalSetJobLevel1(pPrinterHandle, pJob, (PJOB_INFO_1W)pJobInfo);
+    else if (Level == 2)
+        dwErrorCode = _LocalSetJobLevel2(pPrinterHandle, pJob, (PJOB_INFO_2W)pJobInfo);
 
     if (dwErrorCode != ERROR_SUCCESS)
         goto Cleanup;
@@ -1033,8 +1028,6 @@ LocalSetJob(HANDLE hPrinter, DWORD JobId, DWORD Level, PBYTE pJobInfo, DWORD Com
         }
     }
 
-    dwErrorCode = ERROR_SUCCESS;
-
 Cleanup:
     SetLastError(dwErrorCode);
     return (dwErrorCode == ERROR_SUCCESS);