[PATCH 0/1] MR6100: msxml3: Ensure node name is null terminated.
From: Daniel Lehman <dlehman25(a)gmail.com> --- dlls/msxml3/node.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/msxml3/node.c b/dlls/msxml3/node.c index 5a3238edca8..690a38f0d0b 100644 --- a/dlls/msxml3/node.c +++ b/dlls/msxml3/node.c @@ -199,8 +199,8 @@ HRESULT node_get_nodeName(xmlnode *This, BSTR *name) static const WCHAR colW = ':'; WCHAR *ptr; - /* +1 for ':' */ - ptr = *name = SysAllocStringLen(NULL, SysStringLen(base) + SysStringLen(prefix) + 1); + /* +1 for possible ':', +1 for \0 */ + ptr = *name = SysAllocStringLen(NULL, SysStringLen(base) + SysStringLen(prefix) + 1 + 1); if (SysStringByteLen(prefix)) { memcpy(ptr, prefix, SysStringByteLen(prefix)); @@ -211,8 +211,10 @@ HRESULT node_get_nodeName(xmlnode *This, BSTR *name) if (SysStringByteLen(prefix)) memcpy(ptr++, &colW, sizeof(WCHAR)); memcpy(ptr, base, SysStringByteLen(base)); + ptr += SysStringLen(base); } + *ptr = 0; SysFreeString(base); SysFreeString(prefix); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6100
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147168 Your paranoid android. === debian11b (64 bit WoW report) === kernel32: comm.c:1574: Test failed: AbortWaitCts hComPortEvent failed comm.c:1586: Test failed: Unexpected time 1001, expected around 500
@nsivov this ok to go in? it fixes random garbage added to xml because of the missing \0 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6100#note_78797
On Thu Aug 15 08:33:21 2024 +0000, Daniel Lehman wrote:
@nsivov this ok to go in? it fixes random garbage added to xml because of the missing \0 Are you sure you need to set the null explicitly? SysAllocStringLen(NULL) already initializes the whole buffer I think.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6100#note_78806
On Thu Aug 15 08:33:21 2024 +0000, Nikolay Sivov wrote:
Are you sure you need to set the null explicitly? SysAllocStringLen(NULL) already initializes the whole buffer I think. ah... it does. forgot i had a local change that doesn't clear the buffer
not sure when windows stopped clearing it, but my windows 10 box doesn't. makes a measurable difference in some cases i can submit that separately if no objections. but if/when that's merged, msxml will randomly return garbage without the above fix -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6100#note_78922
This merge request was approved by Nikolay Sivov. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6100
On Fri Aug 16 04:28:11 2024 +0000, Daniel Lehman wrote:
ah... it does. forgot i had a local change that doesn't clear the buffer not sure when windows stopped clearing it, but my windows 10 box doesn't. makes a measurable difference in some cases i can submit that separately if no objections. but if/when that's merged, msxml will randomly return garbage without the above fix Do you mean that SysAllocStringLen() changed recently on Windows? If so, I don't think we need such disturbance in wine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6100#note_78937
Alexandre Julliard (@julliard) commented about dlls/msxml3/node.c:
static const WCHAR colW = ':'; WCHAR *ptr;
- /* +1 for ':' */ - ptr = *name = SysAllocStringLen(NULL, SysStringLen(base) + SysStringLen(prefix) + 1); + /* +1 for possible ':', +1 for \0 */ + ptr = *name = SysAllocStringLen(NULL, SysStringLen(base) + SysStringLen(prefix) + 1 + 1);
This shouldn't be necessary. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6100#note_79015
This merge request was closed by Daniel Lehman. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6100
participants (5)
-
Alexandre Julliard (＠julliard) -
Daniel Lehman
-
Daniel Lehman (＠dlehman25)
-
Marvin -
Nikolay Sivov (＠nsivov)