Problem/Motivation

When a file is missing on the file system, it crashes when moving the file and the batch process stops.

Steps to reproduce

  1. Manually delete a file from the files folder
  2. Start the move process
  3. The process will throw an error and stop

There are also significant bugs in the original filefield_paths_filefield_paths_process_file() code.

It gets $temporary_scheme_name and $source_scheme_name incorrectly, causing the file move/rename not to happen at all in some cases.

This is fixed by using StreamWrapperManager::getScheme(). See this commit on the MR.

Proposed resolution

Adding a try-catch around the move process, fix the code to correctly get the schema names.

Remaining tasks

Review the MR

CommentFileSizeAuthor
prevent-crashing.patch3.61 KBdaou
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

daou created an issue. See original summary.

solideogloria’s picture

Status: Needs review » Needs work

The patch changes other things besides adding a try-catch. It removed the prepareDirectory function from the if check, which shouldn't have been changed. It doesn't throw an exception on failure; it returns false. So having two try statements isn't helpful.

solideogloria’s picture

Status: Needs work » Needs review
solideogloria’s picture

The test failures appear to be unrelated. I reran the tests for the previously submitted patch file, which had all tests passing previously, but now tests fail. I think this means the branch's tests are failing, and I can't figure out what is causing them.

solideogloria’s picture

Title: Prevent crashing while file move, when file is not on the fileserver » Prevent crashing on file move when file is not found
solideogloria’s picture

Title: Prevent crashing on file move when file is not found » Prevent crashing on file move when file is not found, fix bugs
Category: Feature request » Bug report
Priority: Normal » Major

I found significant bugs in the original filefield_paths_filefield_paths_process_file() code.

It gets $temporary_scheme_name and $source_scheme_name incorrectly, causing the file move/rename not to happen at all in some cases.

This is fixed by using StreamWrapperManager::getScheme(). See the latest commit on the MR.

solideogloria’s picture

Title: Prevent crashing on file move when file is not found, fix bugs » Prevent crashing on file move when file is not found, fix schema bugs
solideogloria’s picture

Issue summary: View changes
solideogloria’s picture

solideogloria’s picture

Hmm, the test that fails succeeded in PHP 7.4 with Drupal 9.5.

voleger’s picture

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

Needs rebase.

solideogloria’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
voleger’s picture

Status: Needs review » Needs work

Thanks. Changes look good, but two PHPCS issues must be addressed before merging.

solideogloria’s picture

Status: Needs work » Needs review

  • voleger committed fe25ee47 on 8.x-1.x authored by solideogloria
    Issue #3403246 by solideogloria, daou, voleger: Prevent crashing on file...
voleger’s picture

Status: Needs review » Fixed

Thanks

Status: Fixed » Closed (fixed)

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