Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sensor and data stream modules do not check payload data validity #716

Open
mstenta opened this issue Sep 4, 2023 · 1 comment
Open
Labels

Comments

@mstenta
Copy link
Member

mstenta commented Sep 4, 2023

Describe the bug

Saw a few errors on one of the instances I host. Seems like invalid data was being sent to a sensor, and farmOS code doesn't check for that. These are the two specific errors I saw, one in the farm_sensor module and one in the data_stream module:

TypeError: Drupal\farm_sensor\Controller\SensorDataController::getUniqueNamedValues(): Argument #1 ($data) must be of type array, null given, called in /opt/drupal/web/profiles/farm/modules/asset/sensor/src/Controller/SensorDataController.php on line 138 in Drupal\farm_sensor\Controller\SensorDataController->getUniqueNamedValues() (line 186 of /opt/drupal/web/profiles/farm/modules/asset/sensor/src/Controller/SensorDataController.php) 
TypeError: Drupal\data_stream\Plugin\DataStream\DataStreamType\Basic::storageSave(): Argument #2 ($data) must be of type array, null given, called in /opt/drupal/web/profiles/farm/modules/core/data_stream/src/Plugin/DataStream/DataStreamType/Basic.php on line 320 in Drupal\data_stream\Plugin\DataStream\DataStreamType\Basic->storageSave() (line 422 of /opt/drupal/web/profiles/farm/modules/core/data_stream/src/Plugin/DataStream/DataStreamType/Basic.php) 

To Reproduce

TBD

Expected behavior

farmOS should return a 422 Unprocessable Content response.

@mstenta mstenta added the bug label Sep 4, 2023
@mstenta
Copy link
Member Author

mstenta commented Sep 4, 2023

I think we can fix this particular issue by simply adding the following to SensorDataController.php right after the line where the data is loaded:

$data = Json::decode($request->getContent());

        // Bail if data is invalid.
        if (is_null($data)) {
          throw new UnprocessableEntityHttpException();
        }

But this might not be the only place we need to address this. IIRC there are endpoints for posting directly to data streams too, which are different than this one, which is an endpoint for the sensor asset itself.

I have not tested any of this myself, so it's just conjecture. We should also add automated test(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

1 participant