Skip to content

gh-106844: Fix null-bytes handling in LCMapStringEx in _winapi #106857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sobolevn wants to merge 6 commits into from
Closed

gh-106844: Fix null-bytes handling in LCMapStringEx in _winapi #106857

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dff8b2f
gh-106844: Fix null-bytes handling in `LCMapStringEx` in `_winapi`
sobolevn Jul 18, 2023
46be70b
Fix CI
sobolevn Jul 18, 2023
13ce79c
Keep simpler code for the locale parameter.
serhiy-storchaka Aug 10, 2023
88d4193
Handle large strings containing the null character.
serhiy-storchaka Aug 10, 2023
73551b9
Merge branch 'main' into issue-106844
serhiy-storchaka Aug 10, 2023
26161c3
Fix other issues.
serhiy-storchaka Aug 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Lib/test/test_ntpath.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,7 @@ def test_path_normcase(self):
self._check_function(self.path.normcase)
if sys.platform == 'win32':
self.assertEqual(ntpath.normcase('\u03a9\u2126'), 'ωΩ')
self.assertEqual(ntpath.normcase('abc\x00def'), 'abc\x00def')

def test_path_isabs(self):
self._check_function(self.path.isabs)
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix null-bytes handling in ``LCMapStringEx`` which affects
:func:`ntpath.normcase`.
40 changes: 32 additions & 8 deletions Modules/_winapi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1537,42 +1537,66 @@ _winapi_PeekNamedPipe_impl(PyObject *module, HANDLE handle, int size)
/*[clinic input]
_winapi.LCMapStringEx

locale: LPCWSTR
locale: unicode

@serhiy-storchaka serhiy-storchaka Jul 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change it for locale?

@sobolevn sobolevn Jul 18, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just backported the solution from 927b5af#diff-42415407f8d0ef2d42e29d13d979f633e3543770e62c3871e1101ad532d336a8R1518

So, 3.11 and 3.12 would match.

@serhiy-storchaka serhiy-storchaka Jul 19, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code would be simpler if keep the LPCWSTR converter here.

@zooba zooba Aug 9, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. We don't need to treat this argument any differently - it's only the src argument that requires special handling.

flags: DWORD
src: LPCWSTR
src: unicode

[clinic start generated code]*/

static PyObject *
_winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags,
LPCWSTR src)
/*[clinic end generated code: output=cf4713d80e2b47c9 input=9fe26f95d5ab0001]*/
_winapi_LCMapStringEx_impl(PyObject *module, PyObject *locale, DWORD flags,
PyObject *src)
/*[clinic end generated code: output=8ea4c9d85a4a1f23 input=2fa6ebc92591731b]*/
{
if (flags & (LCMAP_SORTHANDLE | LCMAP_HASH | LCMAP_BYTEREV |
LCMAP_SORTKEY)) {
return PyErr_Format(PyExc_ValueError, "unsupported flags");
}

int dest_size = LCMapStringEx(locale, flags, src, -1, NULL, 0,
wchar_t *locale_ = PyUnicode_AsWideCharString(locale, NULL);
if (!locale_) {
return NULL;
}
Py_ssize_t srcLenAsSsize;
int srcLen;
wchar_t *src_ = PyUnicode_AsWideCharString(src, &srcLenAsSsize);
if (!src_) {
PyMem_Free(locale_);
return NULL;
}
srcLen = (int)srcLenAsSsize;
if (srcLen != srcLenAsSsize) {
srcLen = -1;
}

@serhiy-storchaka serhiy-storchaka Jul 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would it work for large (more than 2**32 characters) string containing NUL?

@zooba zooba Aug 9, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't, but that's the caller's responsibility.

@serhiy-storchaka serhiy-storchaka Aug 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LCMapStringEx() will simply return the result for the truncated input. It is bad. How is the caller expected to detect this problem if it only occurs in special circumferences?

What does LCMapStringEx() return for large input not containing NUL? Most likely it fails. We should either emulate this response (raise an OSError with the same Windows error code) or simply raise an OverflowError.


int dest_size = LCMapStringEx(locale_, flags, src_, srcLen, NULL, 0,
NULL, NULL, 0);
if (dest_size == 0) {
PyMem_Free(locale_);
PyMem_Free(src_);
return PyErr_SetFromWindowsErr(0);
}

wchar_t* dest = PyMem_NEW(wchar_t, dest_size);
if (dest == NULL) {
PyMem_Free(locale_);
PyMem_Free(src_);
return PyErr_NoMemory();
}

int nmapped = LCMapStringEx(locale, flags, src, -1, dest, dest_size,
int nmapped = LCMapStringEx(locale_, flags, src_, srcLen, dest, dest_size,
NULL, NULL, 0);
if (nmapped == 0) {
DWORD error = GetLastError();
PyMem_Free(locale_);
PyMem_Free(src_);
PyMem_DEL(dest);
return PyErr_SetFromWindowsErr(error);
}

PyObject *ret = PyUnicode_FromWideChar(dest, dest_size - 1);
PyObject *ret = PyUnicode_FromWideChar(dest, dest_size);
PyMem_Free(locale_);
PyMem_Free(src_);
PyMem_DEL(dest);

return ret;
Expand Down
19 changes: 7 additions & 12 deletions Modules/clinic/_winapi.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.