Skip to content
This repository has been archived by the owner on Dec 13, 2022. It is now read-only.

fix(conf): Sanitize contact group names from illegal characters (#11480) #11689

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix(conf): Sanitize contact group names from illegal characters (#11480) #11689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion www/class/config-generate-remote/ContactGroup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class ContactGroup extends AbstractObject
protected $stmtContact = null;
protected $stmtCgService = null;

protected $illegalCharsTab = array();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected $illegalCharsTab = array();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no need of this property.
The logic of sinitazing could be put in a method.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no need of this property. The logic of sinitazing could be put in a method.

I added this property to have a cache of the SQL request, in order to avoid querying the database for each name to sanitize (especially since this process is not on the input of a unique value, but when the entire engine configuration is generated).


/**
* Generate contact group cache
*
Expand Down Expand Up @@ -184,7 +186,34 @@ public function getContactFromCgId(int $cgId)
}

/**
* Generate contact group and get contact group name
* Check illegal char defined into nagios.cfg file
*
* @param string $name The string to sanitize
* @return string The string sanitized
*/
protected function checkIllegalChar($name)
{
$pollerId = $this->backendInstance->getPollerId();

if (! isset($this->illegalCharsTab[ $pollerId ])) {
$query = "SELECT illegal_object_name_chars FROM cfg_nagios WHERE nagios_server_id = :pollerId";
$stmt = $this->backendInstance->db->prepare($query);
$stmt->bindParam(':pollerId', $pollerId, PDO::PARAM_INT);
$stmt->execute();

$illegalCharsStr = implode('', $stmt->fetchAll(PDO::FETCH_COLUMN));
$illegalCharsStr = html_entity_decode($illegalCharsStr, ENT_QUOTES, "UTF-8");
$this->illegalCharsTab[ $pollerId ] = array_unique(str_split($illegalCharsStr));
}

if (! empty($this->illegalCharsTab[ $pollerId ])) {
$name = str_replace($this->illegalCharsTab[ $pollerId ], '_', $name);
}
return $name;
}

/**
* Generate contact group and get contact group name (sanitized)
Comment on lines +194 to +216
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected function checkIllegalChar($name)
{
$pollerId = $this->backendInstance->getPollerId();
if (! isset($this->illegalCharsTab[ $pollerId ])) {
$query = "SELECT illegal_object_name_chars FROM cfg_nagios WHERE nagios_server_id = :pollerId";
$stmt = $this->backendInstance->db->prepare($query);
$stmt->bindParam(':pollerId', $pollerId, PDO::PARAM_INT);
$stmt->execute();
$illegalCharsStr = implode('', $stmt->fetchAll(PDO::FETCH_COLUMN));
$illegalCharsStr = html_entity_decode($illegalCharsStr, ENT_QUOTES, "UTF-8");
$this->illegalCharsTab[ $pollerId ] = array_unique(str_split($illegalCharsStr));
}
if (! empty($this->illegalCharsTab[ $pollerId ])) {
$name = str_replace($this->illegalCharsTab[ $pollerId ], '_', $name);
}
return $name;
}
/**
* Generate contact group and get contact group name (sanitized)
private function sanitizeNameFromIllegalChars(string $name): string
{
$pollerId = $this->backendInstance->getPollerId();
$illegalChars = $this->getIllegalCharsByNagiosServerId($pollerId);
if (empty($illegalChars)) {
return $name;
}
return str_replace($illegalChars, '_', $name);
}
private function getIllegalCharsByNagiosServerId(int $nagiosServerId): array
{
$query = 'SELECT illegal_object_name_chars FROM cfg_nagios WHERE nagios_server_id = :nagiosServerId';
$stmt = $this->backendInstance->db->prepare($query);
$stmt->bindParam(':nagiosServerId', $nagiosServerId, PDO::PARAM_INT);
$stmt->execute();
$illegalCharsStr = implode('', $stmt->fetchAll(PDO::FETCH_COLUMN));
$illegalCharsStr = html_entity_decode($illegalCharsStr, ENT_QUOTES, 'UTF-8');
return array_unique(str_split($illegalCharsStr));
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name checkIllegalChar doesn't describe what the method is actually doing. This method retreieves illegal chars and sanitizes the string and returns them.

I propose to split it in 2 methods : a get method (from database) and a sinitize method. Then the sinitize method calls get method.

I'd make them private. no need of protected.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name checkIllegalChar doesn't describe what the method is actually doing. This method retreieves illegal chars and sanitizes the string and returns them.

I propose to split it in 2 methods : a get method (from database) and a sinitize method. Then the sinitize method calls get method.

I'd make them private. no need of protected.

I'm agree with you...
If I choose a unique function named "checkIllegalChar", it's only because it's already like that in other existing files:

  • www/class/centreon.class.php:291
  • www/class/centreonHost.class.php:618
  • www/class/centreonService.class.php:307
  • www/class/centreon-clapi/centreonObject.class.php:638

So I tried to stay consistent with the existing code, I don't know which is preferable.

*
* @param null|integer $cgId
* @return void|string
Expand Down Expand Up @@ -213,6 +242,8 @@ public function generateFromCgId(?int $cgId)
return $this->cg[$cgId]['cg_name'];
}

$this->cg[$cgId]['cg_name'] = $this->checkIllegalChar($this->cg[$cgId]['cg_name']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->cg[$cgId]['cg_name'] = $this->checkIllegalChar($this->cg[$cgId]['cg_name']);
$this->sanitizeNameFromIllegalChars($this->cg[$cgId]['cg_name']);


$this->getContactFromCgId($cgId);

$this->cg[$cgId]['cg_id'] = $cgId;
Expand Down
34 changes: 34 additions & 0 deletions www/class/config-generate/contactgroup.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ class Contactgroup extends AbstractObject
protected $stmt_contact = null;
protected $stmt_cg_service = null;

protected $illegalCharsTab = array();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments here


protected function getCgCache()
{
$stmt = $this->backend_instance->db->prepare("SELECT
Expand Down Expand Up @@ -187,6 +189,36 @@ public function getContactFromCgId(int $cgId): void
}

/**
* Check illegal char defined into nagios.cfg file
*
* @param string $name The string to sanitize
* @return string The string sanitized
*/
protected function checkIllegalChar($name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here

{
$pollerId = $this->backend_instance->getPollerId();

if (! isset($this->illegalCharsTab[ $pollerId ])) {
$query = "SELECT illegal_object_name_chars FROM cfg_nagios WHERE nagios_server_id = :pollerId";
$stmt = $this->backend_instance->db->prepare($query);
$stmt->bindParam(':pollerId', $pollerId, PDO::PARAM_INT);
$stmt->execute();

$illegalCharsStr = implode('', $stmt->fetchAll(PDO::FETCH_COLUMN));
$illegalCharsStr = html_entity_decode($illegalCharsStr, ENT_QUOTES, "UTF-8");
$this->illegalCharsTab[ $pollerId ] = array_unique(str_split($illegalCharsStr));
$stmt->closeCursor();
}

if (! empty($this->illegalCharsTab[ $pollerId ])) {
$name = str_replace($this->illegalCharsTab[ $pollerId ], '_', $name);
}
return $name;
}

/**
* Generate contact group and get contact group name (sanitized)
*
* @param int $cgId
* @return string|null contactgroup_name
*/
Expand Down Expand Up @@ -214,6 +246,8 @@ public function generateFromCgId(int $cgId): ?string
return $this->cg[$cgId]['contactgroup_name'];
}

$this->cg[$cgId]['contactgroup_name'] = $this->checkIllegalChar($this->cg[$cgId]['contactgroup_name']);

$this->getContactFromCgId($cgId);

$this->generateObjectInFile($this->cg[$cgId], $cgId);
Expand Down