sethmlarson / virtualbox-python

Complete implementation of VirtualBox's COM API with a Pythonic interface.
https://pypi.org/project/virtualbox
Apache License 2.0
354 stars 75 forks source link

gs.execute with stdin does not work properly #8

Closed maxfragg closed 7 years ago

maxfragg commented 10 years ago

/usr/local/lib/python2.7/dist-packages/pyvbox-0.1->py2.7.egg/virtualbox/library_ext/guest_session.pyc in execute(self, command, >arguments, stdin, environment, flags, priority, affinity, timeout_ms) 59 while index < len(stdin): 60 index += process.write(0, [library.ProcessInputFlag.none], ---> 61 stdin[index:], 0) 62 process.write(0, [library.ProcessInputFlag.end_of_file], 0) 63

/usr/local/lib/python2.7/dist-packages/pyvbox-0.1-py2.7.egg/virtualbox/library.pyc in >write(self, handle, flags, data, timeout_ms) 13748 raise TypeError("handle can only be an instance of type baseinteger") 13749 if not isinstance(flags, baseinteger): 13750 raise TypeError("flags can only be an instance of type baseinteger") 13751 if not isinstance(data, list): 13752 raise TypeError("data can only be an instance of type list")

TypeError: flags can only be an instance of type baseinteger

i get this error, when ever trying to use execute with input.

calling process.write() manualy with 0 instead of [library.ProcessInputFlag.none] seams to work, but then i get a type error for the stdin, no matter what i put there, it really should be a string, right? I'm not really sure, but it looks like the error checks in the process.write() don't work properly. And the following process.write(0, [library.ProcessInputFlag.end_of_file], 0) will probably fail as well, since write takes exactly 5 arguments and omitting stdin will probably crash as well.

maxfragg commented 10 years ago

just to be sure, i use virtualbox 4.3.6, and run the git version fo pyvbox, all running on ubuntu 13.10

mjdorma commented 10 years ago

Good find... It's a bug in the xidl defining IProcess.write.

I'll build an extension to IProcess such that write will check for ProcessInputFlag.

Note: this is where the bug comes from: AMENDED: not 100% correct... Bug is a change in API

>>>>
        in flags of type int
            A combination of <link to="ProcessInputFlag"/> flags.
<<<<

Where the xidl should have (IProcess.write_array for reference):

>>>>
        in flags of type ProcessInputFlag
            A combination of <link to="ProcessInputFlag"/> flags.
<<<<
maxfragg commented 10 years ago

ah, so i understand, this bug comes form automatic generated code, which is based on the virtualbox com api? And should not

if not isinstance(data, list): raise TypeError("data can only be an instance of type list")

be

if not isinstance(data, str): raise TypeError("data can only be an instance of type str")

The docstring at least claims, that data should be a string ;-) Is it save, if i just try to fix those things for the moment in the autogenerated code, or would you sugguest, to wait for a propper fix form your side?

mjdorma commented 10 years ago

Yeah that's where it comes from. I just had a good look into this issue, it looks like the interfaces have been updated... I couldn't see a quick way of fixing it (big shame because I need to fix it).

Manually prodding the interface yielded fail as I was getting 0 bytes back when I wrote to the process (maybe I need to electrify my prodder...).

mjdorma commented 10 years ago

Links to this issue: https://www.virtualbox.org/pipermail/vbox-dev/2013-June/011556.html

maxfragg commented 10 years ago

thanks for the info, could you give me a hint, is the IProcess.write_array working as expected, and if so, how can i use it, to send input? i did not have any success, besides it returning the number of entries in the array. I understand it correctly, that the data-array should contain ascii codes for input data? My alternative would be using the session.console.keyboard.put_keys function, but that is racy and does not seam to be a good solution.

thx for any help in advance

mjdorma commented 10 years ago

If we can figure out how to manually drive vboxapi such that you can satisfy the IProcess.write(...) parameters and get a valid bytes written count return that will solve this issue.

Example:

gs = s.console.guest.create_session ... 
# Trying to do something like: gs.execute(r"C:\MinGW\msys\1.0\bin\grep.exe", ["z"], stdin="fooz")

p = gs.process_create_ex(r"C:\MinGW\msys\1.0\bin\grep.exe", ["z"], [], flags = [library.ProcessCreateFlag.wait_for_std_err, library.ProcessCreateFlag.wait_for_std_out, library.ProcessCreateFlag.ignore_orphaned_processes], priority=library.ProcessPriority.default, affinity=[], timeout_ms=0)

# You can invoke directly to the vboxapi (avoiding the checks).
# The following no longer works and raises an error saying it wants a sequence:
written = p._call("write", in_p=[0, 0, "foo", 0])
VBoxError: 0x-1 (Objects for SAFEARRAYS must be sequences (of sequences), or a buffer object.)

# Trying the following raises saying it was a base 10, which is strange because it is supposed to be (and used to be) a byte array:
written = p._call("write", in_p=[0, 0, "foo", 0])
VBoxError: 0x-1 (invalid literal for int() with base 10: 'f')

# Maybe you could get a win with write_array:
# process.write_array == p._call("writeArray", in_p=[ ....  etc etc

I have to leave this for a few days now. It would be great if you find a solution in the interim.

Cheers

mjdorma commented 10 years ago

Oh, you got write_array working? Did you get the number of bytes written to the process?

Pass me the code and I can update library_ext/guest_session.py

maxfragg commented 10 years ago

no i did not, calling it like that: process.write_array(0,[virtualbox.library.ProcessInputFlag.none],['64','0'],0) will return 2, but nothing will reach the process in the VM, at least not for the linux-vm I used. And no hurry with it, I can live without it for now, with enough waiting the raw keyboard input does the job i need, but its really not the best solution, as you might understand ;-)

mjdorma commented 10 years ago

For sure. The execute function supported stdin for good reason... This bug is the result of changes in the vbox API. For now I will just keep this bug opened. If you or anyone notices that the API is behaving correctly again (or find a solution) feel free to post me a hint.

Cheers

sethmlarson commented 7 years ago

I believe the API is working again so this issue can be closed.

Smarre commented 4 years ago

I do see this issue still, though...

password = "foo"
self.guest_session.execute("/usr/bin/passwd", arguments=["build"], stdin=password)

This fails with TypeError: data can only be an instance of type list, caused by execute() to pass a map instead of list to write_array() as data argument.

mjdorma commented 4 years ago

I do see this issue still, though...

password = "foo"
self.guest_session.execute("/usr/bin/passwd", arguments=["build"], stdin=password)

This fails with TypeError: data can only be an instance of type list, caused by execute() to pass a map instead of list to write_array() as data argument.

Try adding the password to a list. [password] ?

Assume your issue is @ https://github.com/sethmlarson/virtualbox-python/blob/d48da8a828fab8bafa425d8d859249ad7d752a50/virtualbox/library_ext/guest_session.py#L80

Process write_array : https://pythonhosted.org/pyvbox/virtualbox/library.html#virtualbox.library.IGuestProcess.write_array

... finally check the code gen for this interface

if not isinstance(data, list):
    raise TypeError("data can only be an instance of type list")

for a in data[:10]:
    if not isinstance(a, basestring):
        raise TypeError("array can only contain objects of ....

Failing that, you could test adding an override to the code generate interface for write_array under the guest process extension module? Force it to accept the string.

Smarre commented 4 years ago

Try adding the password to a list. [password] ?

Still gives the same error.

Failing that, you could test adding an override to the code generate interface for write_array under the guest process extension module? Force it to accept the string.

Well, forcing it to accept a string won’t help, since it won’t get a string, but a map. The execute() convenience method converts the input string to a map of unicode characters, where write() (and I suppose write_array(), though I’m not sure what’s the actual difference between the two) expects a list of unicode characters (which is what Virtualbox wants).

My guess is that changing the map to utilize a list would make this work better. I converted to use process_start() and am passing arguments in with slighty modified converter:

bytearray = [str(ord(char)) for char in password]

Though honestly, I’d rather be able to pass a stream instead, so I could just write whatever I’d need to. But that probably would need some threading logic.

mjdorma commented 4 years ago

Ha, that’s what I get for using a phone to read old code. Ok, that makes sense.

So the simple win is to wrap the map up with list() in the execute helper?

array = list(map(lambda a: str(ord(a)), stdin[index:]))

Smarre commented 4 years ago

Python is not my forte (using it for a work project out of necessity), so I can’t comment on what would be a good way to solve it. I used my version because it was simplest one I was able to figure out.