From 3cc3f0e4ff66b001ebae9ab9ce5575932b90b761 Mon Sep 17 00:00:00 2001 From: Molkobain Date: Tue, 27 Feb 2024 08:34:34 +0100 Subject: [PATCH 1/4] =?UTF-8?q?N=C2=B07288=20-=20Fix=20page=20crash=20due?= =?UTF-8?q?=20to=20unescaped=20characters=20in=20relations=20row=20actions?= =?UTF-8?q?=20(#620)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../base/components/datatable/row-actions/handler.js.twig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/templates/base/components/datatable/row-actions/handler.js.twig b/templates/base/components/datatable/row-actions/handler.js.twig index 38c699f8c..56c0811f9 100644 --- a/templates/base/components/datatable/row-actions/handler.js.twig +++ b/templates/base/components/datatable/row-actions/handler.js.twig @@ -23,16 +23,16 @@ {% if aAction.confirmation is defined %} // Prepare confirmation title - let sTitle = '{{ 'UI:Datatables:RowActions:ConfirmationDialog'|dict_s }}'; + let sTitle = `{{ 'UI:Datatables:RowActions:ConfirmationDialog'|dict_s|escape('js') }}`; {% if aAction.confirmation.title is defined %} - sTitle = '{{ aAction.confirmation.title|dict_s }}'; + sTitle = `{{ aAction.confirmation.title|dict_s|escape('js') }}`; {% endif %} sTitle = sTitle.replaceAll('{item}', aRowData['{{ aAction.confirmation.row_data }}']); // Prepare confirmation message - let sMessage = '{{ 'UI:Datatables:RowActions:ConfirmationMessage'|dict_s }}'; + let sMessage = `{{ 'UI:Datatables:RowActions:ConfirmationMessage'|dict_s|escape('js') }}`; {% if aAction.confirmation.message is defined %} - sMessage = '{{ aAction.confirmation.message|dict_s }}'; + sMessage = `{{ aAction.confirmation.message|dict_s|escape('js') }}`; {% endif %} sMessage = sMessage.replaceAll('{item}', aRowData['{{ aAction.confirmation.row_data }}']); From 8f1d8b1dc290659330eaf2b303fc7310003ed99a Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Tue, 27 Feb 2024 17:39:53 +0100 Subject: [PATCH 2/4] =?UTF-8?q?N=C2=B07294=20-=20EVENT=5FADD=5FATTACHMENT?= =?UTF-8?q?=5FTO=5FOBJECT=20not=20triggered=20on=20the=20object?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../datamodel.itop-attachments.xml | 14 ++-- .../main.itop-attachments.php | 9 ++- .../2.x/itop-attachments/TestAttachment.php | 72 +++++++++++++++++++ 3 files changed, 82 insertions(+), 13 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php diff --git a/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml b/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml index 5e7c9e8fd..2f21cea7a 100755 --- a/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml +++ b/datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml @@ -263,16 +263,15 @@ An attachment has been added to an object Attachment::AfterUpdate - Attachment cmdbAbstractObject - The attachment updated + The object where the attachment is added DBObject - - The object to which the attachment is linked + + The attachment added to the objet DBObject @@ -285,16 +284,15 @@ An attachment has been removed from an object Attachment::AfterUpdate - Attachment cmdbAbstractObject - The attachment updated + The object where the attachment is removed DBObject - - The object to which the attachment is linked + + The attachment removed DBObject diff --git a/datamodels/2.x/itop-attachments/main.itop-attachments.php b/datamodels/2.x/itop-attachments/main.itop-attachments.php index 6e332a9fb..764a558e4 100644 --- a/datamodels/2.x/itop-attachments/main.itop-attachments.php +++ b/datamodels/2.x/itop-attachments/main.itop-attachments.php @@ -262,7 +262,6 @@ class AttachmentPlugIn implements iApplicationUIExtension, iApplicationObjectExt else { $oAttachmentsRenderer->RenderViewAttachmentsList(); - } } @@ -291,8 +290,8 @@ class AttachmentPlugIn implements iApplicationUIExtension, iApplicationObjectExt // Remove attachments that are no longer attached to the current object if (in_array($oAttachment->GetKey(), $aRemovedAttachmentIds)) { - $aData = ['target_object' => $oObject]; - $oAttachment->FireEvent(EVENT_REMOVE_ATTACHMENT_FROM_OBJECT, $aData); + $aData = ['attachment' => $oAttachment]; + $oObject->FireEvent(EVENT_REMOVE_ATTACHMENT_FROM_OBJECT, $aData); $oAttachment->DBDelete(); $aActions[] = self::GetActionChangeOp($oAttachment, false /* false => deletion */); } @@ -320,8 +319,8 @@ class AttachmentPlugIn implements iApplicationUIExtension, iApplicationObjectExt $oAttachment->DBUpdate(); // temporary attachment confirmed, list it in the history $aActions[] = self::GetActionChangeOp($oAttachment, true /* true => creation */); - $aData = ['target_object' => $oObject]; - $oAttachment->FireEvent(EVENT_ADD_ATTACHMENT_TO_OBJECT, $aData); + $aData = ['attachment' => $oAttachment]; + $oObject->FireEvent(EVENT_ADD_ATTACHMENT_TO_OBJECT, $aData); } } if (count($aActions) > 0) diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php new file mode 100644 index 000000000..8485c956f --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php @@ -0,0 +1,72 @@ +sAddAttachmentName = ''; + $this->sRemoveAttachmentName = ''; + + $_REQUEST['transaction_id'] = 'test_transaction'; + $_REQUEST['attachment_plugin'] = 'in_form'; + + $oDocument = new \ormDocument('Test', 'text/plain', 'test.txt'); + + $this->EventService_RegisterListener(EVENT_ADD_ATTACHMENT_TO_OBJECT, [$this, 'OnAddAttachment']); + $this->EventService_RegisterListener(EVENT_REMOVE_ATTACHMENT_FROM_OBJECT, [$this, 'OnRemoveAttachment']); + + $oAttachment = MetaModel::NewObject('Attachment', [ + 'item_class' => 'UserRequest', + 'temp_id' => 'test_transaction', + 'contents' => $oDocument, + ]); + + $oAttachment->DBInsert(); + $oTicket = $this->CreateTicket(1); + + $_REQUEST['removed_attachments'] = [$oAttachment->GetKey()]; + $this->InvokeNonPublicStaticMethod(\AttachmentPlugIn::class, 'UpdateAttachments', [$oTicket]); + + $this->assertEquals('test.txt', $this->sAddAttachmentName); + $this->assertEquals('test.txt', $this->sRemoveAttachmentName); + } + + public function OnAddAttachment(EventData $oData) + { + $this->debug('OnAddAttachment'); + $this->assertEquals('UserRequest', get_class($oData->Get('object'))); + $oAttachment = $oData->Get('attachment'); + /** @var \ormDocument $oDocument */ + $oDocument = $oAttachment->Get('contents'); + $this->sAddAttachmentName = $oDocument->GetFileName(); + } + + public function OnRemoveAttachment(EventData $oData) + { + $this->debug('OnRemoveAttachment'); + $this->assertEquals('UserRequest', get_class($oData->Get('object'))); + $oAttachment = $oData->Get('attachment'); + /** @var \ormDocument $oDocument */ + $oDocument = $oAttachment->Get('contents'); + $this->sRemoveAttachmentName = $oDocument->GetFileName(); + } +} From 82e19f6ecaf61cacb4f3db391d2891a9af034621 Mon Sep 17 00:00:00 2001 From: Anne-Catherine <57360138+accognet@users.noreply.github.com> Date: Tue, 27 Feb 2024 17:42:59 +0100 Subject: [PATCH 3/4] =?UTF-8?q?N=C2=B06543=20-=20Fix=20display=20of=20Attr?= =?UTF-8?q?ibuteText=20with=20width=20parameter=20(#521)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- application/ui.htmleditorwidget.class.inc.php | 12 +++++---- core/attributedef.class.inc.php | 1 - css/backoffice/components/_field.scss | 27 ++++++++++++++----- .../components/datatable/_datatable.scss | 7 +++++ 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/application/ui.htmleditorwidget.class.inc.php b/application/ui.htmleditorwidget.class.inc.php index 0f05f1382..9c4eb243a 100644 --- a/application/ui.htmleditorwidget.class.inc.php +++ b/application/ui.htmleditorwidget.class.inc.php @@ -71,15 +71,16 @@ class UIHTMLEditorWidget // To change the default settings of the editor, // a) edit the file /js/ckeditor/config.js // b) or override some of the configuration settings, using the second parameter of ckeditor() + $sJSDefineWidth = ''; $aConfig = utils::GetCkeditorPref(); $sWidthSpec = addslashes(trim($this->m_oAttDef->GetWidth())); - if ($sWidthSpec != '') - { - $aConfig['width'] = $sWidthSpec; + if ($sWidthSpec != '') { + /*N°6543 - the function min allow to keep text inside the column when width is defined*/ + $aConfig['width'] = "min($sWidthSpec,100%)"; + $sJSDefineWidth = '$("#cke_'.$iId.' iframe").contents().find("body").css("width", "'.$sWidthSpec.'")'; } $sHeightSpec = addslashes(trim($this->m_oAttDef->GetHeight())); - if ($sHeightSpec != '') - { + if ($sHeightSpec != '') { $aConfig['height'] = $sHeightSpec; } $sConfigJS = json_encode($aConfig); @@ -110,6 +111,7 @@ $('#$iId').on('update', function(evt){ else { oMe.data('ckeditorInstance').setReadOnly(oMe.prop('disabled')); + $sJSDefineWidth } }; setTimeout(delayedSetReadOnly, 50); diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index 9b9446081..e80d8de86 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -4534,7 +4534,6 @@ class AttributeText extends AttributeString $sStyle = ''; if (count($aStyles) > 0) { - $aStyles[] = 'overflow:auto'; $sStyle = 'style="'.implode(';', $aStyles).'"'; } diff --git a/css/backoffice/components/_field.scss b/css/backoffice/components/_field.scss index 642715bd2..6b20c1e51 100644 --- a/css/backoffice/components/_field.scss +++ b/css/backoffice/components/_field.scss @@ -80,16 +80,29 @@ $ibo-field--enable-bulk--checkbox--margin-left: $ibo-spacing-300 !default; } } } + + /*N°6543 - We need the rule to keep text inside the column when width is defined*/ + &[data-attribute-type="AttributeHtml"], + &[data-attribute-type="AttributeText"] { + &[data-attribute-flag-read-only="true"] { + display: grid; + + > .ibo-field--value { + max-width: 100%; + overflow: auto; + } + } + } } -/* Large field = Label on top, value below */ -.ibo-field-large { - display: block; + /* Large field = Label on top, value below */ + .ibo-field-large { + display: block; - .ibo-field--label { - position: relative; /* Necessary for fullscreen toggler */ - display: flex; - align-items: center; + .ibo-field--label { + position: relative; /* Necessary for fullscreen toggler */ + display: flex; + align-items: center; max-width: initial; width: 100%; } diff --git a/css/backoffice/components/datatable/_datatable.scss b/css/backoffice/components/datatable/_datatable.scss index 95797d908..c8147baa6 100644 --- a/css/backoffice/components/datatable/_datatable.scss +++ b/css/backoffice/components/datatable/_datatable.scss @@ -123,6 +123,13 @@ $ibo-fieldsorter--selected--background-color: $ibo-color-blue-200 !default; .ibo-datatable--row-actions-toolbar{ justify-content: end; } + /* N°6543 - We need the rule to keep text inside the column when width is defined */ + > [data-attribute-type="AttributeHtml"], + > [data-attribute-type="AttributeText"] { + max-width: 100%; + overflow: auto; + } + } } From 86bf6ba0b0ad54a56213db4f9d679f5df38b177a Mon Sep 17 00:00:00 2001 From: Anne-Catherine <57360138+accognet@users.noreply.github.com> Date: Tue, 27 Feb 2024 17:59:37 +0100 Subject: [PATCH 4/4] =?UTF-8?q?N=C2=B05547=20-=20Object=20deletion=20fails?= =?UTF-8?q?=20if=20friendlyname=20too=20long=20(#529)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/cmdbobject.class.inc.php | 2 +- .../unitary-tests/core/CMDBObjectTest.php | 49 ++++++++++++++ .../unitary-tests/core/DBObjectTest.php | 64 +++++++++++++++++-- 3 files changed, 108 insertions(+), 7 deletions(-) diff --git a/core/cmdbobject.class.inc.php b/core/cmdbobject.class.inc.php index c9ccb436f..ca0413b5b 100644 --- a/core/cmdbobject.class.inc.php +++ b/core/cmdbobject.class.inc.php @@ -335,7 +335,7 @@ abstract class CMDBObject extends DBObject $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 index 4d2e2ad9f..8188ec08f 100644 --- a/tests/php-unit-tests/unitary-tests/core/CMDBObjectTest.php +++ b/tests/php-unit-tests/unitary-tests/core/CMDBObjectTest.php @@ -5,9 +5,11 @@ namespace Combodo\iTop\Test\UnitTest\Core; 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 @@ class CMDBObjectTest extends ItopDataTestCase 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 index 433032aca..321610d78 100644 --- a/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php +++ b/tests/php-unit-tests/unitary-tests/core/DBObjectTest.php @@ -1133,6 +1133,54 @@ class DBObjectTest extends ItopDataTestCase 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 @@ class DBObjectTest extends ItopDataTestCase $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 @@ class DBObjectTest extends ItopDataTestCase /** * 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 */