From 7a4cb3cf167b20e6b7f6cf9eab2ec614a41442fd Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 29 May 2024 16:51:09 +0100 Subject: [PATCH 1/3] gh-119690: Adds Unicode support for named pipes in _winapi (GH-119717) --- Lib/test/audit-tests.py | 11 ++++++ Lib/test/test_audit.py | 14 ++++++++ Lib/test/test_winapi.py | 34 ++++++++++++++++++- ...-05-29-11-06-12.gh-issue-119690.8q6e1p.rst | 1 + Modules/_winapi.c | 33 +++++++++--------- Modules/clinic/_winapi.c.h | 24 ++++++++----- 6 files changed, 90 insertions(+), 27 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2024-05-29-11-06-12.gh-issue-119690.8q6e1p.rst diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py index 9504829e96f00e..f9914d5ac3b5a8 100644 --- a/Lib/test/audit-tests.py +++ b/Lib/test/audit-tests.py @@ -525,6 +525,17 @@ def hook(event, args): sys.monitoring.register_callback(1, 1, None) +def test_winapi_createnamedpipe(pipe_name): + import _winapi + + def hook(event, args): + if event == "_winapi.CreateNamedPipe": + print(event, args) + + sys.addaudithook(hook) + _winapi.CreateNamedPipe(pipe_name, _winapi.PIPE_ACCESS_DUPLEX, 8, 2, 0, 0, 0, 0) + + if __name__ == "__main__": from test.support import suppress_msvcrt_asserts diff --git a/Lib/test/test_audit.py b/Lib/test/test_audit.py index b12ffa5d872e83..9e3e03748da14d 100644 --- a/Lib/test/test_audit.py +++ b/Lib/test/test_audit.py @@ -269,6 +269,20 @@ def test_sys_monitoring_register_callback(self): self.assertEqual(actual, expected) + def test_winapi_createnamedpipe(self): + winapi = import_helper.import_module("_winapi") + + pipe_name = r"\\.\pipe\LOCAL\test_winapi_createnamed_pipe" + returncode, events, stderr = self.run_python("test_winapi_createnamedpipe", pipe_name) + if returncode: + self.fail(stderr) + + if support.verbose: + print(*events, sep='\n') + actual = [(ev[0], ev[2]) for ev in events] + expected = [("_winapi.CreateNamedPipe", f"({pipe_name!r}, 3, 8)")] + + self.assertEqual(actual, expected) if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_winapi.py b/Lib/test/test_winapi.py index 94af7c3237bb9d..53cf85b25b9675 100644 --- a/Lib/test/test_winapi.py +++ b/Lib/test/test_winapi.py @@ -4,7 +4,7 @@ import pathlib import re import unittest -from test.support import import_helper +from test.support import import_helper, os_helper _winapi = import_helper.import_module('_winapi', required_on=['win']) @@ -38,3 +38,35 @@ def test_getshortpathname(self): # Should contain "PROGRA~" but we can't predict the number self.assertIsNotNone(re.match(r".\:\\PROGRA~\d", actual.upper()), actual) + + def test_namedpipe(self): + pipe_name = rf"\\.\pipe\LOCAL\{os_helper.TESTFN}" + + # Pipe does not exist, so this raises + with self.assertRaises(FileNotFoundError): + _winapi.WaitNamedPipe(pipe_name, 0) + + pipe = _winapi.CreateNamedPipe( + pipe_name, + _winapi.PIPE_ACCESS_DUPLEX, + 8, # 8=PIPE_REJECT_REMOTE_CLIENTS + 2, # two instances available + 32, 32, 0, 0) + self.addCleanup(_winapi.CloseHandle, pipe) + + # Pipe instance is available, so this passes + _winapi.WaitNamedPipe(pipe_name, 0) + + with open(pipe_name, 'w+b') as pipe2: + # No instances available, so this times out + # (WinError 121 does not get mapped to TimeoutError) + with self.assertRaises(OSError): + _winapi.WaitNamedPipe(pipe_name, 0) + + _winapi.WriteFile(pipe, b'testdata') + self.assertEqual(b'testdata', pipe2.read(8)) + + self.assertEqual((b'', 0), _winapi.PeekNamedPipe(pipe, 8)[:2]) + pipe2.write(b'testdata') + pipe2.flush() + self.assertEqual((b'testdata', 8), _winapi.PeekNamedPipe(pipe, 8)[:2]) diff --git a/Misc/NEWS.d/next/Windows/2024-05-29-11-06-12.gh-issue-119690.8q6e1p.rst b/Misc/NEWS.d/next/Windows/2024-05-29-11-06-12.gh-issue-119690.8q6e1p.rst new file mode 100644 index 00000000000000..84dd2161aa1db8 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2024-05-29-11-06-12.gh-issue-119690.8q6e1p.rst @@ -0,0 +1 @@ +Adds Unicode support and fixes audit events for ``_winapi.CreateNamedPipe``. diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 746f2a5543aee0..edb1181809c53b 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -186,7 +186,6 @@ create_converter('LPCVOID', '" F_POINTER "') create_converter('BOOL', 'i') # F_BOOL used previously (always 'i') create_converter('DWORD', 'k') # F_DWORD is always "k" (which is much shorter) -create_converter('LPCTSTR', 's') create_converter('UINT', 'I') # F_UINT used previously (always 'I') class LPCWSTR_converter(Py_UNICODE_converter): @@ -221,7 +220,7 @@ class LPVOID_return_converter(CReturnConverter): data.return_conversion.append( 'return_value = HANDLE_TO_PYNUM(_return_value);\n') [python start generated code]*/ -/*[python end generated code: output=da39a3ee5e6b4b0d input=011ee0c3a2244bfe]*/ +/*[python end generated code: output=da39a3ee5e6b4b0d input=ae30321c4cb150dd]*/ #include "clinic/_winapi.c.h" @@ -459,7 +458,7 @@ _winapi_CreateFile_impl(PyObject *module, LPCWSTR file_name, { HANDLE handle; - if (PySys_Audit("_winapi.CreateFile", "uIIII", + if (PySys_Audit("_winapi.CreateFile", "ukkkk", file_name, desired_access, share_mode, creation_disposition, flags_and_attributes) < 0) { return INVALID_HANDLE_VALUE; @@ -675,7 +674,7 @@ _winapi_CreateJunction_impl(PyObject *module, LPCWSTR src_path, /*[clinic input] _winapi.CreateNamedPipe -> HANDLE - name: LPCTSTR + name: LPCWSTR open_mode: DWORD pipe_mode: DWORD max_instances: DWORD @@ -687,25 +686,25 @@ _winapi.CreateNamedPipe -> HANDLE [clinic start generated code]*/ static HANDLE -_winapi_CreateNamedPipe_impl(PyObject *module, LPCTSTR name, DWORD open_mode, +_winapi_CreateNamedPipe_impl(PyObject *module, LPCWSTR name, DWORD open_mode, DWORD pipe_mode, DWORD max_instances, DWORD out_buffer_size, DWORD in_buffer_size, DWORD default_timeout, LPSECURITY_ATTRIBUTES security_attributes) -/*[clinic end generated code: output=80f8c07346a94fbc input=5a73530b84d8bc37]*/ +/*[clinic end generated code: output=7d6fde93227680ba input=5bd4e4a55639ee02]*/ { HANDLE handle; - if (PySys_Audit("_winapi.CreateNamedPipe", "uII", + if (PySys_Audit("_winapi.CreateNamedPipe", "ukk", name, open_mode, pipe_mode) < 0) { return INVALID_HANDLE_VALUE; } Py_BEGIN_ALLOW_THREADS - handle = CreateNamedPipe(name, open_mode, pipe_mode, - max_instances, out_buffer_size, - in_buffer_size, default_timeout, - security_attributes); + handle = CreateNamedPipeW(name, open_mode, pipe_mode, + max_instances, out_buffer_size, + in_buffer_size, default_timeout, + security_attributes); Py_END_ALLOW_THREADS if (handle == INVALID_HANDLE_VALUE) @@ -1720,7 +1719,7 @@ _winapi_OpenProcess_impl(PyObject *module, DWORD desired_access, { HANDLE handle; - if (PySys_Audit("_winapi.OpenProcess", "II", + if (PySys_Audit("_winapi.OpenProcess", "kk", process_id, desired_access) < 0) { return INVALID_HANDLE_VALUE; } @@ -2005,19 +2004,19 @@ _winapi_VirtualQuerySize_impl(PyObject *module, LPCVOID address) /*[clinic input] _winapi.WaitNamedPipe - name: LPCTSTR + name: LPCWSTR timeout: DWORD / [clinic start generated code]*/ static PyObject * -_winapi_WaitNamedPipe_impl(PyObject *module, LPCTSTR name, DWORD timeout) -/*[clinic end generated code: output=c2866f4439b1fe38 input=36fc781291b1862c]*/ +_winapi_WaitNamedPipe_impl(PyObject *module, LPCWSTR name, DWORD timeout) +/*[clinic end generated code: output=e161e2e630b3e9c2 input=099a4746544488fa]*/ { BOOL success; Py_BEGIN_ALLOW_THREADS - success = WaitNamedPipe(name, timeout); + success = WaitNamedPipeW(name, timeout); Py_END_ALLOW_THREADS if (!success) @@ -2382,7 +2381,7 @@ _winapi_CopyFile2_impl(PyObject *module, LPCWSTR existing_file_name, HRESULT hr; COPYFILE2_EXTENDED_PARAMETERS params = { sizeof(COPYFILE2_EXTENDED_PARAMETERS) }; - if (PySys_Audit("_winapi.CopyFile2", "uuI", + if (PySys_Audit("_winapi.CopyFile2", "uuk", existing_file_name, new_file_name, flags) < 0) { return NULL; } diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h index 6d731e6506c8de..d7ad551908fe46 100644 --- a/Modules/clinic/_winapi.c.h +++ b/Modules/clinic/_winapi.c.h @@ -307,7 +307,7 @@ PyDoc_STRVAR(_winapi_CreateNamedPipe__doc__, {"CreateNamedPipe", _PyCFunction_CAST(_winapi_CreateNamedPipe), METH_FASTCALL, _winapi_CreateNamedPipe__doc__}, static HANDLE -_winapi_CreateNamedPipe_impl(PyObject *module, LPCTSTR name, DWORD open_mode, +_winapi_CreateNamedPipe_impl(PyObject *module, LPCWSTR name, DWORD open_mode, DWORD pipe_mode, DWORD max_instances, DWORD out_buffer_size, DWORD in_buffer_size, DWORD default_timeout, @@ -317,7 +317,7 @@ static PyObject * _winapi_CreateNamedPipe(PyObject *module, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; - LPCTSTR name; + LPCWSTR name = NULL; DWORD open_mode; DWORD pipe_mode; DWORD max_instances; @@ -327,8 +327,8 @@ _winapi_CreateNamedPipe(PyObject *module, PyObject *const *args, Py_ssize_t narg LPSECURITY_ATTRIBUTES security_attributes; HANDLE _return_value; - if (!_PyArg_ParseStack(args, nargs, "skkkkkk" F_POINTER ":CreateNamedPipe", - &name, &open_mode, &pipe_mode, &max_instances, &out_buffer_size, &in_buffer_size, &default_timeout, &security_attributes)) { + if (!_PyArg_ParseStack(args, nargs, "O&kkkkkk" F_POINTER ":CreateNamedPipe", + _PyUnicode_WideCharString_Converter, &name, &open_mode, &pipe_mode, &max_instances, &out_buffer_size, &in_buffer_size, &default_timeout, &security_attributes)) { goto exit; } _return_value = _winapi_CreateNamedPipe_impl(module, name, open_mode, pipe_mode, max_instances, out_buffer_size, in_buffer_size, default_timeout, security_attributes); @@ -341,6 +341,9 @@ _winapi_CreateNamedPipe(PyObject *module, PyObject *const *args, Py_ssize_t narg return_value = HANDLE_TO_PYNUM(_return_value); exit: + /* Cleanup for name */ + PyMem_Free((void *)name); + return return_value; } @@ -1235,22 +1238,25 @@ PyDoc_STRVAR(_winapi_WaitNamedPipe__doc__, {"WaitNamedPipe", _PyCFunction_CAST(_winapi_WaitNamedPipe), METH_FASTCALL, _winapi_WaitNamedPipe__doc__}, static PyObject * -_winapi_WaitNamedPipe_impl(PyObject *module, LPCTSTR name, DWORD timeout); +_winapi_WaitNamedPipe_impl(PyObject *module, LPCWSTR name, DWORD timeout); static PyObject * _winapi_WaitNamedPipe(PyObject *module, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; - LPCTSTR name; + LPCWSTR name = NULL; DWORD timeout; - if (!_PyArg_ParseStack(args, nargs, "sk:WaitNamedPipe", - &name, &timeout)) { + if (!_PyArg_ParseStack(args, nargs, "O&k:WaitNamedPipe", + _PyUnicode_WideCharString_Converter, &name, &timeout)) { goto exit; } return_value = _winapi_WaitNamedPipe_impl(module, name, timeout); exit: + /* Cleanup for name */ + PyMem_Free((void *)name); + return return_value; } @@ -1622,4 +1628,4 @@ _winapi_CopyFile2(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyO return return_value; } -/*[clinic end generated code: output=ba2d5ae3f23701b7 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=91b39b70024fa232 input=a9049054013a1b77]*/ From d1363782dcf11d082022ad0397b7c6e35e622587 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 29 May 2024 17:41:31 +0100 Subject: [PATCH 2/3] Pass audit test args --- Lib/test/audit-tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py index f9914d5ac3b5a8..2f848b5e66fdc9 100644 --- a/Lib/test/audit-tests.py +++ b/Lib/test/audit-tests.py @@ -542,4 +542,4 @@ def hook(event, args): suppress_msvcrt_asserts() test = sys.argv[1] - globals()[test]() + globals()[test](*sys.argv[2:]) From 7b89ed9e07c4b399e14ceb8da5de6a0fa78cb4a7 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 29 May 2024 19:01:15 +0100 Subject: [PATCH 3/3] Fix other audit test now getting an argument --- Lib/test/audit-tests.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py index 2f848b5e66fdc9..21c8dd5ed91dec 100644 --- a/Lib/test/audit-tests.py +++ b/Lib/test/audit-tests.py @@ -186,7 +186,7 @@ class C(A): ) -def test_open(): +def test_open(testfn): # SSLContext.load_dh_params uses _Py_fopen_obj rather than normal open() try: import ssl @@ -199,11 +199,11 @@ def test_open(): # All of them should fail with TestHook(raise_on_events={"open"}) as hook: for fn, *args in [ - (open, sys.argv[2], "r"), + (open, testfn, "r"), (open, sys.executable, "rb"), (open, 3, "wb"), - (open, sys.argv[2], "w", -1, None, None, None, False, lambda *a: 1), - (load_dh_params, sys.argv[2]), + (open, testfn, "w", -1, None, None, None, False, lambda *a: 1), + (load_dh_params, testfn), ]: if not fn: continue @@ -216,11 +216,11 @@ def test_open(): [ i for i in [ - (sys.argv[2], "r"), + (testfn, "r"), (sys.executable, "r"), (3, "w"), - (sys.argv[2], "w"), - (sys.argv[2], "rb") if load_dh_params else None, + (testfn, "w"), + (testfn, "rb") if load_dh_params else None, ] if i is not None ],