N°5547 - Object deletion fails if friendlyname too long (#529)

Combodo · Feb 27, 2024 · 86bf6ba · 86bf6ba
1 parent 82e19f6
commit 86bf6ba
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 7 deletions.
diff --git a/core/cmdbobject.class.inc.php b/core/cmdbobject.class.inc.php
@@ -335,7 +335,7 @@ protected function RecordObjDeletion($objkey)
 		$oMyChangeOp->Set("objclass", MetaModel::GetRootClass(get_class($this)));
 		$oMyChangeOp->Set("objkey", $objkey);
 		$oMyChangeOp->Set("fclass", get_class($this));
-		$oMyChangeOp->Set("fname", substr($this->GetRawName(), 0, 255)); // Protect against very long friendly names
+		$oMyChangeOp->SetTrim("fname", $this->GetRawName()); // Protect against very long friendly names
 		$iId = $oMyChangeOp->DBInsertNoReload();
 	}
 

diff --git a/tests/php-unit-tests/unitary-tests/core/CMDBObjectTest.php b/tests/php-unit-tests/unitary-tests/core/CMDBObjectTest.php
@@ -5,9 +5,11 @@
 
 use CMDBObject;
 use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use CoreException;
 use Exception;
 use MetaModel;
 
+
 /**
  * @since 2.7.7 3.0.2 3.1.0 N°3717 tests history objects creation
  *
@@ -168,6 +170,53 @@ public function testCurrentChangeUnderImpersonation($sTrackInfo=null, $sExpected
 		CMDBObject::SetTrackInfo($sInitialTrackInfo);
 	}
 
+	/**
+	 * Data provider for test deletion
+	 *  N°5547 - Object deletion fails if friendlyname too long
+	 *
+	 * @return array data
+	 */
+	public function RecordObjDeletionProvider()
+	{
+		return [
+			'friendlyname longer than 255 characters which will be truncated on a multi-bytes characters' => [
+				str_repeat('e', 250),
+				'😁😂🤣😃😄😅😆😗🥰😘😍😎😋😊😉😙😚',
+			],
+			'friendlyname longer than 255 characters which will be truncated after a single byte characters' => [
+				'😁😂🤣😃😄😅😆😗🥰😘😍😎😋😊😉😙😚',
+				str_repeat('e', 250),
+			],
+		];
+	}
+
+	/**
+	 * N°5547 - Object deletion fails if friendlyname too long
+	 *
+	 * @dataProvider RecordObjDeletionProvider
+	 *
+	 */
+	public function testRecordObjDeletion( string $sFirstName, string $sName)
+	{
+		$oPerson = MetaModel::NewObject('Person', [
+			'first_name' => $sFirstName,
+			'name'       => $sName,
+			'org_id'     => 1,
+		]);
+		$oPerson->DBWrite();
+
+		$bDeletionOK = true;
+		try {
+			$oDeletionPlan = $this->InvokeNonPublicMethod(CMDBObject::class, 'RecordObjDeletion', $oPerson, [$oPerson->GetKey()]);
+		}
+		catch (CoreException $e) {
+			$bDeletionOK = false;
+		}
+		// We don't need to test the result (truncated string), it's already done in \DBObject::SetTrim() with N°3448
+		$this->assertTrue($bDeletionOK);
+	}
+
+
 	private function ReplaceByFriendlyNames($sMessage, $oAdminUser, $oImpersonatedUser) : string {
 		$sNewMessage = str_replace('AdminSurName AdminName', $oAdminUser->GetFriendlyName(), $sMessage);
 		$sNewMessage = str_replace('ImpersonatedSurName ImpersonatedName', $oImpersonatedUser->GetFriendlyName(), $sNewMessage);

diff --git a/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php b/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php
@@ -1133,6 +1133,54 @@ private function CreatePersonInstance()
 		return $oPerson;
 	}
 
+	/**
+	 * Data provider for test deletion
+	 *  N°5547 - Object deletion fails if friendlyname too long
+	 *
+	 * @return array data
+	 */
+	public function getDeletionLongValueProvider()
+	{
+		return [
+			'friendlyname longer than 255 chracters with smiley'               => [
+				'0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789-0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789-ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopq',
+				'😁😂🤣😃😄😅😆😗🥰😘😍😎😋😊😉😙😚',
+			],
+			'the same friendlyname in other order with error before fix 5547 ' => [
+				'😁😂🤣😃😄😅😆😗🥰😘😍😎😋😊😉😙😚',
+				'0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789-0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789-ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopq',
+			],
+		];
+	}
+
+	/**
+	 * N°5547 - Object deletion fails if friendlyname too long
+	 *
+	 * @covers       DBObject::DBIncrement
+	 *
+	 * @dataProvider getDeletionLongValueProvider
+	 *
+	 */
+	public function testDeletionLongValue(string $sName, string $sFirstName)
+	{
+		// Create a UserRequest with 2 contacts
+		$oPerson = MetaModel::NewObject('Person', [
+			'name'       => $sName,
+			'first_name' => $sFirstName,
+			'org_id'     => 1,
+		]);
+		$oPerson->DBWrite();
+
+		$bDeletionOK = true;
+		try {
+			$oDeletionPlan = $oPerson->DBDelete();
+		}
+		catch (CoreException $e) {
+			$bDeletionOK = false;
+		}
+		$this->assertTrue($bDeletionOK);
+	}
+
 	public function ResetReloadCount()
 	{
 		$this->aReloadCount = [];
@@ -1211,8 +1259,14 @@ public function testConstructorMemoryFootprint():void
 		$fTotalDuration = microtime(true) - $fStart;
 		echo 'Total duration: '.sprintf('%.3f s', $fTotalDuration)."\n\n";
 	}
-
-	public function CheckLongValueInAttributeProvider() {
+	/**
+	 * Data provider for test deletion
+	 *  N°5547 - Object deletion fails if friendlyname too long
+	 *
+	 * @return array data
+	 */
+	public function DeletionLongValueProvider()
+	{
 		return [
 			// UserRequest.title is an AttributeString (maxsize = 255)
 			'title 250 chars' => ['title', 250],
@@ -1234,11 +1288,9 @@ public function CheckLongValueInAttributeProvider() {
 	/**
 	 * Test check long field with non ascii characters
 	 *
-	 * @covers       DBObject::Set
-	 * @covers       DBObject::CheckToWrite
-	 * @covers       DBObject::SetTrim
+	 * @covers       DBObject::DBDelete
 	 *
-	 * @dataProvider CheckLongValueInAttributeProvider
+	 * @dataProvider DeletionLongValueProvider
 	 *
 	 * @since 3.1.2 N°3448 - Framework field size check not correctly implemented for multi-bytes languages/strings
 	 */