eleven59 commented 1 month ago

Bug report

What I did

I included an image field in a repeatable element.
I add a repeatable entry and save the page once without adding an image to its subfield, all is fine.
I then going back in to edit and select an image for that same subfield
The upload handler tries to delete the old (nonexistent) image, which throws an error (see below)

What I expected to happen

I expected that the entry would just be saved with the new image, and the old one only getting

What happened

This throws the following error:

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

It has to do with this function in Backpack \ CRUD \ app \ Library \ Uploaders \ SingleBase64Image:

public function uploadRepeatableFiles($values, $previousRepeatableValues, $entry = null)
{
    foreach ($values as $row => $rowValue) {
        if ($rowValue) {
            if (Str::startsWith($rowValue, 'data:image')) {
                $base64Image = Str::after($rowValue, ';base64,');
                $finalPath = $this->getPath().$this->getFileName($rowValue);
                Storage::disk($this->getDisk())->put($finalPath, base64_decode($base64Image));
                $values[$row] = $previousRepeatableValues[] = $finalPath;
                continue;
            }
        }
    }

    $imagesToDelete = array_diff($previousRepeatableValues, $values);

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

    return $values;
}

As you can see this does not check anywhere whether an image actually exists in the spot where it tries to delete it.

What I've already tried to fix it

Updated all backpack packages, made a completely new model with repeatable field that only has an image. Repeated the steps under "What I did". The error remains.

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.3.10

### 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.21.0.0

### BACKPACK PACKAGE VERSIONS:
backpack/basset: 1.3.5
backpack/crud: 6.7.30
backpack/generators: v4.0.5
backpack/pro: 2.2.14
backpack/settings: 3.1.1
backpack/theme-tabler: 1.2.11

karandatwani92 commented 1 month ago

Hey @eleven59

Thanks for pointing out the issue. We are already working on major refactoring of the Backpack Uploader to cover this and other uploader-related problems. It's a BIG PR, which may take some time. Sorry for the inconvenience.

You can subscribe to our newsletter for the Backpack Progress report and other updates.

eleven59 commented 1 month ago

Hi @karandatwani92,

That sounds like a great idea. I've fixed it by overwriting the uploader for now, so no rush in that regard for me. I fully trust you'll make something much better than my hacky solution here and will happily update my project whenever you're done with that.

Thanks for the quick reply, too!

Laravel-Backpack / CRUD

[Bug] error on replacing repeatable subfield image when first saving it empty #5631