[LOCALSPL] Fix parameter handling in LocalSetJob and add tests for the few ways we...
authorColin Finck <colin@reactos.org>
Mon, 25 Dec 2017 13:30:47 +0000 (14:30 +0100)
committerColin Finck <colin@reactos.org>
Mon, 25 Dec 2017 13:30:47 +0000 (14:30 +0100)
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!

modules/rostests/apitests/localspl/dll/CMakeLists.txt
modules/rostests/apitests/localspl/dll/fpSetJob.c [new file with mode: 0644]
modules/rostests/apitests/localspl/dll/main.c
modules/rostests/apitests/localspl/testlist.c
modules/rostests/apitests/localspl/tests.c
win32ss/printing/providers/localspl/jobs.c

index 2ce4287..4c518ee 100644 (file)
@@ -4,11 +4,12 @@ include_directories(${REACTOS_SOURCE_DIR}/win32ss/printing/include)
 list(APPEND SOURCE
     fpEnumPrinters.c
     fpGetPrintProcessorDirectory.c
 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)
     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)
 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
@@ -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);
+}
index 6914dc0..260baa3 100644 (file)
@@ -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
  * 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__
  */
 
 #define __ROS_LONG64__
 // Test list
 extern void func_fpEnumPrinters(void);
 extern void func_fpGetPrintProcessorDirectory(void);
 // 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 },
 
 const struct test winetest_testlist[] =
 {
     { "fpEnumPrinters", func_fpEnumPrinters },
     { "fpGetPrintProcessorDirectory", func_fpGetPrintProcessorDirectory },
+    { "fpSetJob", func_fpSetJob },
     { 0, 0 }
 };
 
     { 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)
 {
 BOOL
 GetLocalsplFuncs(LPPRINTPROVIDOR pp)
 {
index efd9f73..b1a5d9e 100644 (file)
@@ -2,7 +2,7 @@
  * PROJECT:     ReactOS Local Spooler API Tests
  * LICENSE:     GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+)
  * PURPOSE:     Test list
  * 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__
  */
 
 #define __ROS_LONG64__
 
 extern void func_fpEnumPrinters(void);
 extern void func_fpGetPrintProcessorDirectory(void);
 
 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 },
 extern void func_service(void);
 
 const struct test winetest_testlist[] =
 {
     { "fpEnumPrinters", func_fpEnumPrinters },
     { "fpGetPrintProcessorDirectory", func_fpGetPrintProcessorDirectory },
+    { "fpSetJob", func_fpSetJob },
     { "service", func_service },
 
     { 0, 0 }
     { "service", func_service },
 
     { 0, 0 }
index 85d8023..e5c1cc0 100644 (file)
@@ -217,3 +217,8 @@ START_TEST(fpGetPrintProcessorDirectory)
 {
     _RunRemoteTest("fpGetPrintProcessorDirectory");
 }
 {
     _RunRemoteTest("fpGetPrintProcessorDirectory");
 }
+
+START_TEST(fpSetJob)
+{
+    _RunRemoteTest("fpSetJob");
+}
index 0c9e040..b52b25e 100644 (file)
@@ -963,7 +963,7 @@ Cleanup:
 BOOL WINAPI
 LocalSetJob(HANDLE hPrinter, DWORD JobId, DWORD Level, PBYTE pJobInfo, DWORD Command)
 {
 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;
     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;
 
     // 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;
     {
         dwErrorCode = ERROR_INVALID_HANDLE;
         goto Cleanup;
@@ -989,16 +989,11 @@ LocalSetJob(HANDLE hPrinter, DWORD JobId, DWORD Level, PBYTE pJobInfo, DWORD Com
         goto Cleanup;
     }
 
         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;
 
     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);
 Cleanup:
     SetLastError(dwErrorCode);
     return (dwErrorCode == ERROR_SUCCESS);