Problem/Motivation

When programmatically submitting forms, I noticed the logs filling up with the following warning:

Warning: end() expects parameter 1 to be array, null given in Drupal\file\Element\ManagedFile::validateManagedFile() (line 404 of
/core/modules/file/src/Element/ManagedFile.php)

This is because of the following code:

  /**
   * Render API callback: Validates the managed_file element.
   */
  public static function validateManagedFile(&$element, FormStateInterface $form_state, &$complete_form) {
    // If referencing an existing file, only allow if there are existing
    // references. This prevents unmanaged files from being deleted if this
    // item were to be deleted.
    $clicked_button = end($form_state->getTriggeringElement()['#parents']);

According to the FormStateInterface, a valid return value for getTriggeringElement() is null:

  /**
   * Gets the form element that triggered submission.
   *
   * @return array|null
   *   The form element that triggered submission, of NULL if there is none.
   */
  public function &getTriggeringElement();

In the case of my programmatic submission, the call to getTriggeringElement() returns null (again, this is valid according to the interface), which is then passed to php's end function, which expects an array.

One potential solution is below (patch to come):

    $triggering_element = $form_state->getTriggeringElement();
    $clicked_button = $triggering_element ? end($triggering_element['#parents']) : '';

Steps to reproduce

- Install and enable webform 8.x-5.0-beta13
- Create a webform with an element for attachments (i.e. managed_file element)
- Write code to submit a webform programmatically as seen in the webform cookbook.
- Submit the form programmatically

Note: This should happen with any programmatic form submission with a managed_file element, my entry point just happened to be through the webform module.

Expected behavior:
- Submission validated or created without PHP warnings.

What happened instead:
- PHP warning described below observed.

This issue was observed on:
- macOS Sierra 10.12.2
- Drupal 8.3.7 instance of Core, but see that it still exists in 8.5.x.

Proposed resolution

Remaining tasks

Add a test
review
commit

User interface changes

API changes

Data model changes

Release notes snippet

Comments

malik.kotob created an issue. See original summary.

malik.kotob’s picture

StatusFileSize
new891 bytes
malik.kotob’s picture

Status: Active » Needs review
malik.kotob’s picture

StatusFileSize
new922 bytes

Patch for 8.3.7 attached.

barrett’s picture

Status: Needs review » Reviewed & tested by the community

I've reviewed and can confirm both the originally reported issue and that the attached patch resolves the issue.

gábor hojtsy’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

This seems very logical that it would fix the issue. Is there a way to add tests to it though?

henk’s picture

I wrote the custom method to submit form via API and had same problem with saving files, for me fix was adding to submit form argument:

                $element = ['#parents' => ['submit']];
                $form_state->setTriggeringElement($element);

Patch check only if triggering element exist, probably we should check if it's array with key '#parents'.

I think it's design question do we force to use Triggering Element for any form (documentation should be updated) or allow exeption in some case (code must be updated).

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Issue summary: View changes
Issue tags: +Bug Smash Initiative

This was a bugsmash target today. The changes in the patch haven't been added to core so I guess this is still valid and still needs tests. I did not test.

murilohp’s picture

Issue tags: -Needs tests
StatusFileSize
new1.89 KB
new2.78 KB

I've added a test in this new patch and also followed #7 suggestion, now the #parents is validated and this should avoid the error.

The test-only patch is supposed to be failed with the same error provided on the IS.

murilohp’s picture

Status: Needs work » Needs review

Since the test patch failed with the correct error, I'm moving to NR.

henk’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +ContributionWeekend2023

Test for 2910320-17.patch probably faild on CKeditor problem from here: https://www.drupal.org/project/drupal/issues/3268368

Checking if array key "parents" exist should fix the PHP error problem.

larowlan’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/modules/file/tests/file_test/src/Form/ManagedFileTestForm.php
@@ -0,0 +1,38 @@
+class ManagedFileTestForm extends FormBase {

We can have the test and form as the one class (no need for the extra class) https://www.previousnext.com.au/blog/drupal-8-ftw-it-test-or-it-form-act...

The test failure is Media library, not CKEditor, which might be related to this patch, can we get a re-run please?

murilohp’s picture

StatusFileSize
new2.46 KB
new2.53 KB

Hey @larowlan, thanks for your input here, I've changed the test and put the form inside the test class. Awesome blog and great post by the way! Really helpful!

Regarding the failed test, I wasn't able to reproduce it locally, it's passing here :( , so let's take another shot.

jonathanshaw’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

The last submitted patch, 17: 2910320-17.patch, failed testing. View results

larowlan’s picture

Confirming the test fails without the change


Testing Drupal\Tests\file\Kernel\ManagedFileTest
E                                                                   1 / 1 (100%)

Time: 00:01.524, Memory: 10.00 MB

There was 1 error:

1) Drupal\Tests\file\Kernel\ManagedFileTest::testManagedFileElement
TypeError: end(): Argument #1 ($array) must be of type array, null given

The last submitted patch, 17: 2910320-17.patch, failed testing. View results

  • larowlan committed 5f06ef9a on 10.0.x
    Issue #2910320 by murilohp, malik.kotob: Validating managed files should...

  • larowlan committed c886a764 on 10.1.x
    Issue #2910320 by murilohp, malik.kotob: Validating managed files should...

  • larowlan committed 04b5939a on 9.5.x
    Issue #2910320 by murilohp, malik.kotob: Validating managed files should...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 10.1.x and backported to 10.0.x and 9.5.x - thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.