When I add a FAPI managed file field to a non-entity form and upload a file when the sites/default/files directory has insufficient permissions to write I get the following message:

Notice: Undefined index: #field_name in file_managed_file_save_upload() (line 638 of /var/www/ae7/modules/file/file.module).

The problem being that on non-entity forms #field_name is not set.

This affects things like the media module, along with these other issues:
#1373046: Zen | Notice: Undefined index: #field_name in file_managed_file_save_upload() | line 630 file.module
#1218048: Notice: Undefined index: #field_name in file_managed_file_save_upload()

So either the problem is that non-entity form elements should be getting a #field_name, or other modules should not be relying on #field_name being there.

It seems like it should be the former.

I still need to check drupal 8 for the same issue.

CommentFileSizeAuthor
#56 1903010-56-complete.patch3.75 KBJo Fitzgerald
#56 1903010-56-test_only.patch2.58 KBJo Fitzgerald
#51 notice_undefined-1903010-51.patch3.75 KBcebasqueira
#49 notice_undefined-1903010-48.patch3.76 KBcebasqueira
#48 notice_undefined-1903010-48.patch3.76 KBcebasqueira
#46 1903010-46.patch3.48 KBoriol_e9g
#4 drupal-undefinedindex_fileupload-1903010-4.patch1.09 KBbudda
PASSED: [[SimpleTest]]: [MySQL] 41,081 pass(es). View
#17 1903010-file-managed-file-save-upload-no-field-name.patch1.17 KBDave Reid
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,231 pass(es). View
#18 1903010-file-managed-file-save-upload-no-field-name-D7-do-not-test.patch1.09 KBDave Reid
#24 1903010-file-managed-file-save-upload-no-field-name-D7-do-not-test.patch1.04 KBSebastien VR
#30 1903010-file-managed-file-save-upload-no-field-name.patch1.17 KBcatch
#32 notice_undefined-1903010-32.patch1.17 KBdermario
#35 1903010-tests-should-fail.patch1.2 KBoriol_e9g
#36 notice-fix-and-test-1903010.patch2.37 KBoriol_e9g
#38 notice-fix-and-test-1903010-38.png42.19 KBamit.drupal
#43 1903010-43.patch2.37 KBoriol_e9g
#44 file-logger-test.png44.58 KBoriol_e9g
#44 1903010-44.patch3.75 KBoriol_e9g
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

rooby’s picture

Version: 7.19 » 7.x-dev
timbrandin’s picture

This line should not give any errors anyways (modules/file/file.module:637)

    watchdog('file', 'The upload directory %directory for the file field !name could not be created or is not accessible. A newly uploaded file could not be saved in this directory as a consequence, and the upload was canceled.', array('%directory' => $destination, '!name' => $element['#field_name']));
budda’s picture

I've seen the same issue when playing with Open Atrium 2 and uploading a banner image via the Media module.

Notice: Undefined index: #field_name in file_managed_file_save_upload() (line 637 of /webroot/atrium2.drupal.dev/modules/file/file.module).
The file could not be uploaded.

budda’s picture

Issue summary: View changes
FileSize
1.09 KB
PASSED: [[SimpleTest]]: [MySQL] 41,081 pass(es). View

Changed the $element index to match the other one used in the same function later on.

budda’s picture

Status: Active » Needs review
SocialNicheGuru’s picture

Status: Needs review » Needs work
Issue tags: +media

I goto upload a file via the media module
I get the error above and can't upload the file
Notice: Undefined index: #field_name in file_managed_file_save_upload()
The file could not be uploaded.
Upload a new file field is required.

With the patch the error goes away, but I still can't upload the file with:
The file could not be uploaded.
Upload a new file field is required.

Dave Reid’s picture

Status: Needs work » Reviewed & tested by the community

This patch wouldn't affect the ability to upload a file, so the error in #6 is unrelated. #6 actually confirms what this is fixing is fixed.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: drupal-undefinedindex_fileupload-1903010-4.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: drupal-undefinedindex_fileupload-1903010-4.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community
David_Rothstein’s picture

Version: 7.x-dev » 8.0.x-dev
Status: Reviewed & tested by the community » Needs review
Issue tags: +needs backport to D7

I still need to check drupal 8 for the same issue.

Did anyone check? Looks like extremely similar code exists in Drupal 8, so I would expect the bug is very likely to exist there too.

jhedstrom’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
jhedstrom’s picture

Issue tags: -Needs reroll

Not reroll. Just needs to be verified if this is fixed in D8 or not.

Dave Reid’s picture

Assigned: Unassigned » Dave Reid
Status: Needs work » Needs review
Issue tags: +D8Media
FileSize
1.17 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,231 pass(es). View

Same fix for Drupal 8, but changing "for the file field !name" to "for the !name field" since we cannot assume this is being used with a file field.

Dave Reid’s picture

The same patch for Drupal 7 (marked as DNT).

Dave Reid’s picture

This is a blocker for File Entity. We keep getting bug reports about this, anyone able to review the latest versions?

dermario’s picture

The patch in #18 works for me on D7. There is no notice any more.

discipolo’s picture

jday’s picture

Tested the patch in #18 and it works, no longer getting the error message

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

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

Sebastien VR’s picture

Status: Needs review » Needs work

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

oriol_e9g’s picture

Status: Needs work » Reviewed & tested by the community

Same notices detected writing tests in Drupal 8.

You can see the notice using a managed_file in a theme settings: https://www.drupal.org/node/1862892#comment-11766744

And then the notices go way when apply the patch in #17: https://www.drupal.org/node/1862892#comment-11766843

For me patch in #17 is RTBC

amit.drupal’s picture

Tested the patch in #18 and it works, no error message found.

catch’s picture

Status: Reviewed & tested by the community » Needs review

Re-uploading #17 for testing since it's 18 months old. Didn't review yet.

catch’s picture

Status: Needs review » Needs work

The last submitted patch, 30: 1903010-file-managed-file-save-upload-no-field-name.patch, failed testing.

dermario’s picture

Rerolled the patch in #17 to apply against the latest 8.2.x

dermario’s picture

Status: Needs work » Needs review
catch’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks for the re-roll.

The lack of test changes means we don't have test coverage for this hunk in file_managed_save_upload(), so let's add some here.

oriol_e9g’s picture

Status: Needs work » Needs review
FileSize
1.2 KB
oriol_e9g’s picture

And this should pas.

The last submitted patch, 35: 1903010-tests-should-fail.patch, failed testing.

amit.drupal’s picture

@oriol_e9g #36 looking nice.
Please see notice-fix-and-test-1903010-38.png .

catch’s picture

Status: Needs review » Reviewed & tested by the community

That's much better, thanks!

+++ b/core/modules/file/src/Tests/FileManagedFileElementTest.php
@@ -148,6 +148,13 @@ function testManagedFile() {
+    // Submit file to a folder without permissions to force message fail

Missing period on the end of the comment but that can be fixed on commit. Moving back to RTBC.

oriol_e9g’s picture

xjm’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/file/file.module
@@ -1183,7 +1183,7 @@ function file_managed_file_save_upload($element, FormStateInterface $form_state)
-    \Drupal::logger('file')->notice('The upload directory %directory for the file field %name could not be created or is not accessible. A newly uploaded file could not be saved in this directory as a consequence, and the upload was canceled.', array('%directory' => $destination, '%name' => $element['#field_name']));
+    \Drupal::logger('file')->notice('The upload directory %directory for the !name field could not be created or is not accessible. A newly uploaded file could not be saved in this directory as a consequence, and the upload was canceled.', array('%directory' => $destination, '!name' => $element['#title']));

We no longer support !placeholder so we should not use it with the logger either.

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Rend...
https://www.drupal.org/node/2575819
https://www.drupal.org/node/2605274

Thanks!

xjm’s picture

Also the fact that the patch passed implies to me the test coverage isn't quite complete?

oriol_e9g’s picture

Assigned: Dave Reid » Unassigned
Status: Needs work » Needs review
FileSize
2.37 KB
oriol_e9g’s picture

If you want, we can test that the notice is generated and saved correctly in the DBLog... but I don't know if it's really necessary for a notice message.

The last submitted patch, 44: 1903010-44.patch, failed testing.

oriol_e9g’s picture

Random test fail, attached more compact test.

We can use this patch #46 or #43 if we think that it's not necessari test the notice message.

Status: Needs review » Needs work

The last submitted patch, 46: 1903010-46.patch, failed testing.

cebasqueira’s picture

Small change to become more coherent!

cebasqueira’s picture

Status: Needs work » Needs review
FileSize
3.76 KB

Status: Needs review » Needs work

The last submitted patch, 49: notice_undefined-1903010-48.patch, failed testing.

cebasqueira’s picture

Status: Needs work » Needs review
FileSize
3.75 KB

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.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.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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

kenorb’s picture

Same problem in 8.3.7, but the patch doesn't apply cleanly for 8.3.x.

jhedstrom’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Needs a reroll (presumably since arrays are now in bracket syntax). Could somebody also re-upload the test-only patch since that has changed since the last test-only patch?

Jo Fitzgerald’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.58 KB
3.75 KB

Re-rolled.

The last submitted patch, 56: 1903010-56-test_only.patch, failed testing. View results

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.