[ros-diffs] [cfinck] 73373: [LOCALSPL] Tests show that the Print Provider functions are less forgiving and expect winspool.drv and spoolss.dll to perform the necessary sanity checks. Also they don't default to...

Author: cfinck
Date: Thu Nov 24 19:24:27 2016
New Revision: 73373

URL: http://svn.reactos.org/svn/reactos?rev=73373&view=rev
Log:
[LOCALSPL]
Tests show that the Print Provider functions are less forgiving and expect winspool.drv
and spoolss.dll to perform the necessary sanity checks.
Also they don't default to the current environment if none was supplied as
pEnvironment. Change our functions accordingly to pass the tests.

[LOCALSPL_APITEST]
Add a test to prove that spoolss.dll isn't just a forwarder, but also performs some
sanity checks even though winspool.drv and localspl.dll also do.
This rather belongs to spoolss_apitest. But it needs to be tested inside the Spooler
Server Process and it's not worth it to copy all the DLL injection code to
spoolss_apitest just for a single test.

[SPOOLSS]
Check for an invalid buffer in GetPrintProcessorDirectoryW.

[WINSPOOL]
* Check the level before the RPC call in GetPrintProcessorDirectoryW.
* Supply our current environment if the caller didn't give any in EnumPrintProcessorsW
and GetPrintProcessorDirectoryW.
* Implement GetPrintProcessorDirectoryA.

[WINSPOOL_APITEST]
Prove that pcbNeeded isn't touched when calling GetPrintProcessorDirectoryW with an
invalid level.


ReactOS now passes all API tests for GetPrintProcessorDirectoryA and
GetPrintProcessorDirectoryW. These tests also cover all known behaviors of both
functions.
CORE-12399

Added:
    trunk/reactos/win32ss/printing/include/prtprocenv.h   (with props)
Modified:
    trunk/reactos/win32ss/printing/base/spoolss/printprocessors.c
    trunk/reactos/win32ss/printing/base/winspool/printprocessors.c
    trunk/reactos/win32ss/printing/base/winspool/winspool.spec
    trunk/reactos/win32ss/printing/providers/localspl/main.c
    trunk/reactos/win32ss/printing/providers/localspl/printprocessors.c
    trunk/rostests/apitests/localspl/dll/fpGetPrintProcessorDirectory.c
    trunk/rostests/apitests/localspl/dll/main.c
    trunk/rostests/apitests/winspool/GetPrintProcessorDirectory.c

Modified: trunk/reactos/win32ss/printing/base/spoolss/printprocessors.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/printing/base/spoo…
==============================================================================
--- trunk/reactos/win32ss/printing/base/spoolss/printprocessors.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/printing/base/spoolss/printprocessors.c	[iso-8859-1] Thu Nov 24
19:24:27 2016
@@ -26,6 +26,13 @@
 BOOL WINAPI
 GetPrintProcessorDirectoryW(PWSTR pName, PWSTR pEnvironment, DWORD Level, PBYTE
pPrintProcessorInfo, DWORD cbBuf, PDWORD pcbNeeded)
 {
+    // Sanity checks
+    if (cbBuf && !pPrintProcessorInfo)
+    {
+        SetLastError(ERROR_INVALID_USER_BUFFER);
+        return FALSE;
+    }
+
     // Always call this function on the Local Spooler.
     PSPOOLSS_PRINT_PROVIDER pPrintProvider = CONTAINING_RECORD(PrintProviderList.Flink,
SPOOLSS_PRINT_PROVIDER, Entry);
     return pPrintProvider->PrintProvider.fpGetPrintProcessorDirectory(pName,
pEnvironment, Level, pPrintProcessorInfo, cbBuf, pcbNeeded);

Modified: trunk/reactos/win32ss/printing/base/winspool/printprocessors.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/printing/base/wins…
==============================================================================
--- trunk/reactos/win32ss/printing/base/winspool/printprocessors.c	[iso-8859-1]
(original)
+++ trunk/reactos/win32ss/printing/base/winspool/printprocessors.c	[iso-8859-1] Thu Nov 24
19:24:27 2016
@@ -2,10 +2,11 @@
  * PROJECT:     ReactOS Spooler API
  * LICENSE:     GNU LGPL v2.1 or any later version as published by the Free Software
Foundation
  * PURPOSE:     Functions related to Print Processors
- * COPYRIGHT:   Copyright 2015 Colin Finck <colin(a)reactos.org>
+ * COPYRIGHT:   Copyright 2015-2016 Colin Finck <colin(a)reactos.org>
  */
 
 #include "precomp.h"
+#include <prtprocenv.h>
 
 static void
 _MarshallUpDatatypesInfo(PDATATYPES_INFO_1W pDatatypesInfo1)
@@ -82,6 +83,10 @@
     DWORD i;
     PBYTE p = pPrintProcessorInfo;
 
+    // Choose our current environment if the caller didn't give any.
+    if (!pEnvironment)
+        pEnvironment = (PWSTR)wszCurrentEnvironment;
+
     // Do the RPC call
     RpcTryExcept
     {
@@ -90,7 +95,6 @@
     RpcExcept(EXCEPTION_EXECUTE_HANDLER)
     {
         dwErrorCode = RpcExceptionCode();
-        ERR("_RpcEnumPrintProcessors failed with exception code %lu!\n",
dwErrorCode);
     }
     RpcEndExcept;
 
@@ -109,23 +113,106 @@
 }
 
 BOOL WINAPI
+GetPrintProcessorDirectoryA(PSTR pName, PSTR pEnvironment, DWORD Level, PBYTE
pPrintProcessorInfo, DWORD cbBuf, PDWORD pcbNeeded)
+{
+    BOOL bReturnValue = FALSE;
+    DWORD cch;
+    PWSTR pwszName = NULL;
+    PWSTR pwszEnvironment = NULL;
+    PWSTR pwszPrintProcessorInfo = NULL;
+
+    if (pName)
+    {
+        // Convert pName to a Unicode string pwszName.
+        cch = strlen(pName);
+
+        pwszName = HeapAlloc(hProcessHeap, 0, (cch + 1) * sizeof(WCHAR));
+        if (!pwszName)
+        {
+            ERR("HeapAlloc failed for pwszName with last error %lu!\n",
GetLastError());
+            goto Cleanup;
+        }
+
+        MultiByteToWideChar(CP_ACP, 0, pName, -1, pwszName, cch + 1);
+    }
+
+    if (pEnvironment)
+    {
+        // Convert pEnvironment to a Unicode string pwszEnvironment.
+        cch = strlen(pEnvironment);
+
+        pwszEnvironment = HeapAlloc(hProcessHeap, 0, (cch + 1) * sizeof(WCHAR));
+        if (!pwszEnvironment)
+        {
+            ERR("HeapAlloc failed for pwszEnvironment with last error %lu!\n",
GetLastError());
+            goto Cleanup;
+        }
+
+        MultiByteToWideChar(CP_ACP, 0, pEnvironment, -1, pwszEnvironment, cch + 1);
+    }
+
+    if (cbBuf && pPrintProcessorInfo)
+    {
+        // Allocate a temporary buffer for the Unicode result.
+        // We can just go with cbBuf here. The user should have set it based on pcbNeeded
returned in a previous call and our
+        // pcbNeeded is the same for the A and W functions.
+        pwszPrintProcessorInfo = HeapAlloc(hProcessHeap, 0, cbBuf);
+        if (!pwszPrintProcessorInfo)
+        {
+            ERR("HeapAlloc failed for pwszPrintProcessorInfo with last error
%lu!\n", GetLastError());
+            goto Cleanup;
+        }
+    }
+
+    bReturnValue = GetPrintProcessorDirectoryW(pwszName, pwszEnvironment, Level,
(PBYTE)pwszPrintProcessorInfo, cbBuf, pcbNeeded);
+
+    if (bReturnValue)
+    {
+        // Convert pwszPrintProcessorInfo to an ANSI string pPrintProcessorInfo.
+        WideCharToMultiByte(CP_ACP, 0, pwszPrintProcessorInfo, -1,
(PSTR)pPrintProcessorInfo, cbBuf, NULL, NULL);
+    }
+
+Cleanup:
+    if (pwszName)
+        HeapFree(hProcessHeap, 0, pwszName);
+
+    if (pwszEnvironment)
+        HeapFree(hProcessHeap, 0, pwszEnvironment);
+
+    if (pwszPrintProcessorInfo)
+        HeapFree(hProcessHeap, 0, pwszPrintProcessorInfo);
+
+    return bReturnValue;
+}
+
+BOOL WINAPI
 GetPrintProcessorDirectoryW(PWSTR pName, PWSTR pEnvironment, DWORD Level, PBYTE
pPrintProcessorInfo, DWORD cbBuf, PDWORD pcbNeeded)
 {
-    BOOL bReturnValue = FALSE;
     DWORD dwErrorCode;
+
+    // Sanity checks
+    if (Level != 1)
+    {
+        dwErrorCode = ERROR_INVALID_LEVEL;
+        goto Cleanup;
+    }
+
+    // Choose our current environment if the caller didn't give any.
+    if (!pEnvironment)
+        pEnvironment = (PWSTR)wszCurrentEnvironment;
 
     // Do the RPC call
     RpcTryExcept
     {
         dwErrorCode = _RpcGetPrintProcessorDirectory(pName, pEnvironment, Level,
pPrintProcessorInfo, cbBuf, pcbNeeded);
-        SetLastError(dwErrorCode);
-        bReturnValue = (dwErrorCode == ERROR_SUCCESS);
     }
     RpcExcept(EXCEPTION_EXECUTE_HANDLER)
     {
-        ERR("_RpcGetPrintProcessorDirectory failed with exception code %lu!\n",
RpcExceptionCode());
+        dwErrorCode = RpcExceptionCode();
     }
     RpcEndExcept;
 
-    return bReturnValue;
-}
+Cleanup:
+    SetLastError(dwErrorCode);
+    return (dwErrorCode == ERROR_SUCCESS);
+}

Modified: trunk/reactos/win32ss/printing/base/winspool/winspool.spec
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/printing/base/wins…
==============================================================================
--- trunk/reactos/win32ss/printing/base/winspool/winspool.spec	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/printing/base/winspool/winspool.spec	[iso-8859-1] Thu Nov 24
19:24:27 2016
@@ -144,7 +144,7 @@
 243 stub GetFormW
 244 stdcall GetJobA(long long long ptr long ptr)
 245 stdcall GetJobW(long long long ptr long ptr)
-246 stdcall -stub GetPrintProcessorDirectoryA(str str long ptr long ptr)
+246 stdcall GetPrintProcessorDirectoryA(str str long ptr long ptr)
 247 stdcall GetPrintProcessorDirectoryW(wstr wstr long ptr long ptr)
 248 stdcall GetPrinterA(long long ptr long ptr)
 249 stub GetPrinterDataA

Added: trunk/reactos/win32ss/printing/include/prtprocenv.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/printing/include/p…
==============================================================================
--- trunk/reactos/win32ss/printing/include/prtprocenv.h	(added)
+++ trunk/reactos/win32ss/printing/include/prtprocenv.h	[iso-8859-1] Thu Nov 24 19:24:27
2016
@@ -0,0 +1,24 @@
+/*
+ * PROJECT:     ReactOS Printing Include files
+ * LICENSE:     GNU LGPL v2.1 or any later version as published by the Free Software
Foundation
+ * PURPOSE:     Provide a constant for the current Printing Processor Environment based
on the architecture
+ * COPYRIGHT:   Copyright 2016 Colin Finck &lt;colin(a)reactos.org&gt;
+ */
+
+#ifndef _REACTOS_PRTPROCENV_H
+#define _REACTOS_PRTPROCENV_H
+
+const WCHAR wszCurrentEnvironment[] =
+#if defined(_X86_)
+    L"Windows NT x86";
+#elif defined(_AMD64_)
+    L"Windows x64";
+#elif defined(_ARM_)
+    L"Windows ARM";
+#else
+    #error Unsupported architecture
+#endif
+
+const DWORD cbCurrentEnvironment = sizeof(wszCurrentEnvironment);
+
+#endif

Propchange: trunk/reactos/win32ss/printing/include/prtprocenv.h
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: trunk/reactos/win32ss/printing/providers/localspl/main.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/printing/providers…
==============================================================================
--- trunk/reactos/win32ss/printing/providers/localspl/main.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/printing/providers/localspl/main.c	[iso-8859-1] Thu Nov 24
19:24:27 2016
@@ -2,7 +2,7 @@
  * PROJECT:     ReactOS Local Spooler
  * LICENSE:     GNU LGPL v2.1 or any later version as published by the Free Software
Foundation
  * PURPOSE:     Main functions
- * COPYRIGHT:   Copyright 2015 Colin Finck &lt;colin(a)reactos.org&gt;
+ * COPYRIGHT:   Copyright 2015-2016 Colin Finck &lt;colin(a)reactos.org&gt;
  */
 
 #include "precomp.h"
@@ -12,18 +12,7 @@
 DWORD cchSpoolDirectory;
 
 // Global Constants
-const WCHAR wszCurrentEnvironment[] =
-#if defined(_X86_)
-    L"Windows NT x86";
-#elif defined(_AMD64_)
-    L"Windows x64";
-#elif defined(_ARM_)
-    L"Windows ARM";
-#else
-    #error Unsupported architecture
-#endif
-
-const DWORD cbCurrentEnvironment = sizeof(wszCurrentEnvironment);
+#include <prtprocenv.h>
 
 const WCHAR wszDefaultDocumentName[] = L"Local Downlevel Document";
 

Modified: trunk/reactos/win32ss/printing/providers/localspl/printprocessors.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/printing/providers…
==============================================================================
--- trunk/reactos/win32ss/printing/providers/localspl/printprocessors.c	[iso-8859-1]
(original)
+++ trunk/reactos/win32ss/printing/providers/localspl/printprocessors.c	[iso-8859-1] Thu
Nov 24 19:24:27 2016
@@ -2,7 +2,7 @@
  * PROJECT:     ReactOS Local Spooler
  * LICENSE:     GNU LGPL v2.1 or any later version as published by the Free Software
Foundation
  * PURPOSE:     Functions related to Print Processors
- * COPYRIGHT:   Copyright 2015 Colin Finck &lt;colin(a)reactos.org&gt;
+ * COPYRIGHT:   Copyright 2015-2016 Colin Finck &lt;colin(a)reactos.org&gt;
  */
 
 #include "precomp.h"
@@ -17,7 +17,7 @@
  * Checks a supplied pEnvironment variable for validity and opens its registry key.
  *
  * @param pEnvironment
- * The pEnvironment variable to check. Can be NULL to use the current environment.
+ * The pEnvironment variable to check.
  *
  * @param hKey
  * On success, this variable will contain a HKEY to the opened registry key of the
environment.
@@ -36,9 +36,12 @@
     DWORD dwErrorCode;
     PWSTR pwszEnvironmentKey = NULL;
 
-    // Use the current environment if none was supplied.
+    // Sanity checks
     if (!pEnvironment)
-        pEnvironment = wszCurrentEnvironment;
+    {
+        dwErrorCode = ERROR_INVALID_ENVIRONMENT;
+        goto Cleanup;
+    }
 
     // Construct the registry key of the demanded environment.
     cchEnvironment = wcslen(pEnvironment);
@@ -140,7 +143,7 @@
     InitializeListHead(&_PrintProcessorList);
     
     // Prepare the path to the Print Processor directory.
-    if (!LocalGetPrintProcessorDirectory(NULL, NULL, 1, (PBYTE)wszPrintProcessorPath,
sizeof(wszPrintProcessorPath), &cchPrintProcessorPath))
+    if (!LocalGetPrintProcessorDirectory(NULL, (PWSTR)wszCurrentEnvironment, 1,
(PBYTE)wszPrintProcessorPath, sizeof(wszPrintProcessorPath), &cchPrintProcessorPath))
     {
         dwErrorCode = GetLastError();
         goto Cleanup;
@@ -155,7 +158,7 @@
     ++cchPrintProcessorPath;
 
     // Open the environment registry key.
-    dwErrorCode = _OpenEnvironment(NULL, &hKey);
+    dwErrorCode = _OpenEnvironment(wszCurrentEnvironment, &hKey);
     if (dwErrorCode != ERROR_SUCCESS)
     {
         ERR("_OpenEnvironment failed with error %lu!\n", dwErrorCode);
@@ -602,7 +605,6 @@
  *
  * @param pEnvironment
  * One of the predefined operating system and architecture "environment"
strings (like "Windows NT x86").
- * Alternatively, NULL to output the Print Processor directory of the current
environment.
  *
  * @param Level
  * The level of the (non-existing) structure supplied through pPrintProcessorInfo. This
must be 1.
@@ -633,20 +635,6 @@
     HKEY hKey = NULL;
     PWSTR pwszDirectory = (PWSTR)pPrintProcessorInfo;
 
-    // Sanity checks
-    if (Level != 1)
-    {
-        dwErrorCode = ERROR_INVALID_LEVEL;
-        goto Cleanup;
-    }
-
-    if (!pcbNeeded)
-    {
-        // This error is also caught by RPC and returned as RPC_X_NULL_REF_POINTER.
-        dwErrorCode = ERROR_INVALID_PARAMETER;
-        goto Cleanup;
-    }
-
     // Verify pEnvironment and open its registry key.
     dwErrorCode = _OpenEnvironment(pEnvironment, &hKey);
     if (dwErrorCode != ERROR_SUCCESS)

Modified: trunk/rostests/apitests/localspl/dll/fpGetPrintProcessorDirectory.c
URL:
http://svn.reactos.org/svn/reactos/trunk/rostests/apitests/localspl/dll/fpG…
==============================================================================
--- trunk/rostests/apitests/localspl/dll/fpGetPrintProcessorDirectory.c	[iso-8859-1]
(original)
+++ trunk/rostests/apitests/localspl/dll/fpGetPrintProcessorDirectory.c	[iso-8859-1] Thu
Nov 24 19:24:27 2016
@@ -18,17 +18,24 @@
 #include "../localspl_apitest.h"
 #include <spoolss.h>
 
+typedef BOOL (WINAPI *PGetPrintProcessorDirectoryW)(LPWSTR, LPWSTR, DWORD, LPBYTE, DWORD,
LPDWORD);
 extern BOOL GetLocalsplFuncs(LPPRINTPROVIDOR pp);
+extern PVOID GetSpoolssFunc(const char* FunctionName);
 
 START_TEST(fpGetPrintProcessorDirectory)
 {
     DWORD cbNeeded;
     DWORD cbTemp;
     DWORD dwReturned;
+    PGetPrintProcessorDirectoryW pGetPrintProcessorDirectoryW;
     PRINTPROVIDOR pp;
     PWSTR pwszBuffer;
 
     if (!GetLocalsplFuncs(&pp))
+        return;
+
+    pGetPrintProcessorDirectoryW =
GetSpoolssFunc("GetPrintProcessorDirectoryW");
+    if (!pGetPrintProcessorDirectoryW)
         return;
 
     // In contrast to GetPrintProcessorDirectoryW, fpGetPrintProcessorDirectory needs an
environment and doesn't just accept NULL.
@@ -80,6 +87,14 @@
 
     ok(dwReturned == EXCEPTION_ACCESS_VIOLATION, "dwReturned is %lu!\n",
dwReturned);
 
+    // Prove that this check is implemented in spoolss' GetPrintProcessorDirectoryW
instead.
+    // In contrast to winspool's GetPrintProcessorDirectoryW, cbTemp is left
untouched though. This comes from the fact that RPC isn't involved here.
+    SetLastError(0xDEADBEEF);
+    cbTemp = 0xDEADBEEF;
+    ok(!pGetPrintProcessorDirectoryW(NULL, L"wIndows nt x86", 1, NULL,
cbNeeded, &cbTemp), "pGetPrintProcessorDirectoryW returns TRUE!\n");
+    ok(GetLastError() == ERROR_INVALID_USER_BUFFER, "pGetPrintProcessorDirectoryW
returns error %lu!\n", GetLastError());
+    ok(cbTemp == 0xDEADBEEF, "cbTemp is %lu!\n", cbTemp);
+
     // Finally use the function as intended and aim for success!
     // We only check success by the boolean return value though. GetLastError doesn't
return anything meaningful here.
     pwszBuffer = DllAllocSplMem(cbNeeded);

Modified: trunk/rostests/apitests/localspl/dll/main.c
URL:
http://svn.reactos.org/svn/reactos/trunk/rostests/apitests/localspl/dll/mai…
==============================================================================
--- trunk/rostests/apitests/localspl/dll/main.c	[iso-8859-1] (original)
+++ trunk/rostests/apitests/localspl/dll/main.c	[iso-8859-1] Thu Nov 24 19:24:27 2016
@@ -67,6 +67,22 @@
     return TRUE;
 }
 
+PVOID
+GetSpoolssFunc(const char* FunctionName)
+{
+    HMODULE hSpoolss;
+
+    // Get us a handle to the loaded spoolss.dll.
+    hSpoolss = GetModuleHandleW(L"spoolss");
+    if (!hSpoolss)
+    {
+        skip("GetModuleHandleW failed with error %u!\n", GetLastError());
+        return FALSE;
+    }
+
+    return GetProcAddress(hSpoolss, FunctionName);
+}
+
 // Running the tests from the injected DLL and redirecting their output to the pipe.
 BOOL WINAPI
 DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)

Modified: trunk/rostests/apitests/winspool/GetPrintProcessorDirectory.c
URL:
http://svn.reactos.org/svn/reactos/trunk/rostests/apitests/winspool/GetPrin…
==============================================================================
--- trunk/rostests/apitests/winspool/GetPrintProcessorDirectory.c	[iso-8859-1] (original)
+++ trunk/rostests/apitests/winspool/GetPrintProcessorDirectory.c	[iso-8859-1] Thu Nov 24
19:24:27 2016
@@ -21,8 +21,10 @@
 
     // Try with an invalid level, this needs to be caught first.
     SetLastError(0xDEADBEEF);
-    ok(!GetPrintProcessorDirectoryA(NULL, NULL, 0, NULL, 0, NULL),
"GetPrintProcessorDirectoryA returns TRUE!\n");
+    cbNeeded = 0xDEADBEEF;
+    ok(!GetPrintProcessorDirectoryA(NULL, NULL, 0, NULL, 0, &cbNeeded),
"GetPrintProcessorDirectoryA returns TRUE!\n");
     ok(GetLastError() == ERROR_INVALID_LEVEL, "GetPrintProcessorDirectoryA returns
error %lu!\n", GetLastError());
+    ok(cbNeeded == 0xDEADBEEF, "cbNeeded is %lu!\n", cbNeeded);
 
     // Now try with valid level, but no pcbNeeded.
     SetLastError(0xDEADBEEF);
@@ -77,8 +79,10 @@
 
     // Try with an invalid level, this needs to be caught first.
     SetLastError(0xDEADBEEF);
-    ok(!GetPrintProcessorDirectoryW(NULL, NULL, 0, NULL, 0, NULL),
"GetPrintProcessorDirectoryW returns TRUE!\n");
+    cbNeeded = 0xDEADBEEF;
+    ok(!GetPrintProcessorDirectoryW(NULL, NULL, 0, NULL, 0, &cbNeeded),
"GetPrintProcessorDirectoryW returns TRUE!\n");
     ok(GetLastError() == ERROR_INVALID_LEVEL, "GetPrintProcessorDirectoryW returns
error %lu!\n", GetLastError());
+    ok(cbNeeded == 0xDEADBEEF, "cbNeeded is %lu!\n", cbNeeded);
 
     // Now try with valid level, but no pcbNeeded.
     SetLastError(0xDEADBEEF);