Use new string APIs and add memory safety for PyWinObject_AsPARAM

CHANGES.txt

@@ -11,6 +11,14 @@ or
 
 Coming in build 305, as yet unreleased
 --------------------------------------
+* Changes in PARAM handling. Some functions which returned a WPARAM or LPARAM
+  allowed you to return a pointer to a Python buffer object or a PyUnicode.
+  These functions now only accept a Python long to be returned. Note that
+  this DOES NOT apply to functions with accept WPARAM or LPARAM as arguments,
+  only when they are being returned. Impacted functions are `OnNotify`
+  handler, LV_ITEM/TV_ITEM objects, PyIContextMenu3::HandleMenuMsg2, and the
+  result of a WNDPROC/DLGPROC (#1927).
+
 * service registration had an overhaul, avoiding a complicated, and ultimately
   unnecessary "single globally registered service runner" concept.
   Now, when registering a service, the host pythonservice.exe runner will be

Pythonwin/win32notify.cpp

@@ -386,7 +386,7 @@ BOOL Python_OnNotify(CWnd *pFrom, WPARAM, LPARAM lParam, LRESULT *pResult)
             gui_print_error();
         }
     // Otherwise result is just the LRESULT, which can be anything that fits in pointer size
-    } else if (!PyWinObject_AsPARAM(result, (LPARAM *)&rc)) {
+    } else if (!PyWinObject_AsSimplePARAM(result, (LPARAM *)&rc)) {
         gui_print_error();
         PyErr_SetString(ui_module_error,
                         "OnNotify did not return an LRESULT, or a tuple of (LRESULT, notify info tuple)");

Pythonwin/win32util.cpp

@@ -436,7 +436,7 @@ BOOL PyWinObject_AsLV_ITEM(PyObject *args, LV_ITEM *pItem)
     if (len < 7)
         return TRUE;
     ob = PyTuple_GET_ITEM(args, 6);
-    if (!PyWinObject_AsPARAM(ob, &pItem->lParam)) {
+    if (!PyWinObject_AsSimplePARAM(ob, &pItem->lParam)) {
         PyWinObject_FreeLV_ITEM(pItem);
         return FALSE;
     }
@@ -740,7 +740,7 @@ BOOL PyWinObject_AsTV_ITEM(PyObject *args, TV_ITEM *pItem)
     ob = PyTuple_GET_ITEM(args, 7);
     if (ob != Py_None) {
         // @tupleitem 7|int|lParam|User defined integer param.
-        if (!PyWinObject_AsPARAM(ob, &pItem->lParam)) {
+        if (!PyWinObject_AsSimplePARAM(ob, &pItem->lParam)) {
             PyWinObject_FreeTV_ITEM(pItem);
             return FALSE;
         }

Pythonwin/win32win.cpp

@@ -804,8 +804,8 @@ static PyObject *ui_window_def_window_proc(PyObject *self, PyObject *args)
                           &obwparam,   // @pyparm int|idLast||The lParam for the message.
                           &oblparam))  // @pyparm int|idCheck||The wParam for the message.
         return NULL;
-    WPARAM wparam;
-    LPARAM lparam;
+    PyWin_PARAMHolder wparam;
+    PyWin_PARAMHolder lparam;
     if (!PyWinObject_AsPARAM(obwparam, &wparam) || !PyWinObject_AsPARAM(oblparam, &lparam))
         return NULL;
     GUI_BGN_SAVE;
@@ -1849,8 +1849,8 @@ static PyObject *ui_window_on_wnd_msg(PyObject *self, PyObject *args)
 {
     LRESULT res;
     int msg;
-    WPARAM wParam;
-    LPARAM lParam;
+    PyWin_PARAMHolder wParam;
+    PyWin_PARAMHolder lParam;
     PyObject *obwParam, *oblParam;
     CRect rect;
     BOOL bRepaint = TRUE;
@@ -1875,7 +1875,6 @@ static PyObject *ui_window_on_wnd_msg(PyObject *self, PyObject *args)
     // return value from the MFC function call, and the value of the
     // lResult param.  Please see the MFC documentation for more details.
     return Py_BuildValue("iN", rc, PyWinObject_FromPARAM(res));
-    RETURN_NONE;
 }
 
 // @pymethod |PyCWnd|PostMessage|Post a message to the window.
@@ -1891,8 +1890,8 @@ PyObject *ui_window_post_message(PyObject *self, PyObject *args)
                           &obwParam,   // @pyparm int|wParam|0|The wParam for the message
                           &oblParam))  // @pyparm int|lParam|0|The lParam for the message
         return NULL;
-    WPARAM wParam = 0;
-    LPARAM lParam = 0;
+    PyWin_PARAMHolder wParam;
+    PyWin_PARAMHolder lParam;
     if (obwParam != Py_None && !PyWinObject_AsPARAM(obwParam, &wParam))
         return NULL;
     if (oblParam != Py_None && !PyWinObject_AsPARAM(oblParam, &lParam))
@@ -2185,8 +2184,8 @@ PyObject *ui_window_send_message(PyObject *self, PyObject *args)
         return NULL;
     assert(!PyErr_Occurred());  // lingering exception?
     int message;
-    WPARAM wp = 0;
-    LPARAM lp = 0;
+    PyWin_PARAMHolder wp;
+    PyWin_PARAMHolder lp;
     BOOL ok = FALSE;
     PyWinBufferView pybuf;
     // Old code assumes the following behaviour:
@@ -2249,8 +2248,8 @@ PyObject *ui_window_send_message_to_desc(PyObject *self, PyObject *args)
             &oblParam,  // @pyparm int|lParam|0|The lParam for the message
             &bDeep))    // @pyparm int|bDeep|1|Indicates if the message should be recursively sent to all children
         return NULL;
-    WPARAM wParam = 0;
-    LPARAM lParam = 0;
+    PyWin_PARAMHolder wParam;
+    PyWin_PARAMHolder lParam;
     if (obwParam != Py_None && !PyWinObject_AsPARAM(obwParam, &wParam))
         return NULL;
     if (oblParam != Py_None && !PyWinObject_AsPARAM(oblParam, &lParam))

Pythonwin/win32win.h

@@ -121,8 +121,8 @@ PyObject *__DoBaseOnCommand(ui_type_CObject *type, PyObject *self, PyObject *arg
     PyObject *obwparam, *oblparam;
     if (!PyArg_ParseTuple(args, "OO", &obwparam, &oblparam))
         return NULL;
-    WPARAM wparam;
-    LPARAM lparam;
+    PyWin_PARAMHolder wparam;
+    PyWin_PARAMHolder lparam;
     if (!PyWinObject_AsPARAM(obwparam, &wparam))
         return NULL;
     if (!PyWinObject_AsPARAM(oblparam, &lparam))

com/win32comext/shell/src/PyIContextMenu3.cpp

@@ -42,7 +42,7 @@ STDMETHODIMP PyGContextMenu3::HandleMenuMsg2(UINT uMsg, WPARAM wParam, LPARAM lP
         if (ret == Py_None)
             *lpResult = FALSE;
         else {
-            PyWinObject_AsPARAM(ret, (WPARAM *)lpResult);
+            PyWinObject_AsSimplePARAM(ret, (WPARAM *)lpResult);
             hr = PyCom_SetAndLogCOMErrorFromPyException("HandleMenuMsg2", IID_IContextMenu3);
         }
     }

com/win32comext/shell/src/PyIShellBrowser.cpp

@@ -262,12 +262,11 @@ PyObject *PyIShellBrowser::SendControlMsg(PyObject *self, PyObject *args)
     PyObject *obwparam, *oblparam;
     if (!PyArg_ParseTuple(args, "IIOO:SendControlMsg", &id, &uMsg, &obwparam, &oblparam))
         return NULL;
-    WPARAM wParam;
-    LPARAM lParam;
+    PyWin_PARAMHolder wParam;
+    PyWin_PARAMHolder lParam;
     if (!PyWinObject_AsPARAM(obwparam, &wParam))
         return NULL;
-    // WPARAM and LPARAM are defined as UINT_PTR and LONG_PTR, so they can't be used interchangeably without a cast
-    if (!PyWinObject_AsPARAM(oblparam, (WPARAM *)&lParam))
+    if (!PyWinObject_AsPARAM(oblparam, &lParam))
         return NULL;
 
     HRESULT hr;

com/win32comext/shell/src/PyIShellFolder.cpp

@@ -199,7 +199,7 @@ PyObject *PyIShellFolder::CompareIDs(PyObject *self, PyObject *args)
     PyObject *obpidl2;
     PyObject *ret = NULL;
     PyObject *oblparam;
-    LPARAM lparam;
+    PyWin_PARAMHolder lparam;
     ITEMIDLIST *pidl1 = NULL;
     ITEMIDLIST *pidl2 = NULL;
     if (!PyArg_ParseTuple(args, "OOO:CompareIDs", &oblparam, &obpidl1, &obpidl2))

win32/src/PyWinTypes.h

@@ -354,12 +354,61 @@ PYWINTYPES_EXPORT void PyWinObject_FreeResourceIdA(char *resource_id);
 PYWINTYPES_EXPORT BOOL PyWinObject_AsResourceId(PyObject *ob, WCHAR **presource_id, BOOL bNoneOK = FALSE);
 PYWINTYPES_EXPORT void PyWinObject_FreeResourceId(WCHAR *resource_id);
 
-// WPARAM and LPARAM conversion
-PYWINTYPES_EXPORT BOOL PyWinObject_AsPARAM(PyObject *ob, WPARAM *pparam);
+// WPARAM and LPARAM conversion.
+// Auto-freed WPARAM / LPARAM which ensure any memory referenced remains valid when a String or
+// Buffer object is used. Make sure the destructor is called with the GIL held.
+class PyWin_PARAMHolder {
+    protected:
+        WPARAM _pa;
+        // Holds *either* a PyWinBufferView (which will auto-free) *or* a "void *" that we
+        // will auto-free.
+        PyWinBufferView _bufferView;
+        void *_pymem;
+        void _free() {
+            if (_pymem) {
+                PyMem_Free(_pymem);
+                _pymem = NULL;
+            }
+            _bufferView.release();
+            _pa = NULL;
+        }
+  public:
+    PyWin_PARAMHolder(WPARAM t=0):_pa(t),_pymem(NULL) {}
+    ~PyWin_PARAMHolder() {
+        _free();
+    }
+    WCHAR *set_allocated(WCHAR *t) {
+        assert(!_bufferView.ptr()); // should be one or the other.
+        _free();
+        _pymem = t;
+        _pa = (WPARAM)t;
+        return t;
+    }
+    bool init_buffer(PyObject *ob) {
+        assert(!_pymem); // should be one or the other!
+        _free();
+        if (!_bufferView.init(ob)) {
+            return false;
+        }
+        _pa = (WPARAM)_bufferView.ptr();
+        return true;
+    }
+
+    WPARAM operator=(WPARAM t) {
+        _free();
+        return _pa = t;
+    }
+    operator WPARAM() { return _pa; }
+    operator LPARAM() { return (LPARAM)_pa; }
+};
+
+PYWINTYPES_EXPORT BOOL PyWinObject_AsPARAM(PyObject *ob, PyWin_PARAMHolder *pparam);
 inline PyObject *PyWinObject_FromPARAM(WPARAM param) { return PyWinObject_FromULONG_PTR(param); }
-inline BOOL PyWinObject_AsPARAM(PyObject *ob, LPARAM *pparam) { return PyWinObject_AsPARAM(ob, (WPARAM *)pparam); }
 inline PyObject *PyWinObject_FromPARAM(LPARAM param) { return PyWinObject_FromULONG_PTR(param); }
 
+PYWINTYPES_EXPORT BOOL PyWinObject_AsSimplePARAM(PyObject *ob, WPARAM *pparam);
+inline BOOL PyWinObject_AsSimplePARAM(PyObject *ob, LPARAM *pparam) { return PyWinObject_AsSimplePARAM(ob, (WPARAM *)pparam); }
+
 // RECT conversions
 // @object PyRECT|Tuple of 4 ints defining a rectangle: (left, top, right, bottom)
 PYWINTYPES_EXPORT BOOL PyWinObject_AsRECT(PyObject *obrect, LPRECT prect);

win32/src/PyWinTypesmodule.cpp

@@ -602,32 +602,44 @@ void PyWinObject_FreeResourceId(WCHAR *resource_id)
         PyWinObject_FreeWCHAR(resource_id);
 }
 
+// Conversion for WPARAM and LPARAM from a simple integer value. Used when we
+// can't guarantee memory pointed at will remain valid as long as necessary.
+// In that scenario, the caller is responsible for arranging memory safety.
+BOOL PyWinObject_AsSimplePARAM(PyObject *ob, WPARAM *wparam) {
+    if (PyWinLong_AsVoidPtr(ob, (void **)wparam)) {
+        return TRUE;
+    }
+
+    PyErr_Format(PyExc_TypeError, "WPARAM is simple, so must be an int object (got %s)",
+                 ob->ob_type->tp_name);
+    return FALSE;
+}
+
 // Conversion for WPARAM and LPARAM
 // (WPARAM is defined as UINT_PTR, and LPARAM is defined as LONG_PTR - see
 // pywintypes.h for inline functions to resolve this)
-BOOL PyWinObject_AsPARAM(PyObject *ob, WPARAM *pparam)
+BOOL PyWinObject_AsPARAM(PyObject *ob, PyWin_PARAMHolder *holder)
 {
     assert(!PyErr_Occurred());  // lingering exception?
     if (ob == NULL || ob == Py_None) {
-        *pparam = NULL;
+        *holder = (WPARAM)0;
         return TRUE;
     }
 
     if (PyUnicode_Check(ob)) {
-        *pparam = (WPARAM)PyUnicode_AS_UNICODE(ob);
-        return TRUE;
+        return holder->set_allocated(PyUnicode_AsWideCharString(ob, NULL)) != NULL;
     }
-    PyWinBufferView pybuf(ob);
-    if (pybuf.ok()) {
-        // note: this might be unsafe, as we give away the buffer pointer to a
-        // client outside of the scope where our RAII object 'pybuf' resides.
-        *pparam = (WPARAM)pybuf.ptr();
+
+    if (holder->init_buffer(ob)) {
         return TRUE;
     }
 
     PyErr_Clear();
-    if (PyWinLong_AsVoidPtr(ob, (void **)pparam))
+    void *simple = NULL;
+    if (PyWinLong_AsVoidPtr(ob, &simple)) {
+        *holder = (WPARAM)simple;
         return TRUE;
+    }
 
     PyErr_Format(PyExc_TypeError, "WPARAM must be a unicode string, int, or buffer object (got %s)",
                  ob->ob_type->tp_name);
@@ -749,8 +761,8 @@ BOOL PyWinObject_AsMSG(PyObject *ob, MSG *pMsg)
                                         // coordinates, when the message was posted.
                           &pMsg->pt.y))
         return FALSE;
-    return PyWinObject_AsHANDLE(obhwnd, (HANDLE *)&pMsg->hwnd) && PyWinObject_AsPARAM(obwParam, &pMsg->wParam) &&
-           PyWinObject_AsPARAM(oblParam, &pMsg->lParam);
+    return PyWinObject_AsHANDLE(obhwnd, (HANDLE *)&pMsg->hwnd) && PyWinObject_AsSimplePARAM(obwParam, &pMsg->wParam) &&
+           PyWinObject_AsSimplePARAM(oblParam, &pMsg->lParam);
 }
 
 PyObject *PyWinObject_FromMSG(const MSG *pMsg)

win32/src/win32apimodule.cpp

@@ -2692,8 +2692,8 @@ PyObject *PyPostMessage(PyObject *self, PyObject *args)
     HWND hwnd;
     PyObject *obhwnd, *obwParam = Py_None, *oblParam = Py_None;
     UINT message;
-    WPARAM wParam = 0;
-    LPARAM lParam = 0;
+    PyWin_PARAMHolder wParam = 0;
+    PyWin_PARAMHolder lParam = 0;
     if (!PyArg_ParseTuple(args, "OI|OO:PostMessage",
                           &obhwnd,     // @pyparm <o PyHANDLE>|hwnd||The hWnd of the window to receive the message.
                           &message,    // @pyparm int|idMessage||The ID of the message to post.
@@ -2704,11 +2704,13 @@ PyObject *PyPostMessage(PyObject *self, PyObject *args)
         return NULL;
     if (!PyWinObject_AsPARAM(obwParam, &wParam))
         return NULL;
-    if (!PyWinObject_AsPARAM(oblParam, (WPARAM *)&lParam))
+    if (!PyWinObject_AsPARAM(oblParam, &lParam))
         return NULL;
     // @pyseeapi PostMessage
-    PyW32_BEGIN_ALLOW_THREADS BOOL ok = ::PostMessage(hwnd, message, wParam, lParam);
-    PyW32_END_ALLOW_THREADS if (!ok) return ReturnAPIError("PostMessage");
+    PyW32_BEGIN_ALLOW_THREADS;
+    BOOL ok = ::PostMessage(hwnd, message, wParam, lParam);
+    PyW32_END_ALLOW_THREADS;
+    if (!ok) return ReturnAPIError("PostMessage");
     Py_INCREF(Py_None);
     return Py_None;
 }
@@ -2718,8 +2720,8 @@ PyObject *PyPostThreadMessage(PyObject *self, PyObject *args)
 {
     DWORD threadId;
     UINT message;
-    WPARAM wParam = 0;
-    LPARAM lParam = 0;
+    PyWin_PARAMHolder wParam = 0;
+    PyWin_PARAMHolder lParam = 0;
     PyObject *obwParam = Py_None, *oblParam = Py_None;
     if (!PyArg_ParseTuple(args, "iI|OO:PostThreadMessage",
                           &threadId,   // @pyparm int|tid||Identifier of the thread to which the message will be posted.
@@ -2729,12 +2731,14 @@ PyObject *PyPostThreadMessage(PyObject *self, PyObject *args)
         return NULL;
     if (!PyWinObject_AsPARAM(obwParam, &wParam))
         return NULL;
-    if (!PyWinObject_AsPARAM(oblParam, (WPARAM *)&lParam))
+    if (!PyWinObject_AsPARAM(oblParam, &lParam))
         return NULL;
 
     // @pyseeapi PostThreadMessage
-    PyW32_BEGIN_ALLOW_THREADS BOOL ok = ::PostThreadMessage(threadId, message, wParam, lParam);
-    PyW32_END_ALLOW_THREADS if (!ok) return ReturnAPIError("PostThreadMessage");
+    PyW32_BEGIN_ALLOW_THREADS;
+    BOOL ok = ::PostThreadMessage(threadId, message, wParam, lParam);
+    PyW32_END_ALLOW_THREADS;
+    if (!ok) return ReturnAPIError("PostThreadMessage");
     Py_INCREF(Py_None);
     return Py_None;
 }
@@ -4179,8 +4183,8 @@ PyObject *PySendMessage(PyObject *self, PyObject *args)
     HWND hwnd;
     PyObject *obhwnd, *obwParam = Py_None, *oblParam = Py_None;
     UINT message;
-    WPARAM wParam = 0;
-    LPARAM lParam = 0;
+    PyWin_PARAMHolder wParam;
+    PyWin_PARAMHolder lParam;
     if (!PyArg_ParseTuple(args, "OI|OO:SendMessage",
                           &obhwnd,     // @pyparm <o PyHANDLE>|hwnd||The hWnd of the window to receive the message.
                           &message,    // @pyparm int|idMessage||The ID of the message to send.
@@ -4191,12 +4195,14 @@ PyObject *PySendMessage(PyObject *self, PyObject *args)
         return NULL;
     if (!PyWinObject_AsPARAM(obwParam, &wParam))
         return NULL;
-    if (!PyWinObject_AsPARAM(oblParam, (WPARAM *)&lParam))
+    if (!PyWinObject_AsPARAM(oblParam, &lParam))
         return NULL;
     LRESULT rc;
     // @pyseeapi SendMessage
-    PyW32_BEGIN_ALLOW_THREADS rc = ::SendMessage(hwnd, message, wParam, lParam);
-    PyW32_END_ALLOW_THREADS return PyWinLong_FromVoidPtr((void *)rc);
+    PyW32_BEGIN_ALLOW_THREADS;
+    rc = ::SendMessage(hwnd, message, wParam, lParam);
+    PyW32_END_ALLOW_THREADS;
+    return PyWinLong_FromVoidPtr((void *)rc);
 }
 
 // @pymethod |win32api|SetConsoleTitle|Sets the title for the current console.