N°4244 - Add protection against unfortunate massive delete of inline images / attachments when a null temp ID is passed

core/inlineimage.class.inc.php

@@ -229,7 +229,7 @@ class InlineImage extends DBObject
 	 *
 	 * @param string $sTempId
 	 *
-	 * @return void
+	 * @return bool True if cleaning was successful, false if anything aborted it
 	 * @throws \ArchivedObjectException
 	 * @throws \CoreCannotSaveObjectException
 	 * @throws \CoreException
@@ -239,8 +239,19 @@ class InlineImage extends DBObject
 	 * @throws \MySQLHasGoneAwayException
 	 * @throws \OQLException
 	 */
-	public static function OnFormCancel($sTempId)
+	public static function OnFormCancel($sTempId): bool
 	{
+		// Protection against unfortunate massive delete of inline images when a null temp ID is passed
+		if (strlen($sTempId) === 0) {
+			IssueLog::Trace('OnFormCancel "error" $sTempId is null or empty', LogChannels::INLINE_IMAGE, array(
+				'$sTempId' => $sTempId,
+				'$sUser' => UserRights::GetUser(),
+				'HTTP_REFERER' => @$_SERVER['HTTP_REFERER'],
+			));
+
+			return false;
+		}
+
 		// Delete all "pending" InlineImages for this form
 		$sOQL = 'SELECT InlineImage WHERE temp_id = :temp_id';
 		$oSearch = DBObjectSearch::FromOQL($sOQL);
@@ -257,6 +268,8 @@ class InlineImage extends DBObject
 			'$sUser' => UserRights::GetUser(),
 			'HTTP_REFERER' => @$_SERVER['HTTP_REFERER'],
 		));
+
+		return true;
 	}
 
 	/**

datamodels/2.x/itop-attachments/main.itop-attachments.php

@@ -111,6 +111,11 @@ class AttachmentPlugIn implements iApplicationUIExtension, iApplicationObjectExt
 
 	public function OnFormCancel($sTempId)
 	{
+		// Protection against unfortunate massive delete of attachments when a null temp ID is passed
+		if (strlen($sTempId) === 0) {
+			return;
+		}
+
 		// Delete all "pending" attachments for this form
 		$sOQL = 'SELECT Attachment WHERE temp_id = :temp_id';
 		$oSearch = DBObjectSearch::FromOQL($sOQL);

test/core/InlineImageTest.php

@@ -0,0 +1,68 @@
+<?php
+/*
+ * @copyright   Copyright (C) 2010-2021 Combodo SARL
+ * @license     http://opensource.org/licenses/AGPL-3.0
+ */
+
+
+namespace Combodo\iTop\Test\UnitTest\Core;
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use InlineImage;
+
+
+/**
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ * @backupGlobals disabled
+ */
+class InlineImageTest extends ItopDataTestCase
+{
+	/**
+	 * @dataProvider OnFormCancelInvalidTempIdProvider
+	 *
+	 * @param $sTempId
+	 * @param bool $bExpectedReturn
+	 *
+	 * @throws \ArchivedObjectException
+	 * @throws \CoreCannotSaveObjectException
+	 * @throws \CoreException
+	 * @throws \CoreUnexpectedValue
+	 * @throws \DeleteException
+	 * @throws \MySQLException
+	 * @throws \MySQLHasGoneAwayException
+	 * @throws \OQLException
+	 * @covers       InlineImage::OnFormCancel()
+	 */
+	public function testOnFormCancelInvalidTempId($sTempId, bool $bExpectedReturn)
+	{
+		$bTestReturn = InlineImage::OnFormCancel($sTempId);
+		$this->assertEquals($bExpectedReturn, $bTestReturn);
+	}
+
+	public function OnFormCancelInvalidTempIdProvider()
+	{
+		return [
+			'Null temp_id' => [
+				null,
+				false,
+			],
+			'Empty temp_id' => [
+				'',
+				false,
+			],
+			'0 as integer temp_id' => [
+				0,
+				true,
+			],
+			'0 as string temp_id' => [
+				'0',
+				true,
+			],
+			'String temp_id' => [
+				'fake_temp_id',
+				true,
+			],
+		];
+	}
+}