Skip to content

Commit e5ed8e9

Browse files
authored
Merge pull request PHPOffice#3509 from PHPOffice/Stricter-Identify-Spreadsheet-Files-Only
Validate that OLE file contains a workbook object (ie. isn't a doc or a ppt file)
2 parents 937d5f7 + 9e384a1 commit e5ed8e9

File tree

4 files changed

+33
-3
lines changed

4 files changed

+33
-3
lines changed

src/PhpSpreadsheet/Reader/Xls.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ public function __construct()
430430
*/
431431
public function canRead(string $filename): bool
432432
{
433-
if (!File::testFileNoThrow($filename)) {
433+
if (File::testFileNoThrow($filename) === false) {
434434
return false;
435435
}
436436

@@ -440,6 +440,9 @@ public function canRead(string $filename): bool
440440

441441
// get excel data
442442
$ole->read($filename);
443+
if ($ole->wrkbook === null) {
444+
throw new Exception('The filename ' . $filename . ' is not recognised as a Spreadsheet file');
445+
}
443446

444447
return true;
445448
} catch (PhpSpreadsheetException $e) {
@@ -449,7 +452,7 @@ public function canRead(string $filename): bool
449452

450453
public function setCodepage(string $codepage): void
451454
{
452-
if (!CodePage::validate($codepage)) {
455+
if (CodePage::validate($codepage) === false) {
453456
throw new PhpSpreadsheetException('Unknown codepage: ' . $codepage);
454457
}
455458

src/PhpSpreadsheet/Shared/OLERead.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public function read(string $filename): void
134134

135135
$bbdBlocks = $this->numBigBlockDepotBlocks;
136136

137-
if ($this->numExtensionBlocks != 0) {
137+
if ($this->numExtensionBlocks !== 0) {
138138
$bbdBlocks = (self::BIG_BLOCK_SIZE - self::BIG_BLOCK_DEPOT_BLOCKS_POS) / 4;
139139
}
140140

tests/PhpSpreadsheetTests/IOFactoryTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,33 @@ public function providerIdentify(): array
108108
];
109109
}
110110

111+
public function testIdentifyInvalid(): void
112+
{
113+
$file = __DIR__ . '/../data/Reader/NotASpreadsheetFile.doc';
114+
115+
$this->expectException(ReaderException::class);
116+
$this->expectExceptionMessage('Unable to identify a reader for this file');
117+
IOFactory::identify($file);
118+
}
119+
120+
public function testCreateInvalid(): void
121+
{
122+
$file = __DIR__ . '/../data/Reader/NotASpreadsheetFile.doc';
123+
124+
$this->expectException(ReaderException::class);
125+
$this->expectExceptionMessage('Unable to identify a reader for this file');
126+
IOFactory::createReaderForFile($file);
127+
}
128+
129+
public function testLoadInvalid(): void
130+
{
131+
$file = __DIR__ . '/../data/Reader/NotASpreadsheetFile.doc';
132+
133+
$this->expectException(ReaderException::class);
134+
$this->expectExceptionMessage('Unable to identify a reader for this file');
135+
IOFactory::load($file);
136+
}
137+
111138
public function testFormatAsExpected(): void
112139
{
113140
$fileName = 'samples/templates/30template.xls';
27.5 KB
Binary file not shown.

0 commit comments

Comments
 (0)