From d80bab4c19c9de5893093905744b40404ddf029a Mon Sep 17 00:00:00 2001
From: Daniel Lorenz <daniel.lorenz@extco.de>
Date: Fri, 8 Sep 2017 13:52:32 +0200
Subject: [PATCH] [BUGFIX] EXT:form - catch YAML parsing errors
Catche YAML parsing errors and display this them alongside
their form definition files in form module and plugin.
Resolves: #82369
Releases: master, 8.7
Change-Id: Icf71027d21d0a8e30c238a51369676715de2e5c5
Reviewed-on: https://review.typo3.org/54014
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Mathias Brodala <mbrodala@pagemachine.de>
Tested-by: Mathias Brodala <mbrodala@pagemachine.de>
Reviewed-by: Carlos Meyer <cm@davitec.de>
Tested-by: Carlos Meyer <cm@davitec.de>
Reviewed-by: Bjoern Jacob <bjoern.jacob@tritum.de>
Tested-by: Bjoern Jacob <bjoern.jacob@tritum.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
---
.../Hooks/DataStructureIdentifierHook.php | 17 ++++--
.../Classes/Mvc/Configuration/YamlSource.php | 2 +-
.../Persistence/FormPersistenceManager.php | 21 ++++++-
.../Backend/Templates/FormManager/Index.html | 31 +++++++---
.../Resources/Private/Language/Database.xlf | 6 ++
.../Hooks/DataStructureIdentifierHookTest.php | 60 ++++++++++++-------
6 files changed, 102 insertions(+), 35 deletions(-)
diff --git a/typo3/sysext/form/Classes/Hooks/DataStructureIdentifierHook.php b/typo3/sysext/form/Classes/Hooks/DataStructureIdentifierHook.php
index 13436ba17115..cf284ea7c1a1 100644
--- a/typo3/sysext/form/Classes/Hooks/DataStructureIdentifierHook.php
+++ b/typo3/sysext/form/Classes/Hooks/DataStructureIdentifierHook.php
@@ -110,10 +110,19 @@ class DataStructureIdentifierHook
$formIsAccessible = true;
}
- $dataStructure['sheets']['sDEF']['ROOT']['el']['settings.persistenceIdentifier']['TCEforms']['config']['items'][] = [
- $form['name'] . ' (' . $form['persistenceIdentifier'] . ')',
- $form['persistenceIdentifier'],
- ];
+ if ($form['invalid']) {
+ $dataStructure['sheets']['sDEF']['ROOT']['el']['settings.persistenceIdentifier']['TCEforms']['config']['items'][] = [
+ $form['name'] . ' (' . $form['persistenceIdentifier'] . ')',
+ $form['persistenceIdentifier'],
+ 'overlay-missing'
+ ];
+ } else {
+ $dataStructure['sheets']['sDEF']['ROOT']['el']['settings.persistenceIdentifier']['TCEforms']['config']['items'][] = [
+ $form['name'] . ' (' . $form['persistenceIdentifier'] . ')',
+ $form['persistenceIdentifier'],
+ 'content-form'
+ ];
+ }
}
if (!empty($identifier['ext-form-persistenceIdentifier']) && !$formIsAccessible) {
diff --git a/typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php b/typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php
index d342c5441850..a82c83dcf2e8 100644
--- a/typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php
+++ b/typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php
@@ -107,7 +107,7 @@ class YamlSource
}
} catch (ParseException $exception) {
throw new ParseErrorException(
- 'A parse error occurred while parsing file "' . $fileIdentifier . '". Error message: ' . $exception->getMessage(),
+ 'An error occurred while parsing file "' . $fileIdentifier . '": ' . $exception->getMessage(),
1480195405
);
}
diff --git a/typo3/sysext/form/Classes/Mvc/Persistence/FormPersistenceManager.php b/typo3/sysext/form/Classes/Mvc/Persistence/FormPersistenceManager.php
index fca17892f828..64eef0172e78 100644
--- a/typo3/sysext/form/Classes/Mvc/Persistence/FormPersistenceManager.php
+++ b/typo3/sysext/form/Classes/Mvc/Persistence/FormPersistenceManager.php
@@ -105,7 +105,22 @@ class FormPersistenceManager implements FormPersistenceManagerInterface
} else {
$file = $this->getFileByIdentifier($persistenceIdentifier);
}
- return $this->yamlSource->load([$file]);
+
+ try {
+ $yaml = $this->yamlSource->load([$file]);
+ } catch (\Exception $e) {
+ $yaml = [
+ 'identifier' => $file->getCombinedIdentifier(),
+ 'label' => $e->getMessage(),
+ 'error' => [
+ 'code' => $e->getCode(),
+ 'message' => $e->getMessage()
+ ],
+ 'invalid' => true,
+ ];
+ }
+
+ return $yaml;
}
/**
@@ -237,6 +252,8 @@ class FormPersistenceManager implements FormPersistenceManagerInterface
'removable' => true,
'location' => 'storage',
'duplicateIdentifier' => false,
+ 'invalid' => $form['invalid'],
+ 'error' => $form['error'],
];
$identifiers[$form['identifier']]++;
}
@@ -258,6 +275,8 @@ class FormPersistenceManager implements FormPersistenceManagerInterface
'removable' => $this->formSettings['persistenceManager']['allowDeleteFromExtensionPaths'] ? true: false,
'location' => 'extension',
'duplicateIdentifier' => false,
+ 'invalid' => $form['invalid'],
+ 'error' => $form['error'],
];
$identifiers[$form['identifier']]++;
}
diff --git a/typo3/sysext/form/Resources/Private/Backend/Templates/FormManager/Index.html b/typo3/sysext/form/Resources/Private/Backend/Templates/FormManager/Index.html
index 26aec41d7b68..e59648edef54 100644
--- a/typo3/sysext/form/Resources/Private/Backend/Templates/FormManager/Index.html
+++ b/typo3/sysext/form/Resources/Private/Backend/Templates/FormManager/Index.html
@@ -33,12 +33,17 @@
<f:for each="{forms}" as="form">
<tr>
<td class="col-icon nowrap">
- <f:if condition="{form.duplicateIdentifier}">
+ <f:if condition="{form.invalid}">
<f:then>
- <span title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_identifier')} {form.identifier}" data-toggle="tooltip" data-placement="top">
+ <span title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.invalid')}" data-toggle="tooltip" data-placement="top">
<core:icon identifier="overlay-missing" />
</span>
</f:then>
+ <f:else if="{form.duplicateIdentifier}">
+ <span title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_identifier')} {form.identifier}" data-toggle="tooltip" data-placement="top">
+ <core:icon identifier="overlay-missing" />
+ </span>
+ </f:else>
<f:else>
<span title="id={form.identifier}" data-toggle="tooltip" data-placement="right">
<core:icon identifier="content-elements-mailform" />
@@ -47,9 +52,9 @@
</f:if>
</td>
<td class="col-title col-responsive nowrap">
- <f:if condition="{form.readOnly}">
+ <f:if condition="{form.invalid} || {form.readOnly}">
<f:then>
- <div>{form.name}</div>
+ <div title="{form.name}">{form.name}</div>
</f:then>
<f:else>
<f:link.action controller="FormEditor" action="index" arguments="{formPersistenceIdentifier: form.persistenceIdentifier}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.edit_form')}" data="{toggle: 'tooltip', placement: 'right'}">{form.name}</f:link.action>
@@ -58,7 +63,7 @@
</td>
<td class="col-control nowrap">
<div class="btn-group" role="group">
- <f:if condition="{form.readOnly}">
+ <f:if condition="{form.invalid} || {form.readOnly}">
<f:then>
<button class="btn btn-default form-record-readonly" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.edit_form_not_allowed')}"><core:icon identifier="actions-open" /></button>
</f:then>
@@ -66,13 +71,21 @@
<f:link.action controller="FormEditor" action="index" arguments="{formPersistenceIdentifier: form.persistenceIdentifier}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.edit_form')}" class="btn btn-default form-record-open"><core:icon identifier="actions-open" /></f:link.action>
</f:else>
</f:if>
- <a href="#" data-identifier="duplicateForm" data-form-persistence-identifier="{form.persistenceIdentifier}" data-form-name="{form.name}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_this_form')}" class="btn btn-default form-record-duplicate"><core:icon identifier="t3-form-icon-duplicate" /></a>
- <f:if condition="{form.removable}">
+ <f:if condition="{form.invalid}">
<f:then>
- <a href="#" data-identifier="removeForm" data-form-persistence-identifier="{form.persistenceIdentifier}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form')}" class="btn btn-default form-record-delete"><core:icon identifier="actions-edit-delete" /></a>
+ <button class="btn btn-default form-record-readonly" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_form_not_allowed')}"><core:icon identifier="t3-form-icon-duplicate" /></button>
+ <button class="btn btn-default form-record-readonly" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form_not_allowed')}"><core:icon identifier="actions-edit-delete" /></button>
</f:then>
<f:else>
- <button class="btn btn-default form-record-delete" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form_not_allowed')}"><core:icon identifier="actions-edit-delete" /></button>
+ <a href="#" data-identifier="duplicateForm" data-form-persistence-identifier="{form.persistenceIdentifier}" data-form-name="{form.name}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_this_form')}" class="btn btn-default form-record-duplicate"><core:icon identifier="t3-form-icon-duplicate" /></a>
+ <f:if condition="{form.removable}">
+ <f:then>
+ <a href="#" data-identifier="removeForm" data-form-persistence-identifier="{form.persistenceIdentifier}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form')}" class="btn btn-default form-record-delete"><core:icon identifier="actions-edit-delete" /></a>
+ </f:then>
+ <f:else>
+ <button class="btn btn-default form-record-delete" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form_not_allowed')}"><core:icon identifier="actions-edit-delete" /></button>
+ </f:else>
+ </f:if>
</f:else>
</f:if>
</div>
diff --git a/typo3/sysext/form/Resources/Private/Language/Database.xlf b/typo3/sysext/form/Resources/Private/Language/Database.xlf
index 34651b1e12bd..92877d9f1626 100644
--- a/typo3/sysext/form/Resources/Private/Language/Database.xlf
+++ b/typo3/sysext/form/Resources/Private/Language/Database.xlf
@@ -178,9 +178,15 @@
<trans-unit id="formManager.edit_form_not_allowed" xml:space="preserve">
<source>Edit this form is not allowed.</source>
</trans-unit>
+ <trans-unit id="formManager.duplicate_form_not_allowed" xml:space="preserve">
+ <source>Duplicate this form is not allowed.</source>
+ </trans-unit>
<trans-unit id="formManager.delete_form_not_allowed" xml:space="preserve">
<source>Delete this form is not allowed.</source>
</trans-unit>
+ <trans-unit id="formManager.invalid" xml:space="preserve">
+ <source>Invalid YAML file!</source>
+ </trans-unit>
<trans-unit id="formManager.duplicate_identifier" xml:space="preserve">
<source>Duplicate identifier!</source>
</trans-unit>
diff --git a/typo3/sysext/form/Tests/Unit/Hooks/DataStructureIdentifierHookTest.php b/typo3/sysext/form/Tests/Unit/Hooks/DataStructureIdentifierHookTest.php
index 7ac936971417..74bc44f95f4a 100644
--- a/typo3/sysext/form/Tests/Unit/Hooks/DataStructureIdentifierHookTest.php
+++ b/typo3/sysext/form/Tests/Unit/Hooks/DataStructureIdentifierHookTest.php
@@ -171,8 +171,12 @@ class DataStructureIdentifierHookTest extends \TYPO3\TestingFramework\Core\Unit\
/**
* @test
+ * @dataProvider parseDataStructureByIdentifierPostProcessDataProvider
+ *
+ * @param array $formDefinition
+ * @param array $expectedItem
*/
- public function parseDataStructureByIdentifierPostProcessAddsExistingFormItems()
+ public function parseDataStructureByIdentifierPostProcessAddsExistingFormItems(array $formDefinition, array $expectedItem)
{
$objectMangerProphecy = $this->prophesize(ObjectManager::class);
GeneralUtility::setSingletonInstance(ObjectManager::class, $objectMangerProphecy->reveal());
@@ -180,17 +184,7 @@ class DataStructureIdentifierHookTest extends \TYPO3\TestingFramework\Core\Unit\
$objectMangerProphecy->get(FormPersistenceManagerInterface::class)
->willReturn($formPersistenceManagerProphecy->reveal());
- $existingForms = [
- [
- 'persistenceIdentifier' => 'hugo1',
- 'name' => 'myHugo1',
- ],
- [
- 'persistenceIdentifier' => 'hugo2',
- 'name' => 'myHugo2',
- ]
- ];
- $formPersistenceManagerProphecy->listForms()->shouldBeCalled()->willReturn($existingForms);
+ $formPersistenceManagerProphecy->listForms()->shouldBeCalled()->willReturn([$formDefinition]);
$incomingDataStructure = [
'sheets' => [
@@ -228,14 +222,7 @@ class DataStructureIdentifierHookTest extends \TYPO3\TestingFramework\Core\Unit\
0 => 'default, no value',
1 => '',
],
- 1 => [
- 0 => 'myHugo1 (hugo1)',
- 1 => 'hugo1',
- ],
- 2 => [
- 0 => 'myHugo2 (hugo2)',
- 1 => 'hugo2',
- ],
+ 1 => $expectedItem,
],
],
],
@@ -254,6 +241,39 @@ class DataStructureIdentifierHookTest extends \TYPO3\TestingFramework\Core\Unit\
$this->assertEquals($expected, $result);
}
+ /**
+ * @return array
+ */
+ public function parseDataStructureByIdentifierPostProcessDataProvider(): array
+ {
+ return [
+ 'simple' => [
+ [
+ 'persistenceIdentifier' => 'hugo1',
+ 'name' => 'myHugo1',
+ ],
+ [
+ 'myHugo1 (hugo1)',
+ 'hugo1',
+ 'content-form',
+ ],
+ ],
+ 'invalid' => [
+ [
+ 'persistenceIdentifier' => 'Error.yaml',
+ 'label' => 'Test Error Label',
+ 'name' => 'Test Error Name',
+ 'invalid' => true,
+ ],
+ [
+ 'Test Error Name (Error.yaml)',
+ 'Error.yaml',
+ 'overlay-missing',
+ ],
+ ],
+ ];
+ }
+
/**
* Data provider for implodeArrayKeysReturnsString
*
--
GitLab