N°7213 - PHP 8.1: Migrate remaining usages of md5() with null value

[Molkobain]

Base information

Question	Answer
Related to a SourceForge thead / Another PR / Combodo ticket?	Combodo N°7213, mentioned in #600
Type of change?	Bug fix

Symptom (bug)

Deprecation notice in the logs

Deprecated: md5(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/html/core/ormdocument.class.inc.php on line 346

Reproduction procedure (bug)

No real use case so far, only programatically with the following delta.

1. On iTop 3.1.0
2. With PHP 8.1.0
3. Apply following delta
4. Create new User Request
5. Change urgency to trigger a ComputeValues
6. Check that notice is present either in iTop logs or the network panel of the browser

<?xml version="1.0" encoding="UTF-8"?>
<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.0">
  <classes>
    <class id="UserRequest" _delta="must_exist">
      <methods>
        <method id="ComputeValues" _delta="force">
          <static>false</static>
          <access>public</access>
          <type>Overload-DBObject</type>
          <code>
<![CDATA[
public function ComputeValues()
{
  $oLog = new ormCaseLog(null);
  $oLog->GetAsArray();
  $oLog->GetAsEmailHtml();
  $oLog->GetAsSimpleHtml();
  $oLog->GetAsHTML();

  $oDoc = new ormDocument();
  $oDoc->GetSignature();

  parent::ComputeValues();
}
]]>
          </code>
        </method>
      </methods>
    </class>
  </classes>
</itop_design>

Cause (bug)

In some cases, the $m_data property of the \ormDocument can be null and cause this. But how it can be null is not clear yet.

Proposed solution (bug and enhancement)

Protect code by using the coalesce operator (??) to use '' instead of null in the md5().

My opinion was that the 2 usages are for fingerprint / signature / cache buster, so we should return a signature even for an empty content as when the content would change, it signature / fingerprint will as well.

• \ormDocument::GetSignature():
  • Used to generate the secret so a document can be accessed via a URL without authentication
  • Also used to invalidate browser cache if the document content has changed
• AttributeBlob::Fingerprint()
  • Used to compare 2 document, so if both are empty, generating the same MD5 through an empty string will be fine

Checklist before requesting a review

• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• Would a unit test be relevant and have I added it?
• Is the PR clear and detailled enough so anyone can understand digging in the code?

Checklist of things to do before PR is ready to merge

None

[piRGoif]

\ormDocument::GetSignature is used in multiple places to build URL to the content.
\AttributeBlob::Fingerprint is used in \DBObjectSetComparator::ComputeFingerprints
So for the second use, having the same generated signature for both null and empty string values should be ok.

[Molkobain]

Accepted during support PRs review

[Molkobain]

Accepted during functional review