From 77994cce4e3cc3841fbee62c7f5dff5a245ddb52 Mon Sep 17 00:00:00 2001 From: Robert Shearman Date: Fri, 3 Sep 2004 01:04:38 +0000 Subject: [PATCH] - Fix PropVariantClear to reject invalid types. - Don't crash on NULL pointers. - Add test case. --- dlls/ole32/ole2.c | 92 +++++++++++++++--- dlls/ole32/tests/.cvsignore | 1 + dlls/ole32/tests/Makefile.in | 1 + dlls/ole32/tests/propvariant.c | 166 +++++++++++++++++++++++++++++++++ 4 files changed, 246 insertions(+), 14 deletions(-) create mode 100644 dlls/ole32/tests/propvariant.c diff --git a/dlls/ole32/ole2.c b/dlls/ole32/ole2.c index efeb81e597c..71060428663 100644 --- a/dlls/ole32/ole2.c +++ b/dlls/ole32/ole2.c @@ -2329,23 +2329,90 @@ static void OLE_FreeClipDataArray(ULONG count, CLIPDATA * pClipDataArray) HRESULT WINAPI FreePropVariantArray(ULONG,PROPVARIANT*); +/****************************************************************************** + * Check if a PROPVARIANT's type is valid. + */ +static inline HRESULT PROPVARIANT_ValidateType(VARTYPE vt) +{ + switch (vt) + { + case VT_EMPTY: + case VT_NULL: + case VT_I2: + case VT_I4: + case VT_R4: + case VT_R8: + case VT_CY: + case VT_DATE: + case VT_BSTR: + case VT_ERROR: + case VT_BOOL: + case VT_UI1: + case VT_UI2: + case VT_UI4: + case VT_I8: + case VT_UI8: + case VT_LPSTR: + case VT_LPWSTR: + case VT_FILETIME: + case VT_BLOB: + case VT_STREAM: + case VT_STORAGE: + case VT_STREAMED_OBJECT: + case VT_STORED_OBJECT: + case VT_BLOB_OBJECT: + case VT_CF: + case VT_CLSID: + case VT_I2|VT_VECTOR: + case VT_I4|VT_VECTOR: + case VT_R4|VT_VECTOR: + case VT_R8|VT_VECTOR: + case VT_CY|VT_VECTOR: + case VT_DATE|VT_VECTOR: + case VT_BSTR|VT_VECTOR: + case VT_ERROR|VT_VECTOR: + case VT_BOOL|VT_VECTOR: + case VT_VARIANT|VT_VECTOR: + case VT_UI1|VT_VECTOR: + case VT_UI2|VT_VECTOR: + case VT_UI4|VT_VECTOR: + case VT_I8|VT_VECTOR: + case VT_UI8|VT_VECTOR: + case VT_LPSTR|VT_VECTOR: + case VT_LPWSTR|VT_VECTOR: + case VT_FILETIME|VT_VECTOR: + case VT_CF|VT_VECTOR: + case VT_CLSID|VT_VECTOR: + return S_OK; + } + WARN("Bad type %d\n", vt); + return STG_E_INVALIDPARAMETER; +} + /*********************************************************************** * PropVariantClear [OLE32.@] */ HRESULT WINAPI PropVariantClear(PROPVARIANT * pvar) /* [in/out] */ { + HRESULT hr; + TRACE("(%p)\n", pvar); if (!pvar) return S_OK; + hr = PROPVARIANT_ValidateType(pvar->vt); + if (FAILED(hr)) + return hr; + switch(pvar->vt) { case VT_STREAM: case VT_STREAMED_OBJECT: case VT_STORAGE: case VT_STORED_OBJECT: - IUnknown_Release((LPUNKNOWN)pvar->u.pStream); + if (pvar->u.pStream) + IUnknown_Release(pvar->u.pStream); break; case VT_CLSID: case VT_LPSTR: @@ -2370,12 +2437,7 @@ HRESULT WINAPI PropVariantClear(PROPVARIANT * pvar) /* [in/out] */ } break; default: - if (pvar->vt & VT_ARRAY) - { - FIXME("Need to call SafeArrayDestroy\n"); - /* SafeArrayDestroy(pvar->u.caub); */ - } - switch (pvar->vt & VT_VECTOR) + switch (pvar->vt & ~VT_VECTOR) { case VT_VARIANT: FreePropVariantArray(pvar->u.capropvar.cElems, pvar->u.capropvar.pElems); @@ -2386,9 +2448,10 @@ HRESULT WINAPI PropVariantClear(PROPVARIANT * pvar) /* [in/out] */ case VT_BSTR: case VT_LPSTR: case VT_LPWSTR: + case VT_CLSID: FIXME("Freeing of vector sub-type not supported yet\n"); } - if (pvar->vt & VT_VECTOR) + if (pvar->vt & ~VT_VECTOR) { /* pick an arbitary VT_VECTOR structure - they all have the same * memory layout */ @@ -2408,7 +2471,13 @@ HRESULT WINAPI PropVariantCopy(PROPVARIANT *pvarDest, /* [out] */ const PROPVARIANT *pvarSrc) /* [in] */ { ULONG len; - TRACE("(%p, %p): stub:\n", pvarDest, pvarSrc); + HRESULT hr; + + TRACE("(%p, %p)\n", pvarDest, pvarSrc); + + hr = PROPVARIANT_ValidateType(pvarSrc->vt); + if (FAILED(hr)) + return hr; /* this will deal with most cases */ CopyMemory(pvarDest, pvarSrc, sizeof(*pvarDest)); @@ -2456,11 +2525,6 @@ HRESULT WINAPI PropVariantCopy(PROPVARIANT *pvarDest, /* [out] */ } break; default: - if (pvarSrc->vt & VT_ARRAY) - { - FIXME("Need to call SafeArrayCopy\n"); - /* SafeArrayCopy(...); */ - } if (pvarSrc->vt & VT_VECTOR) { int elemSize; diff --git a/dlls/ole32/tests/.cvsignore b/dlls/ole32/tests/.cvsignore index 13350e610c4..95dd44e4858 100644 --- a/dlls/ole32/tests/.cvsignore +++ b/dlls/ole32/tests/.cvsignore @@ -1,3 +1,4 @@ Makefile +propvariant.ok storage32.ok testlist.c diff --git a/dlls/ole32/tests/Makefile.in b/dlls/ole32/tests/Makefile.in index b0ab23ba92f..4eb7b7a7406 100644 --- a/dlls/ole32/tests/Makefile.in +++ b/dlls/ole32/tests/Makefile.in @@ -7,6 +7,7 @@ IMPORTS = ole32 kernel32 ntdll EXTRALIBS = -luuid CTESTS = \ + propvariant.c \ storage32.c @MAKE_TEST_RULES@ diff --git a/dlls/ole32/tests/propvariant.c b/dlls/ole32/tests/propvariant.c new file mode 100644 index 00000000000..acd99acb9dd --- /dev/null +++ b/dlls/ole32/tests/propvariant.c @@ -0,0 +1,166 @@ +/* + * PropVariant Tests + * + * Copyright 2004 Robert Shearman + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include "windows.h" +/* not present in Wine yet */ +/*#include "propidl.h"*/ +WINOLEAPI PropVariantClear(PROPVARIANT*); + +#include "wine/test.h" + +struct valid_mapping +{ + BOOL simple; + BOOL with_array; + BOOL with_vector; + BOOL byref; +} valid_types[] = +{ + { TRUE , FALSE, FALSE, FALSE }, /* VT_EMPTY */ + { TRUE , FALSE, FALSE, FALSE }, /* VT_NULL */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_I2 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_I4 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_R4 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_R8 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_CY */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_DATE */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_BSTR */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_DISPATCH */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_ERROR */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_BOOL */ + { FALSE, FALSE, TRUE , FALSE }, /* VT_VARIANT */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_UNKNOWN */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_DECIMAL */ + { FALSE, FALSE, FALSE, FALSE }, /* 15 */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_I1 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_UI1 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_UI2 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_UI4 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_I8 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_UI8 */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_INT */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_UINT */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_VOID */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_HRESULT */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_PTR */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_SAFEARRAY */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_CARRAY */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_USERDEFINED */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_LPSTR */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_LPWSTR */ + { FALSE, FALSE, FALSE, FALSE }, /* 32 */ + { FALSE, FALSE, FALSE, FALSE }, /* 33 */ + { FALSE, FALSE, FALSE, FALSE }, /* 34 */ + { FALSE, FALSE, FALSE, FALSE }, /* 35 */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_RECORD */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_INT_PTR */ + { FALSE, FALSE, FALSE, FALSE }, /* VT_UINT_PTR */ + { FALSE, FALSE, FALSE, FALSE }, /* 39 */ + { FALSE, FALSE, FALSE, FALSE }, /* 40 */ + { FALSE, FALSE, FALSE, FALSE }, /* 41 */ + { FALSE, FALSE, FALSE, FALSE }, /* 42 */ + { FALSE, FALSE, FALSE, FALSE }, /* 43 */ + { FALSE, FALSE, FALSE, FALSE }, /* 44 */ + { FALSE, FALSE, FALSE, FALSE }, /* 45 */ + { FALSE, FALSE, FALSE, FALSE }, /* 46 */ + { FALSE, FALSE, FALSE, FALSE }, /* 47 */ + { FALSE, FALSE, FALSE, FALSE }, /* 48 */ + { FALSE, FALSE, FALSE, FALSE }, /* 49 */ + { FALSE, FALSE, FALSE, FALSE }, /* 50 */ + { FALSE, FALSE, FALSE, FALSE }, /* 51 */ + { FALSE, FALSE, FALSE, FALSE }, /* 52 */ + { FALSE, FALSE, FALSE, FALSE }, /* 53 */ + { FALSE, FALSE, FALSE, FALSE }, /* 54 */ + { FALSE, FALSE, FALSE, FALSE }, /* 55 */ + { FALSE, FALSE, FALSE, FALSE }, /* 56 */ + { FALSE, FALSE, FALSE, FALSE }, /* 57 */ + { FALSE, FALSE, FALSE, FALSE }, /* 58 */ + { FALSE, FALSE, FALSE, FALSE }, /* 59 */ + { FALSE, FALSE, FALSE, FALSE }, /* 60 */ + { FALSE, FALSE, FALSE, FALSE }, /* 61 */ + { FALSE, FALSE, FALSE, FALSE }, /* 62 */ + { FALSE, FALSE, FALSE, FALSE }, /* 63 */ + { TRUE , FALSE, TRUE , FALSE }, /* VT_FILETIME */ + { TRUE , FALSE, FALSE, FALSE }, /* VT_BLOB */ + { TRUE , FALSE, FALSE, FALSE }, /* VT_STREAM */ + { TRUE , FALSE, FALSE, FALSE }, /* VT_STORAGE */ + { TRUE , FALSE, FALSE, FALSE }, /* VT_STREAMED_OBJECT */ + { TRUE , FALSE, FALSE, FALSE }, /* VT_STORED_OBJECT */ + { TRUE , FALSE, FALSE, FALSE }, /* VT_BLOB_OBJECT */ + { TRUE , FALSE, TRUE , FALSE } /* VT_CF */ +}; + +static const char* wine_vtypes[VT_CLSID+1] = +{ + "VT_EMPTY","VT_NULL","VT_I2","VT_I4","VT_R4","VT_R8","VT_CY","VT_DATE", + "VT_BSTR","VT_DISPATCH","VT_ERROR","VT_BOOL","VT_VARIANT","VT_UNKNOWN", + "VT_DECIMAL","15","VT_I1","VT_UI1","VT_UI2","VT_UI4","VT_I8","VT_UI8", + "VT_INT","VT_UINT","VT_VOID","VT_HRESULT","VT_PTR","VT_SAFEARRAY", + "VT_CARRAY","VT_USERDEFINED","VT_LPSTR","VT_LPWSTR","32","33","34","35", + "VT_RECORD","VT_INT_PTR","VT_UINT_PTR","39","40","41","42","43","44","45", + "46","47","48","49","50","51","52","53","54","55","56","57","58","59","60", + "61","62","63","VT_FILETIME","VT_BLOB","VT_STREAM","VT_STORAGE", + "VT_STREAMED_OBJECT","VT_STORED_OBJECT","VT_BLOB_OBJECT","VT_CF","VT_CLSID" +}; + +static void test_validtypes() +{ + PROPVARIANT propvar; + HRESULT hr; + int i; + + memset(&propvar, 0, sizeof(propvar)); + + for (i = 0; i < sizeof(valid_types)/sizeof(valid_types[0]); i++) + { + propvar.vt = i; + hr = PropVariantClear(&propvar); + ok(valid_types[i].simple == !(hr == STG_E_INVALIDPARAMETER), + "PropVariantClear(%s) should have returned 0x%08lx, but returned 0x%08lx\n", + wine_vtypes[i], + valid_types[i].simple ? S_OK : STG_E_INVALIDPARAMETER, hr); + + propvar.vt = i | VT_ARRAY; + hr = PropVariantClear(&propvar); + ok(valid_types[i].with_array == !(hr == STG_E_INVALIDPARAMETER), + "PropVariantClear(%s|VT_ARRAY) should have returned 0x%08lx, but returned 0x%08lx\n", + wine_vtypes[i], + valid_types[i].with_array ? S_OK : STG_E_INVALIDPARAMETER, hr); + + propvar.vt = i | VT_VECTOR; + hr = PropVariantClear(&propvar); + ok(valid_types[i].with_vector == !(hr == STG_E_INVALIDPARAMETER), + "PropVariantClear(%s|VT_VECTOR) should have returned 0x%08lx, but returned 0x%08lx\n", + wine_vtypes[i], + valid_types[i].with_vector ? S_OK : STG_E_INVALIDPARAMETER, hr); + + propvar.vt = i | VT_BYREF; + hr = PropVariantClear(&propvar); + ok(valid_types[i].byref == !(hr == STG_E_INVALIDPARAMETER), + "PropVariantClear(%s|VT_BYREF) should have returned 0x%08lx, but returned 0x%08lx\n", + wine_vtypes[i], + valid_types[i].byref ? S_OK : STG_E_INVALIDPARAMETER, hr); + } +} + +START_TEST(propvariant) +{ + test_validtypes(); +}