Calling update() may result in data loss

[dunglas]

I discovered a potentially serious flaw while working on #309.

According to the API documentation, PUT has the following behavior for some types (load balancers, firewalls, and maybe others)

The request should contain a full representation of the load balancer including existing attributes. [...] Note that any attribute that is not provided will be reset to its default value.

The structure of APIs changes from time to time and new fields are added. These fields are not always (or immediately) supported by this library. This means that when calling update(), existing data (set through the UI for instance) may be deleted and replaced by the default values.

Example of dangerous broken code:

$lbApi = (new DigitalOceanV2\Client())->loadBalancer();

foreach ($lbApi->getAll() as $lb) {
    $lb->redirectHttpToHttps = true;
    $lbApi->update($lb->id, $lb);
}

The size_unit property is reset to its default value because it's not supported yet by this library.

A safer approach would be to always store all retrieved properties through GET (even if currently unsupported by the library) in the entity (by creating dynamic properties, or storing them in a dedicated prop AbstractEntity::$unsupportedProperty), and include them in the JSON payload sent during a PUT.

[GrahamCampbell]

Thanks for letting us know. I'd review a PR to implement this, but I don't have capacity to implement this at this time. Consider sponsoring me, which would allow me to allocate more time to OSS. I am spread across many OSS repos, including Guzzle.

[stale[bot]]

This issue has been automatically marked as stale because there has been no recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions.