Laravel-Backpack / CRUD

Build custom admin panels. Fast!
https://backpackforlaravel.com
MIT License
3k stars 880 forks source link

[Bug] Error when adding image to previously empty image subfield in repeatable field #5543

Closed eleven59 closed 2 weeks ago

eleven59 commented 2 weeks ago

Bug report

What I did

Added an image field in a repeatable field, and used withFiles for storage. I encounter a bug when I try to update an image if that image was previously empty.

Steps to reproduce:

  1. Create new entry
  2. Add repeatable item, but leave image field empty
  3. Save entry
  4. Edit entry
  5. Pick an image
  6. Save entry

This results in the following error: League\Flysystem\Filesystem::delete(): Argument #1 ($location) must be of type string, null given, called in vendor/laravel/framework/src/Illuminate/Filesystem/FilesystemAdapter.php on line 513

The issue, I think, is that uploadRepeatableFiles, in vendor/Backpack/CRUD/app/Library/Uploaders/SingleBase64Image.php does not check whether an image actually exists. I think this happens on line 56, which should be changed:

Now:

foreach ($imagesToDelete as $image) {
    Storage::disk($this->getDisk())->delete($image);
}

Should be:

foreach ($imagesToDelete as $image) {
    if(!empty($image)) {
        Storage::disk($this->getDisk())->delete($image);
    }
}

What I expected to happen

Image uploaded to correct folder, no errors thrown.

What happened

Error message, failed to delete non-existent old image

What I've already tried to fix it

Not much else to try, am I wrong?

Is it a bug in the latest version of Backpack?

After I run composer update backpack/crud the bug... is it still there? Yes.

Backpack, Laravel, PHP, DB version

When I run php artisan backpack:version the output is:

### PHP VERSION:
8.2.20

### PHP EXTENSIONS:
Core, date, libxml, openssl, pcre, sqlite3, zlib, bcmath, bz2, calendar, ctype, curl, dba, dom, hash, FFI, fileinfo, filter, ftp, gd, gmp, json, iconv, SPL, session, standard, mbstring, igbinary, imagick, imap, intl, ldap, exif, mysqlnd, mysqli, pcntl, PDO, pdo_mysql, pdo_pgsql, pdo_sqlite, pgsql, Phar, posix, random, readline, redis, Reflection, shmop, SimpleXML, soap, sockets, sodium, sysvmsg, sysvsem, sysvshm, tokenizer, xml, xmlreader, xmlwriter, xsl, zip, zstd, Zend OPcache

### LARAVEL VERSION:
11.11.1.0

### BACKPACK PACKAGE VERSIONS:
backpack/basset: 1.3.4
backpack/crud: 6.7.17
backpack/generators: v4.0.5
backpack/pro: 2.2.4
backpack/settings: 3.1.1
backpack/theme-tabler: 1.2.10
welcome[bot] commented 2 weeks ago

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use GitHub Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication channels:

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

-- Justin Case The Backpack Robot