Skip to content

Handle zips with Windoze directory separators (\) rather than the standard linux (/) #3482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/PhpSpreadsheet/Reader/Xlsx.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ public function listWorksheetInfo($filename)
$xmlWorkbook = $this->loadZip($relTarget, $mainNS);
if ($xmlWorkbook->sheets) {
$dir = dirname($relTarget);

/** @var SimpleXMLElement $eleSheet */
foreach ($xmlWorkbook->sheets->sheet as $eleSheet) {
$tmpInfo = [
Expand All @@ -267,8 +268,8 @@ public function listWorksheetInfo($filename)

$xml = new XMLReader();
$xml->xml(
$this->getSecurityScannerOrThrow()->scanFile(
'zip://' . File::realpath($filename) . '#' . $fileWorksheetPath
$this->getSecurityScannerOrThrow()->scan(
$this->getFromZipArchive($this->zip, $fileWorksheetPath)
),
null,
Settings::getLibXmlLoaderOptions()
Expand Down Expand Up @@ -403,12 +404,18 @@ private function getFromZipArchive(ZipArchive $archive, $fileName = '')
// Sadly, some 3rd party xlsx generators don't use consistent case for filenaming
// so we need to load case-insensitively from the zip file

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

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

// Has the file been saved with Windoze directory separators rather than unix?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Has the file been saved with Windoze directory separators rather than unix?
// Has the file been saved with Windows directory separators rather than unix?

if ($contents === false) {
$contents = $archive->getFromName(str_replace('/', '\\', $fileName), 0, ZipArchive::FL_NOCASE);
}

return ($contents === false) ? '' : $contents;
}

Expand Down
16 changes: 14 additions & 2 deletions src/PhpSpreadsheet/Shared/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,11 @@ public static function assertFile(string $filename, string $zipMember = ''): voi
if ($zipMember !== '') {
$zipfile = "zip://$filename#$zipMember";
if (!self::fileExists($zipfile)) {
throw new ReaderException("Could not find zip member $zipfile");
// Has the file been saved with Windoze directory separators rather than unix?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Has the file been saved with Windoze directory separators rather than unix?
// Has the file been saved with Windows directory separators rather than unix?

$zipfile = "zip://$filename#" . str_replace('/', '\\', $zipMember);
if (!self::fileExists($zipfile)) {
throw new ReaderException("Could not find zip member $zipfile");
}
}
}
}
Expand All @@ -180,6 +184,14 @@ public static function testFileNoThrow(string $filename, ?string $zipMember = nu
return self::validateZipFirst4($filename);
}

return self::fileExists("zip://$filename#$zipMember");
$zipfile = "zip://$filename#$zipMember";
if (self::fileExists($zipfile)) {
return true;
}

// Has the file been saved with Windoze directory separators rather than unix?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Has the file been saved with Windoze directory separators rather than unix?
// Has the file been saved with Windows directory separators rather than unix?

$zipfile = "zip://$filename#" . str_replace('/', '\\', $zipMember);

return self::fileExists($zipfile);
}
}
71 changes: 71 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/Xlsx/DirectorySeparatorTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;

use PhpOffice\PhpSpreadsheet\IOFactory;
use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
use PHPUnit\Framework\TestCase;

class DirectorySeparatorTest extends TestCase
{
/**
* @dataProvider providerDirectorySeparator
*/
public function testDirectorySeparatorIdentify(string $fileName): void
{
$filename = "tests/data/Reader/XLSX/{$fileName}";
$reader = IOFactory::identify($filename);

self::assertSame('Xlsx', $reader);
}

/**
* @dataProvider providerDirectorySeparator
*/
public function testDirectorySeparatorWorksheetNames(string $fileName): void
{
$filename = "tests/data/Reader/XLSX/{$fileName}";
$reader = new Xlsx();
$sheetList = $reader->listWorksheetNames($filename);

self::assertCount(1, $sheetList);
self::assertSame('Sheet', $sheetList[0]);
}

/**
* @dataProvider providerDirectorySeparator
*/
public function testDirectorySeparatorWorksheetInfo(string $fileName): void
{
$filename = "tests/data/Reader/XLSX/{$fileName}";
$reader = new Xlsx();
$sheetData = $reader->listWorksheetInfo($filename);

self::assertCount(1, $sheetData);
self::assertSame('Sheet', $sheetData[0]['worksheetName']);
self::assertSame(3, (int) $sheetData[0]['totalRows']);
self::assertSame(21, (int) $sheetData[0]['totalColumns']);
}

/**
* @dataProvider providerDirectorySeparator
*/
public function testDirectorySeparatorLoad(string $fileName): void
{
$filename = "tests/data/Reader/XLSX/{$fileName}";
$reader = new Xlsx();
$spreadsheet = $reader->load($filename);

$cellValue = $spreadsheet->getActiveSheet()->getCell('A1')->getValue();

self::assertSame('Key ID', $cellValue);
}

public function providerDirectorySeparator(): array
{
return [
['Zip-Linux-Directory-Separator.xlsx'],
['Zip-Windows-Directory-Separator.xlsx'],
];
}
}
Binary file not shown.
Binary file not shown.