VirtualBox

Opened 14 years ago

Closed 14 years ago

#6378 closed defect (fixed)

PHP web API return wrong value(s) when only a single machine is present on host

Reported by: Sm0k1n Owned by:
Component: webservices Version: VirtualBox 3.1.4
Keywords: php, collection, constructor Cc:
Guest type: other Host type: Linux

Description

If only one virtual machine is present on the host, some calls to the php web api like IVirtualBox->machines should return an array of machines, but since there is only one machine, only one object is returned. It seems that either there is a bug in the api itself or PHP tries to be smart and does not create an array with only one object. Either way, the constructor of the collection object requires an array and it fails to get one, therefore this error comes up:

Argument 2 passed to VBox_ManagedObjectCollection::__construct() must be an array, string given

This is also the case with IVirtualBox->DVDImage when only one dvd image is present. Having more than one machine/dvd/etc. solves the issue.

System info
----
VirtualBox: 3.1.2, 3.1.4 (bug is present in both versions)
Host: openSuse 11.2
Kernel: 2.6.31.12-0.1-default
PHP: 5.3.1
Web Server: Apache 2.0

Attachments (3)

modifiedCollectionConstructor.php (1.9 KB ) - added by Sm0k1n 14 years ago.
VBox_ManagedObjectCollection replacement constructor
vboxServiceWrappers.php (255.0 KB ) - added by James Lucas 14 years ago.
Updated PHP wrappers (small fix)
vboxServiceWrappers(old collection).php (255.7 KB ) - added by James Lucas 14 years ago.
With old style collection class

Download all attachments as: .zip

Change History (17)

comment:1 by James Lucas, 14 years ago

I have noticed this also. It appears that the issue is on the SOAP side. We discussed this on the vbox-dev mailing list http://vbox.innotek.de/pipermail/vbox-dev/2010-March/thread.html (Topic IMachine::mediumAttachments and incorrect behavior using vboxwebsrv API). I posted some raw SOAP calls http://vbox.innotek.de/pipermail/vbox-dev/2010-March/002427.html.

comment:2 by Sm0k1n, 14 years ago

I was going to post this as a separate ticket but after further investigation it seems that it is related to this one.


When I request the attachmens for a machine with a SATA controller, IMachine->mediumAttachments returns quite strange data.

IMachine->mediumAttachments->count() returns 6, IMachine->mediumAttachments->valid() evaluates to true for the first 5 objects, but the objects themselves are not valid (I get Trying to get property of non-object... when trying to get an attribute).

The machine itself seems to work just fine without any errors. The CLI shows the proper disk setup and does not complain in any way.

This error also appeared on a machine with two IDE controllers after I removed on of them and the disk on it. Before the removal, there were no errors coming out of the API (ofc. the machine would not run) and the disk setup was properly shown.

Attacing a second device to both machines resolves the issue.


comment:3 by Sm0k1n, 14 years ago

The inconsistencies I mentioned in my previous reply (from 2010-03-30 12:03:07) are coming from the original bug. Rather than getting a collection with one element the element itself is returned and it's attributes are being counted and reported as valid, etc...

Another thing I noticed is that between VirtualBox versions 3.1.2 and 3.1.6 something was changed because right now I get this issue with only two (as far as I know) interfaces: IMediumAttachmentCollection and ISharedFolderCollection.

Here is a possible workaround:

This is the modified VBox_ManagedObjectCollection constructor:

public function __construct($soap, array $handles = array())
    {
        $this->connection = $soap;
        $this->handles = $handles;

        if($this->_interfaceName != "IMediumAttachment" && $this->_interfaceName != "ISharedFolder")
            return;
        else
            ;

        foreach($this->handles as $value)
        {
            if(!is_object($value))
            {
                $this->handles = array(0 => (object)$this->handles);
                break;
            }
            else
                ;
        }
    }

The original looks like this:

 public function __construct($soap, array $handles = array())
    {
        $this->connection = $soap;
        $this->handles = $handles;
    }

In a few words, this code creates the expected one element array. See the attached file modifiedCollectionConstructor.php for more information.

This "fix" seems to work for me (VBox 3.1.6 on openSuse 11.2, 2.6.31.12-0.1-default kernel) without adverse effects on anything else.

by Sm0k1n, 14 years ago

VBox_ManagedObjectCollection replacement constructor

comment:4 by James Lucas, 14 years ago

Hi Sm0k1n,

Could you please try the updated vboxServiceWrappers.php attached to this ticket. I've split handling of struct collections to handle the return value differences between return list of type struct with one item, and a return list of type struct with multiple items. This version also includes a slightly different parent Collection class design to allow it to act more like a array.

by James Lucas, 14 years ago

Attachment: vboxServiceWrappers.php added

Updated PHP wrappers (small fix)

in reply to:  4 comment:5 by Sm0k1n, 14 years ago

Replying to mjlucas:

Hi Sm0k1n,

Could you please try the updated vboxServiceWrappers.php attached to this ticket. I've split handling of struct collections to handle the return value differences between return list of type struct with one item, and a return list of type struct with multiple items. This version also includes a slightly different parent Collection class design to allow it to act more like a array.


I tried it and it didn't work for me. You made too many changes to the collection class without adhering to the original interfaces. It could be ok for someone that is just starting to work with this, but rewriting it every time a new VirtualBox version comes out will be a pain.

comment:6 by James Lucas, 14 years ago

Could you explain what you mean by didn't work for you? Commands run, exception output? As much error information as possible will allow me to track down your issue. I have run the new wrappers through my test suite and another VBox developer has also tested on his web based management system with success. I didn't have to change any code.

The changes to the collection class should not break anything (if they did I'll fix that up). They actually expose more to PHP so you can use functions like current(), next(), list() etc on the collection instead of just foreach. Do you happen to use the "Collection::current(), next(), rewind() commands directly?"

comment:7 by Sm0k1n, 14 years ago

I don't know about the current(), next(), etc.. but valid() is not present, therefore I get the function not defined error message. And no, I don't use directly the Collection methods. I use those that are defined in the original wrappers file and those are not present in your file.

This comes from your change of the implemented classes in the VBox_Collection class. You use IteratorAggregate and the original VBox_ManagedObjectCollection uses Iterator.

comment:8 by James Lucas, 14 years ago

Ok, I've uploaded a version with the old style collection class. Please test with that. I'm curious the use case of calling valid() directly. Could you please post a short php snippet of how you use the collection. I want to improve the collection class while also keeping BC.

by James Lucas, 14 years ago

With old style collection class

comment:9 by James Lucas, 14 years ago

Note that I haven't tested this version out. Please let me know how it goes for you.

comment:10 by Sm0k1n, 14 years ago

vboxServiceWrappers(old collection).php works ok.

Here is a sample use of valid():

...
$servers = $virtualbox->DHCPServers;
while($servers->valid())
{
    $currentServer = $servers->current();

    $result = $result."Info about the current server";

    $servers->next();
}
...
print $result;
...

This is part of my networking module and it prints out information about the present dhcp servers.

Btw, have you tried the solution I came up with. It adds only a few bits of code and so far I have had no troubles with it.

comment:11 by James Lucas, 14 years ago

If you have a look at the new wrappers you will see I did a slightly modified version of your code. In the VBox_StructCollection I have overridden the constructor and wrap the soap return value in a array when there is only one value returned.

The vboxServiceWrappers here are generated from the SVN VirtualBox.xidl definition file. This is slightly different from the 3.1.6 release (no IMachine::setStatisticsUpdateInterval() in SVN is one thing I have noticed).

Another way to write your code sample (and the reason why I hadn't thought about your use case but have now fixed up to work as previous and still have the new array access functions).

...
$servers = $virtualbox->DHCPServers;
foreach($servers as $currentServer)
{
 $result = $result."Info about the current server";
}
...
print $result;
...

I will be away on leave (and away from computer) for the next 7 days. Once I return I'll package up the patch for the wrappers and submit them for inclusion.

comment:12 by Sm0k1n, 14 years ago

After a more detailed check of your wrappers file I'd have to say that I do not agree with your modificaitons. It obviously works and that makes it as good a solution as any but you added two more classes and modified a few others.

Could you explain why these changes were necessary?

comment:13 by James Lucas, 14 years ago

Fixed in 3.1.8 / 3.2

Collections of structs have been updated as they should not be treated the same as collections of managed objects

comment:14 by Sander van Leeuwen, 14 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.

© 2023 Oracle
ContactPrivacy policyTerms of Use