Project

General

Profile

Actions

Bug #94729

open

Method ArrayUtility::replaceAndAppendScalarValuesRecursive re-numbers associative arrays with non-consecutively numbered integer-only keys

Added by Stephan Jorek almost 3 years ago. Updated almost 1 year ago.

Status:
Under Review
Priority:
Should have
Assignee:
-
Category:
-
Start date:
2021-08-05
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
10
PHP Version:
7.4
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

Description

Hello Everybody,

as already indicated in [[http://forge.typo3.org/issues/91236]] under certain conditions the method …

\TYPO3\CMS\Core\Utility\ArrayUtility::replaceAndAppendScalarValuesRecursive

… sometimes re-numbers associative keys, which look like plain consecutively numbered (YAML-)list keys. Before TYPO3 version 10 this was not as obvious as it became in version 10 and above, because previous versions of the Form-Framework did not use the …

\TYPO3\CMS\Core\Configuration\Loader\YamlFileLoader

… as documented in [[https://docs.typo3.org/c/typo3/cms-core/master/en-us/Changelog/10.2/Feature-84203-UnifyFormSetupYAMLLoading.html]]. But of course it was/is also an issue in version below 10, but not in the form-framework.

Before getting into detail, here is a working solution: [[https://review.typo3.org/c/Packages/TYPO3.CMS/+/70246]]

How can this be explained and reproduced?

First of all I will describe the State of YAML-Loading in the Form-Framework before TYPO3 Version 10. Let's assume we have the following (simplified) setup:

Typoscript

plugin.tx_form.settings.yamlConfigurations {
    10 = EXT:form/Configuration/Yaml/FormSetup.yaml
    20 = EXT:my_custom_extension/Configuration/Yaml/FormSetup.yaml
}

EXT:form/Configuration/Yaml/FormSetup.yaml

TYPO3:
  CMS:
    Form:
      ########### FORMEDITOR CONFIGURATION ###########
      prototypes:
        standard:
          ########### DEFAULT FORM ELEMENT DEFINITIONS ###########
          formElementsDefinition:
            Form:
              formEditor:
                editors:
                  900:
                    selectOptions:
                      10:
                        value: ''
                        label: 'formEditor.elements.Form.editor.finishers.EmptyValue.label'
                      20:
                        value: 'EmailToSender'
                        label: 'formEditor.elements.Form.editor.finishers.EmailToSender.label'
                      30:
                        # …
                      40:
                        # …
                      50:
                        # …
                      60:
                        # …

# …

EXT:my_custom_extension/Configuration/Yaml/FormSetup.yaml

TYPO3:
  CMS:
    Form:
      prototypes:
        standard:
          formElementsDefinition:
            Form:
              formEditor:
                editors:
                  900:
                      # Example:
                      # Hide the EmailToSender finisher, as 
                      # the customer never wants to use it,
                      # even not by accident
                      20: null

Processing of the Form-Framework YAML-files in TYPO3 ≤ v9

The Yaml-Parser turns both YAML-files into Arrays and both arrays are merged together using array_replace_recursive, see [[https://github.com/TYPO3/typo3/blob/9.5/typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php#L90-L94]]

The result is something like:

TYPO3:
  CMS:
    Form:
      prototypes:
        standard:
          formElementsDefinition:
            Form:
              formEditor:
                editors:
                  900:
                    selectOptions:

                      10:
                        value: ''
                        label: 'formEditor.elements.Form.editor.finishers.EmptyValue.label'

                      # Here is the 20: null from above
                      20: null

                      30:
                        # …
                      40:
                        # …
                      50:
                        # …
                      60:
                        # …

Processing of the Form-Framework YAML-files in TYPO3 ≥ v10

When we assume the exact same setup as explained above, nothing really changes, and everything works as expected, see [[https://github.com/TYPO3/typo3/blob/master/typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php#L69-L77]]. The only difference is, that this time the YAML::parse is not used directly, but indirectly through the YamlLoader, which introduces the import feature, see [[https://github.com/TYPO3/typo3/blob/master/typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php#L136-L139]]. Hence the result is the same as shown above.

A slightly different setup in TYPO3 ≥ v10 triggering the error

Typoscript

# same as above
plugin.tx_form.settings.yamlConfigurations {
    10 = EXT:form/Configuration/Yaml/FormSetup.yaml
    20 = EXT:my_custom_extension/Configuration/Yaml/FormSetup.yaml
}

EXT:form/Configuration/Yaml/FormSetup.yaml


# the same as above

TYPO3:
  CMS:
    Form:
      ########### FORMEDITOR CONFIGURATION ###########
      prototypes:
        standard:
          ########### DEFAULT FORM ELEMENT DEFINITIONS ###########
          formElementsDefinition:
            Form:
              formEditor:
                editors:
                  900:
                    selectOptions:
                      10:
                        value: ''
                        label: 'formEditor.elements.Form.editor.finishers.EmptyValue.label'
                      20:
                        value: 'EmailToSender'
                        label: 'formEditor.elements.Form.editor.finishers.EmailToSender.label'
                      30:
                        # …
                      40:
                        # …
                      50:
                        # …
                      60:
                        # …

# …

EXT:my_custom_extension/Configuration/Yaml/FormSetup.yaml

# Now we're using an import here!
import:
  - { resource: "./Finishers/EmailToSender.yaml" }

TYPO3:
  CMS:
    Form:
      prototypes:
        

EXT:my_custom_extension/Configuration/Yaml/Finishers/EmailToSender.yaml

# The same content, as used in FormSetup.yaml previously
TYPO3:
  CMS:
    Form:
      prototypes:
        standard:
          formElementsDefinition:
            Form:
              formEditor:
                editors:
                  900:
                      # Example:
                      # Hide the EmailToSender finisher, as 
                      # the customer never wants to use it,
                      # even not by accident
                      20: null

The (broken) Result

The result is now neither the same as above, nor correct anymore. It looks like:


# the same as above

TYPO3:
  CMS:
    Form:
      prototypes:
        standard:
          formElementsDefinition:
            Form:
              formEditor:
                editors:
                  900:
                    selectOptions:

                      # Here is the 20: null from above
                      0: null

                      10:
                        value: ''
                        label: 'formEditor.elements.Form.editor.finishers.EmptyValue.label'
                      20:
                        value: 'EmailToSender'
                        label: 'formEditor.elements.Form.editor.finishers.EmailToSender.label'
                      30:
                        # …
                      40:
                        # …
                      50:
                        # …
                      60:
                        # …

Processing of the Form-Framework YAML-file imports in TYPO3 ≥ v10

So what happened? The reason for this, is that the YamlLoader treats the import differently, as the YamlSource implementation does. The latter uses, as mentioned above, array_replace_recursive, whereas the import of the EmailToSender.yaml fragment-file into the FormSetup.yaml file is using ArrayUtility::replaceAndAppendScalarValuesRecursive to merge the two arrays, see [[https://github.com/TYPO3/typo3/blob/master/typo3/sysext/core/Classes/Configuration/Loader/YamlFileLoader.php#L164-L166]]. For understanding, it is important to know, that the array of the FormSetup.yaml is merged into the array of the EmailToSender.yaml-file, so the array of the EmailToSender.yaml is the first argument in the ArrayUtility::replaceAndAppendScalarValuesRecursive method-call, see previous link.

ArrayUtility::replaceAndAppendScalarValuesRecursive

The purpose of this method is to merge two arrays like array_replace_recursive does, with one exception - if the first argument (target array or one if it's subpaths) is a simple (YAML-) list, the key does not get replaced. Instead the source-value-list (array) is appended to the target-value-list (array) using a simple array_merge($target, $source), see [[https://github.com/TYPO3/typo3/blob/master/typo3/sysext/core/Classes/Utility/ArrayUtility.php#L940-L943]]. And as mentioned in [[https://www.php.net/manual/en/function.array-merge.php]]:

Values in the input arrays with numeric keys will be renumbered with incrementing keys starting from zero in the result array.

This is where the renumbered keys are created.

// …
        // Simple lists get merged / added up
        if (!self::isAssociative($array1)) {
            return array_merge($array1, $array2);
        }
// …

Remember: $array1 is the array from to import, so in the ArrayUtility::isAssociative the branch containing the 20: null declaration, looks like not being associative.

ArrayUtility::isAssociative

The current implementation of this method, assumes an array to be associative, when at least one key is a string, see [[https://github.com/TYPO3/typo3/blob/master/typo3/sysext/core/Classes/Utility/ArrayUtility.php#L924-L927]]

// …
    public static function isAssociative(array $array): bool
    {
        return count(array_filter(array_keys($array), 'is_string')) > 0;
    }
// …

Is the YAML-declaration 20: null associative? Yes - we want to override exactly this key. Does the array contain a string as the key? No … now one could argument, to declare the key simply as a string, like '20': null. But this also does not work, due to special treatment of integer-like string-keys in arrays, which automatically are turned into integers. Here is an example to demonstrate this behaviour in the PHP 7.0 to 8.0:


for PHP in php{70,71,72,73,74,80} ; do 
  echo "$PHP:" 
  $PHP -r 'var_dump(array_keys(["10" => null]));'
done

php70:
Command line code:1:
array(1) {
  [0] =>
  int(10)
}
php71:
Command line code:1:
array(1) {
  [0] =>
  int(10)
}
php72:
Command line code:1:
array(1) {
  [0] =>
  int(10)
}
php73:
Command line code:1:
array(1) {
  [0] =>
  int(10)
}
php74:
Command line code:1:
array(1) {
  [0] =>
  int(10)
}
php80:
Command line code:1:
array(1) {
  [0] =>
  int(10)
}

So somehow, we must find a way to identify this case as being associative, while not losing the ability to identify simple lists. What is the difference between the simple list …

[
    'value 1',
    'value 2',
    'value 3',
    'value 4'
]

… and the following associative list, with integer-only keys ?

[
    10 => 'value',
    20 => 'value',
    30 => 'value',
    40 => 'value'
]

Obviously the keys in the latter array are not numbered consecutively. So the solution to the bug, is to assume an array to be associative, when either one of its keys is a string or all keys are not numbered consecutively (or when the array is empty). The first condition can even be even left out, as real string keys are no integers. So the final solution is:

// …
     public static function isAssociative(array $array): bool
     {
        return !empty($array) && array_keys($array) !== range(0, count($array) - 1);
     }
// …

Someone working on the TYPO3-Core had the same idea, see [[https://github.com/TYPO3/typo3/blob/master/typo3/sysext/backend/Classes/Form/Container/SingleFieldContainer.php#L327]]. The updated ArrayUtility Unit-Tests in the initially linked Gerrit-Review prove, that it is sufficient to just determine if all keys are not numbered consecutively.

Thanks, for your patience!
Stephan


Related issues 1 (1 open0 closed)

Is duplicate of TYPO3 Core - Bug #91236: Numeric array keys are not merged correctly in YAML importNew2020-04-29

Actions
Actions

Also available in: Atom PDF