Re: [2/2] ntdll: rtlstr.c: Implement checking for control characters in RtlIsTextUnicode [try 7]

[Zac Brown]

Zac Brown wrote:
> Add checking for control characters in both standard and byte-reversed 
> forms to (Rtl)IsTextUnicode.
>
> Changelog:
> * Add if-statement to ensure we only found control characters in the 
> first 256 indices. If we found them beyond that, we don't care since 
> Windows doesn't either.
>
> ------------------------------------------------------------------------
>
> ---
>  dlls/ntdll/rtlstr.c       |   23 +++++++++++++++++++++++
>  dlls/ntdll/tests/rtlstr.c |   21 +++++----------------
>  2 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/dlls/ntdll/rtlstr.c b/dlls/ntdll/rtlstr.c
> index deec931..802782f 100644
> --- a/dlls/ntdll/rtlstr.c
> +++ b/dlls/ntdll/rtlstr.c
> @@ -1591,7 +1591,10 @@ NTSTATUS WINAPI RtlFindCharInUnicodeString(
>   */
>  BOOLEAN WINAPI RtlIsTextUnicode( LPCVOID buf, INT len, INT *pf )
>  {
> +    static const WCHAR std_control_chars[] = {'\r','\n','\t',' ',0x3000,0};
> +    static const WCHAR byterev_control_chars[] = {0x0d00,0x0a00,0x0900,0x2000,0x0030,0};
>      const WCHAR *s = buf;
> +    WCHAR *tmp_ptr;
>      int i;
>      unsigned int flags = ~0U, out_flags = 0;
>
> @@ -1650,6 +1653,26 @@ BOOLEAN WINAPI RtlIsTextUnicode( LPCVOID buf, INT len, INT *pf )
>          }
>      }
>
> +    /* These two tests are not mutually exclusive so regardless of whether */
> +    /* the string has the BOM for byte-reversed order, it still checks */
> +    /* the string if the flag is set. */
> +
> +    /* Check for standard Unicode control characters. */
> +    if (flags & IS_TEXT_UNICODE_CONTROLS)
> +    {
> +        tmp_ptr = strpbrkW(s, std_control_chars);
> +        if (tmp_ptr != NULL && (tmp_ptr - s) < len)
> +            out_flags |= IS_TEXT_UNICODE_CONTROLS;
> +    }
> +
> +    /* Check for byte-reversed Unicode control characters. */
> +    if (flags & IS_TEXT_UNICODE_REVERSE_CONTROLS && strpbrkW(s, byterev_control_chars) != NULL)
> +    {
> +        tmp_ptr = strpbrkW(s, byterev_control_chars);
> +        if (tmp_ptr != NULL && (tmp_ptr - s) < len)
> +            out_flags |= IS_TEXT_UNICODE_REVERSE_CONTROLS;
> +    }
> +
>      if (pf)
>      {
>          out_flags &= *pf;
> diff --git a/dlls/ntdll/tests/rtlstr.c b/dlls/ntdll/tests/rtlstr.c
> index 74ba8ea..d5d8839 100644
> --- a/dlls/ntdll/tests/rtlstr.c
> +++ b/dlls/ntdll/tests/rtlstr.c
> @@ -1693,7 +1693,6 @@ static void test_RtlIsTextUnicode(void)
>
> flags =  IS_TEXT_UNICODE_UNICODE_MASK;
>      ok(pRtlIsTextUnicode(unicode, sizeof(unicode), &flags), "Text should not pass a Unicode\n");
> -    todo_wine
>      ok(flags == (IS_TEXT_UNICODE_STATISTICS | IS_TEXT_UNICODE_CONTROLS),
>         "Expected flags 0x6, obtained %x\n", flags);
>
> @@ -1712,7 +1711,7 @@ static void test_RtlIsTextUnicode(void)
>          be_unicode[i + 1] = (unicode[i] >> 8) | ((unicode[i] & 0xff) << 8);
>      }
>      ok(!pRtlIsTextUnicode(be_unicode, sizeof(unicode) + 2, NULL), "Reverse endian should not be Unicode\n");
> -    todo_wine ok(!pRtlIsTextUnicode(&be_unicode[1], sizeof(unicode), NULL), "Reverse endian should not be Unicode\n");
> +    ok(!pRtlIsTextUnicode(&be_unicode[1], sizeof(unicode), NULL), "Reverse endian should not be Unicode\n");
>
> flags = IS_TEXT_UNICODE_REVERSE_MASK;
>      ok(!pRtlIsTextUnicode(&be_unicode[1], sizeof(unicode), &flags), "Reverse endian should be Unicode\n");
> @@ -1722,7 +1721,6 @@ static void test_RtlIsTextUnicode(void)
>
> flags = IS_TEXT_UNICODE_REVERSE_MASK;
>      ok(!pRtlIsTextUnicode(be_unicode, sizeof(unicode) + 2, &flags), "Reverse endian should be Unicode\n");
> -    todo_wine
>      ok(flags == (IS_TEXT_UNICODE_REVERSE_CONTROLS | IS_TEXT_UNICODE_REVERSE_SIGNATURE),
>         "Expected flags 0xc0, obtained %x\n", flags);
>
> @@ -1750,11 +1748,8 @@ static void test_RtlIsTextUnicode(void)
>      ok(flags == 0, "Expected flags 0x0, obtained %x\n", flags);
>
> flags = IS_TEXT_UNICODE_CONTROLS;
> -    todo_wine
> -    {
> -        ok(pRtlIsTextUnicode(unicode, sizeof(unicode), &flags), "Test should pass on Unicode string lacking control characters.\n");
> -        ok(flags == IS_TEXT_UNICODE_CONTROLS, "Expected flags 0x04, obtained %x\n", flags);
> -    }
> +    ok(pRtlIsTextUnicode(unicode, sizeof(unicode), &flags), "Test should pass on Unicode string lacking control characters.\n");
> +    ok(flags == IS_TEXT_UNICODE_CONTROLS, "Expected flags 0x04, obtained %x\n", flags);
>
> flags = IS_TEXT_UNICODE_CONTROLS;
>      ok(!pRtlIsTextUnicode(be_unicode_no_controls, sizeof(unicode_no_controls) + 2, &flags),
> @@ -1762,11 +1757,8 @@ static void test_RtlIsTextUnicode(void)
>      ok(flags == 0, "Expected flags 0x0, obtained %x\n", flags);
>
> flags = IS_TEXT_UNICODE_CONTROLS;
> -    todo_wine
> -    {
> -        ok(pRtlIsTextUnicode(mixed_controls, sizeof(mixed_controls), &flags), "Test should pass on a string containing control characters.\n");
> -        ok(flags == IS_TEXT_UNICODE_CONTROLS, "Expected flags 0x04, obtained %x\n", flags);
> -    }
> +    ok(pRtlIsTextUnicode(mixed_controls, sizeof(mixed_controls), &flags), "Test should pass on a string containing control characters.\n");
> +    ok(flags == IS_TEXT_UNICODE_CONTROLS, "Expected flags 0x04, obtained %x\n", flags);
>
> /* Test IS_TEXT_UNICODE_REVERSE_CONTROLS flag */
>      flags = IS_TEXT_UNICODE_REVERSE_CONTROLS;
> @@ -1784,18 +1776,15 @@ static void test_RtlIsTextUnicode(void)
>      flags = IS_TEXT_UNICODE_REVERSE_CONTROLS;
>      ok(!pRtlIsTextUnicode(be_unicode, sizeof(unicode) + 2, &flags),
>          "Test should pass with byte-reversed Unicode string containing control characters.\n");
> -    todo_wine
>      ok(flags == IS_TEXT_UNICODE_REVERSE_CONTROLS, "Expected flags 0x40, obtained %x\n", flags);
>
> flags = IS_TEXT_UNICODE_REVERSE_CONTROLS;
>      ok(!pRtlIsTextUnicode(mixed_controls, sizeof(mixed_controls), &flags), "Test should pass on a string containing byte-reversed control characters.\n");
> -    todo_wine
>      ok(flags == IS_TEXT_UNICODE_REVERSE_CONTROLS, "Expected flags 0x40, obtained %x\n", flags);
>
> /* Test with flags for both byte-reverse and standard Unicode characters */
>      flags = IS_TEXT_UNICODE_CONTROLS | IS_TEXT_UNICODE_REVERSE_CONTROLS;
>      ok(!pRtlIsTextUnicode(mixed_controls, sizeof(mixed_controls), &flags), "Test should pass on string containing both byte-reversed and standard control characters.\n");
> -    todo_wine
>      ok(flags == (IS_TEXT_UNICODE_CONTROLS | IS_TEXT_UNICODE_REVERSE_CONTROLS), "Expected flags 0x44, obtained %x\n", flags);
>
> HeapFree(GetProcessHeap(), 0, be_unicode);
>
> ------------------------------------------------------------------------

Ignore this patch please.