[PATCH 0/1] MR6100: msxml3: Ensure node name is null terminated.

[PATCH 1/1] msxml3: Ensure node name is null terminated.

[Daniel Lehman]

From: Daniel Lehman

---
 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);
     }

[PATCH 1/1] msxml3: Ensure node name is null terminated.

[Marvin]

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

[Daniel Lehman (＠dlehman25)]

@nsivov this ok to go in?  it fixes random garbage added to xml because of the missing \0

[Nikolay Sivov (＠nsivov)]

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.

[Daniel Lehman (＠dlehman25)]

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

[PATCH 0/1] MR6100: msxml3: Ensure node name is null terminated. - approved

[Nikolay Sivov (＠nsivov)]

This merge request was approved by Nikolay Sivov.

[Nikolay Sivov (＠nsivov)]

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.

[Alexandre Julliard (＠julliard)]

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.

[PATCH 0/1] MR6100: msxml3: Ensure node name is null terminated. - closed

[Daniel Lehman (＠dlehman25)]

This merge request was closed by Daniel Lehman.