NdrCString packing is broken

[Couskito]

Hi!

Thanks for the awesome library, I really like it!

While testing some RPC calls i noticed that the marshalling of C strings using NdrCString.pack() seemed to break for some reason (I can only assume this is an artifact left from the Python 2 transition).

Here is a simple program that should trigger the bug (at least it did on my setup using Python 3.9.13 and Windows 11) :

from windows.rpc import ndr

print(ndr.NdrCString.pack("Hello\x00"))

This yields the following stack trace :

PS C:\Users\cousky\Desktop\testdir> python .\test.py
Traceback (most recent call last):
  File "C:\Users\cousky\Desktop\testdir\test.py", line 3, in <module>
    a = ndr.NdrCString.pack("Hello\x00")
  File "C:\Users\cousky\AppData\Local\Programs\Python\Python39\lib\site-packages\windows\rpc\ndr.py", line 195, in pack
    result += data
TypeError: can't concat str to bytes

After having a quick look at windows\rpc\ndr.py it seems that encoding data to bytes by replacing result += data with result += data.encode() on line 195 fixes the issue, I can open a PR with the change for you if you wish.

Have a nice day!

[hakril]

Hi, thank you for the issue.

Indeed, allowing string in python 3 would be nice.
I am not a fan of using data.encode() as it would mean utf-8 encoding. And I am not sure that the RPC runtime will like that.

For example, using French or Japanese would give

>>> print(ndr.NdrCString.pack("Héhé\x00"))
b'\x05\x00\x00\x00\x00\x00\x00\x00\x05\x00\x00\x00H\xc3\xa9h\xc3\xa9\x00P'
>>> print(ndr.NdrCString.pack("ハロー・ワールド"))
b'\t\x00\x00\x00\x00\x00\x00\x00\t\x00\x00\x00\xe3\x83\x8f\xe3\x83\xad\xe3\x83\xbc\xe3\x83\xbb\xe3\x83\xaf\xe3\x83\xbc\xe3\x83\xab\xe3\x83\x89\x00PPP'

That does not seem nice for an FC_C_CSTRING.

My first approach would be to enforce ascii for the sake of simplicity, as encoding may depend on the locals of the computer running the code.
I will try to test some stuff with a custom RPC server and update the issue in the next few days.

[Couskito]

Hi,

You're right, using ascii is definitely safer.

Thanks for the fix!