Unchecked return value of ZipArchive::locateName

[anton-harvey]

This is:

• a bug report
• a feature request
• not a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

The Reader\Xlsx::getFromZipArchive function should return false if the zip entry could not be located.

What is the current behavior?

If it exists, the contents of the zip entry at index 0 is returned.

What are the steps to reproduce?

Provided upon (further) request. 😅

Which versions of PhpSpreadsheet and PHP are affected?

All PhpSpreadsheet versions. Tested on PHP 7.0.17.

---

ZipArchive::locateName

Returns the index of the entry on success or FALSE on failure.

The Reader\Xlsx::getFromZipArchive function does not check whether the return value of ZipArchive::locateName is false. When it is, it's passed straight into ZipArchive::getFromIndex, which casts it to an integer, resulting in it retrieving the entry at index 0.

PhpSpreadsheet/src/PhpSpreadsheet/Reader/Xlsx.php

Lines 304 to 312 in 25ff914

	// Apache POI fixes
	$contents = $archive->getFromIndex(
	$archive->locateName($fileName, ZipArchive::FL_NOCASE)
	);
	if ($contents === false) {
	$contents = $archive->getFromIndex(
	$archive->locateName(substr($fileName, 1), ZipArchive::FL_NOCASE)
	);
	}

ZipArchive::getFromName appears to be a good alternative:

string ZipArchive::getFromName ( string $name [, int $length = 0 [, int $flags ]] )

Returns the entry contents using its name.
Returns the contents of the entry on success or FALSE on failure.

$contents = $archive->getFromName($fileName, 0, ZipArchive::FL_NOCASE);

[PowerKiKi]

That sounds about right. Would you mind providing a PR for that ?

[anton-harvey]

I'll make a PR in a few days.