Bag::loadTagManifests() leaves $this->tagManifests uninitialized if bag files count is 0

[zozlak]

If the early-return condition of Bag::loadTagManifest() is met, the $this->tagManifests property is left uninitialized leading to the Undefined property: whikloj\BagItTools\Bag::$tagManifests error, e.g. on a $bag->validate() call (because of this line).

A solution would be to set $this->tagManifests before returning, e.g.:

    private function loadTagManifests(): bool
    {
        $tagManifests = [];
        $pattern = $this->getBagRoot() . "/tagmanifest-*.txt";
        $files = BagUtils::findAllByPattern($pattern);
        if (count($files) < 1) {
            # ADDED LINE:
            $this->tagManifests = $tagManifests;
            return false;
        }
        (...)
    }

[whikloj]

Hi @zozlak, could you give me an example of what you did to hit this particular situation? I'm just wondering if I'm missing something elsewhere.

[zozlak]

$bag   = whikloj\BagItTools\Bag::load('TestFilesBag.tgz');
$bag->isValid();

The Undefined property: whikloj\BagItTools\Bag::$tagManifests error is thrown by the second line. You can download the test bag file from https://github.com/acdh-oeaw/repo-file-checker/blob/master/_testFiles/BagItTest/bagit/TestFilesBag.tgz

[whikloj]

Thanks for bringing this up @zozlak, this should resolve the issue. Let me know if it's good for you.

[zozlak]

Here you unset the $this->tagManifests and I would expect running is_array($this->tagManifests) here on an unset property will end up with a Undefined property: Bag::$tagManifests, wouldn't it?

But

if (count($this->tagManifests ?? []) > 0) {
    (...)
}

should do the job.

[whikloj]

It's the call to $this->ensureTagManifests(); in loadTagManifests (line 1641 of Bag.php) that ensures it is setup.

[zozlak]

Right!

I can confirm no error is thrown now.

[zozlak]

Could you please make a release including this patch?

[whikloj]

Sorry I was trying to think of anything else I might want to do but then dropped it. I have pushed out 4.2.3