There's a slight issue with the following code logic, which results in the module creating
if ($response->getStatusCode() !== 200) {
$this->logger->error('Invalid connection with client site');
}
else {
$json = $response->getBody()->getContents();
$values = json_decode($json, TRUE);
// ...
Enable
https://www.drupal.org/project/require_login →
, but don't have the /status_dashboard/check route whitelisted.
It results in a 302 redirect into a 200 HTML response.
And results in lots of empty client entries because $values['url'] and $values['sitename'] would be unset and thus returning NULL.
Add appropriate checks on the $value returned and as well as checking that the HTTP header response being valid JSON.
e.g. with something along the lines of:
// Using stripos to account for response headers with the charset directive set, such as "application/json; charset=utf-8"
if ($status !== 200 || 0 !== stripos($res->getHeaderLine('Content-Type') ?? '', 'application/json')) {
\Drupal::logger('status_dashboard')
->error('Invalid connection with client site');
}
else {
$json = $res->getBody()->getContents();
$response = json_decode($json, TRUE);
$values = (array) $response;
if (empty($values['sitename']) || empty($values['url'])) {
\Drupal::logger('status_dashboard')
->error('Could not retrieve the site name or url from the client site');
continue;
}
We'd also need an hook to delete the erroneous entries in the database. I was able to manually delete them using the following drush command:
drush ev '
$client_side_storage = \Drupal::entityTypeManager()->getStorage("client_site");
$client_ids = $client_side_storage->getQuery()->condition("sitename", NULL, "IS NULL")->condition("url", NULL, "IS NULL")->execute();
echo sprintf("Found %d client(s)", count($client_ids));
$client_entities = $client_side_storage->loadMultiple($client_ids); $client_side_storage->delete($client_entities);
echo sprintf("Deleted client ids: %s", implode(",", $client_ids));
'
Provide issue fork/patch.
N/A
N/A
N/A
Active
2.0
Code
Thanks for your very detailed report! I think your proposed resolution is great. And I think your drush ev command can be put in an update hook for the most part.
It would be great if you (or someone else) could provide a merge request or patch :-) more than happy to put this in.