[reactos] 01/01: [LOCALSPL] Fix parameter handling in LocalSetJob and add tests for the few ways we can easily test this function.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=3b93ba0f318f053b9aeff3...
commit 3b93ba0f318f053b9aeff36a7c06334d8c0d0183 Author: Colin Finck colin@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@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@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 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@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 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);
-
Colin Finck