[ros-diffs] [reactos] 01/01: [LOCALSPL] Fix parameter handling in LocalSetJob and add tests for the few ways we can easily test this function.

[Colin Finck]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=3b93ba0f318f053b9aeff…

commit 3b93ba0f318f053b9aeff36a7c06334d8c0d0183
Author: Colin Finck <colin(a)reactos.org>
AuthorDate: Mon Dec 25 14:30:47 2017 +0100

    [LOCALSPL] Fix parameter handling in LocalSetJob and add tests for the few ways we can
easily test this function.
    
    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!
---
 .../rostests/apitests/localspl/dll/CMakeLists.txt  |  3 +-
 modules/rostests/apitests/localspl/dll/fpSetJob.c  | 64 +++++++++++++++++++
 modules/rostests/apitests/localspl/dll/main.c      | 73 +++++++++++++++++++++-
 modules/rostests/apitests/localspl/testlist.c      |  4 +-
 modules/rostests/apitests/localspl/tests.c         |  5 ++
 win32ss/printing/providers/localspl/jobs.c         | 21 +++----
 6 files changed, 153 insertions(+), 17 deletions(-)

diff --git a/modules/rostests/apitests/localspl/dll/CMakeLists.txt
b/modules/rostests/apitests/localspl/dll/CMakeLists.txt
index 2ce4287611..4c518ee1b5 100644
--- 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
index 0000000000..810dc1aee6
--- /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(a)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 6914dc0c1e..260baa3639 100644
--- 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(a)reactos.org)
+ * COPYRIGHT:   Copyright 2015-2017 Colin Finck (colin(a)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 efd9f7314b..b1a5d9e35e 100644
--- 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(a)reactos.org)
+ * COPYRIGHT:   Copyright 2015-2017 Colin Finck (colin(a)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 85d8023384..e5c1cc0e38 100644
--- 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 0c9e040fc2..b52b25eb8f 100644
--- 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);