Attached files are deleted without usage checking. It causes lost of data in some cases.

@see http://cgit.drupalcode.org/webform/tree/src/Plugin/WebformElement/Webfor...
and
@see http://cgit.drupalcode.org/webform/tree/src/Plugin/WebformElement/Webfor...

Problem/Motivation

All managed files should have usage checked before deletion. In general, files in webforms are uploaded by user and won't be used in other places except for the submission entity that the file is uploaded. But in some cases, like when we set a default files in the file field, the same file entity will be used across multiple submissions. Deleting files without checking the usage will cause lost of data.

SUGGESTIONS

* Instead of directly deleting a file, we should delete file usages instead.

CommentFileSizeAuthor
#15 2952419-15.patch25.83 KBjrockowitz
#10 2952419-10.patch10.48 KBjrockowitz
#7 2952419-6.patch10.85 KBjrockowitz

Comments

KelvinWong created an issue. See original summary.

jrockowitz’s picture

jrockowitz commented

Makes sense to fix this. It is also worth noting, that the \Drupal\webform\WebformSubmissionStorage::delete() method also recursively deletes the 'webform/{webform_id}/{sid}' directory. This behavior probably needs to be removed.

jrockowitz’s picture

jrockowitz commented
Priority: Normal » Major
jrockowitz’s picture

jrockowitz commented

I just took a stab at addressing this and it is a lot more work than I expected. Still, I agree that the file usage should be deleted and not the actual file.

jrockowitz’s picture

jrockowitz commented

An unexpected issue I ran into is that we need account for files that have no remaining usages are no longer deleted by default.

I am not very comfortable with uploaded and then deleted webform submission files being orphaned and remaining indefinitely on the server. A webform might be collecting confidential information and when a submission is deleted all the files associated with that submission should to be deleted immediately.

I still think we need to check the file usage table and only delete webform submission upload files that are no longer being used.

  • jrockowitz committed 59dee6c on 2952419-file-delete 
    Issue #2952419: Attached files are deleted without usage checking
jrockowitz’s picture

jrockowitz commented
Status: Active » Needs review
StatusFileSize
new 2952419-6.patch10.85 KB

Status: Needs review » Needs work

The last submitted patch, 7: 2952419-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • jrockowitz committed 41129f8 on 2952419-file-delete 
    Issue #2952419: Attached files are deleted without usage checking
jrockowitz’s picture

jrockowitz commented
Status: Needs work » Needs review
StatusFileSize
new 2952419-10.patch10.48 KB

  • jrockowitz committed 47cbefd on 2952419-file-delete 
    Issue #2952419: Attached files are deleted without usage checking
keshavv’s picture

keshavv commented

I have tested #10 patch.
Patch will fix the entire problem.

  • jrockowitz committed 60d58b7 on 2952419-file-delete 
    Issue #2952419 by jrockowitz: Attached files are deleted without usage...

  • jrockowitz committed 91f6d4b on 2952419-file-delete 
    Issue #2952419 by jrockowitz: Attached files are deleted without usage...
jrockowitz’s picture

jrockowitz commented
StatusFileSize
new 2952419-15.patch25.83 KB

The attached patch adds additional test coverage for signature element's image.

  • jrockowitz committed 7621118 on 8.x-5.x 
    Issue #2952419 by jrockowitz, keshav.k: Attached files are deleted...
jrockowitz’s picture

jrockowitz commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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