Fix incorrect string length calculation #76127
Conversation
ANSI string depends on system encoding charset. Unix's implementation of Marshal.StringToHGlobalAnsi encodes in UTF8. UTF8 characters which are out of 8-bit range (otcet) encodes in multiple bytes (otcets). ASCII characters usually are in 0x00-0x7F range but cyrillic and other characters are not. For example, 'Зфыы123;' (eq. 'Pass123$') will be encoded in 12 bytes instead of 8 Fix #76125
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-directoryservices, @jay98014 Issue DetailsFixes #76125
|
| internal static IntPtr StringToPtr(string s) => Marshal.StringToHGlobalAnsi(s); | ||
|
|
||
| internal static unsafe IntPtr StringToPtr(string s, out int length) | ||
| { | ||
| var pointer = StringToPtr(s); | ||
| length = 0; | ||
|
|
||
| if (pointer == IntPtr.Zero) | ||
| { | ||
| return pointer; | ||
| } | ||
|
|
||
| byte* stringPtr = (byte*)pointer; | ||
| while (stringPtr[length] != '\0') | ||
| { | ||
| length++; | ||
| } |
There was a problem hiding this comment.
If StringToPtr is not used elsewhere, you can shorten the method to this. Either way a while loop to find the NUL is not advised because it won't be vectorized.
| internal static IntPtr StringToPtr(string s) => Marshal.StringToHGlobalAnsi(s); | |
| internal static unsafe IntPtr StringToPtr(string s, out int length) | |
| { | |
| var pointer = StringToPtr(s); | |
| length = 0; | |
| if (pointer == IntPtr.Zero) | |
| { | |
| return pointer; | |
| } | |
| byte* stringPtr = (byte*)pointer; | |
| while (stringPtr[length] != '\0') | |
| { | |
| length++; | |
| } | |
| internal static unsafe IntPtr StringToPtr(string s, out int length) | |
| { | |
| var pointer = Marshal.StringToHGlobalAnsi(s); | |
| length = MemoryMarshal.CreateReadOnlySpanFromNullTerminated(pointer).Length; |
There was a problem hiding this comment.
Thank you! I didn't know about this before. Applied this suggestion partly as a new commit.
Also, StringToPtr is used in other methods but they don't use string length. So that's why I added this overload with out int length
There was a problem hiding this comment.
Another thing I thought is to not add this new overload and instead calculate the length at the callsite where it's needed.
There was a problem hiding this comment.
Something like that? https://github.com/Sparin/runtime/commit/78484539924277e2295756616a639d1d55c14f95
Tell me if I should apply this commit to PR
There was a problem hiding this comment.
Yeah. It's up to you if you like it more.
There was a problem hiding this comment.
Done. Seems cleaner now for me than the first one :D
|
Is it possible to re-run failed checks? It seems failed due to infrastructure performance |
|
Thank you! |
ghost
locked as resolved and limited conversation to collaborators
Fixes #76125