Skip to content

Commit 33eefe7

Browse files
oleibmanoleibman
authored
Resolve Phpstan Messages - FINALE - Shared/OLE (#3364)
* Resolve Phpstan Messages - FINALE - Shared/OLE Reduce number of Phpstan messages by addressing their issues. This is the last of a series of related tickets to achieve that end. Only a handful of messages will be left in Phpstan baseline after this change is merged. Shared/Ole.php and Shared/Ole/ChainedBlockStream.php were completely uncovered in the test suite. So I wrote some tests and found they didn't work at all. There were 2 problems. The first was that ChainedBlockStream checks for a mode of `r`, but Ole invokes it with mode `rb`. ChainedBlockStream will now just check the first character of the mode. The second was a more interesting problem. Ole was reading "an *unsigned* long". However, what it should have been reading was "a 32-bit *signed* long". The irony of this mismatch is that, once the mode problem was corrected, 32-bit Php handled Ole and ChainedBlockStream correctly, but 64-bit Php did not. The code is corrected to work properly for both 32- and 64-bit. * Scrutinizer 2 dead assignments - sufficient to just make calls which are expected to fail.
1 parent 28fb962 commit 33eefe7

File tree

8 files changed

+126
-222
lines changed

8 files changed

+126
-222
lines changed

‎phpstan-baseline.neon‎

Lines changed: 0 additions & 175 deletions
Original file line numberDiff line numberDiff line change
@@ -24,178 +24,3 @@ parameters:
2424
message: "#^Strict comparison using \\=\\=\\= between PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\FormulaToken and null will always evaluate to false\\.$#"
2525
count: 1
2626
path: src/PhpSpreadsheet/Calculation/FormulaParser.php
27-
28-
-
29-
message: "#^Cannot access offset 1 on array\\|false\\.$#"
30-
count: 1
31-
path: src/PhpSpreadsheet/Shared/OLE.php
32-
33-
-
34-
message: "#^Cannot access offset 2 on array\\|false\\.$#"
35-
count: 1
36-
path: src/PhpSpreadsheet/Shared/OLE.php
37-
38-
-
39-
message: "#^Cannot access offset 3 on array\\|false\\.$#"
40-
count: 1
41-
path: src/PhpSpreadsheet/Shared/OLE.php
42-
43-
-
44-
message: "#^Cannot access offset 4 on array\\|false\\.$#"
45-
count: 1
46-
path: src/PhpSpreadsheet/Shared/OLE.php
47-
48-
-
49-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\:\\:getData\\(\\) should return string but returns string\\|false\\.$#"
50-
count: 1
51-
path: src/PhpSpreadsheet/Shared/OLE.php
52-
53-
-
54-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\:\\:getStream\\(\\) should return resource but returns resource\\|false\\.$#"
55-
count: 1
56-
path: src/PhpSpreadsheet/Shared/OLE.php
57-
58-
-
59-
message: "#^Parameter \\#1 \\$No of class PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS constructor expects int, null given\\.$#"
60-
count: 1
61-
path: src/PhpSpreadsheet/Shared/OLE.php
62-
63-
-
64-
message: "#^Parameter \\#1 \\$oleTimestamp of static method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\:\\:OLE2LocalDate\\(\\) expects string, string\\|false given\\.$#"
65-
count: 2
66-
path: src/PhpSpreadsheet/Shared/OLE.php
67-
68-
-
69-
message: "#^Parameter \\#1 \\$string of function substr expects string, string\\|false given\\.$#"
70-
count: 1
71-
path: src/PhpSpreadsheet/Shared/OLE.php
72-
73-
-
74-
message: "#^Parameter \\#2 \\$name of class PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS constructor expects string, null given\\.$#"
75-
count: 1
76-
path: src/PhpSpreadsheet/Shared/OLE.php
77-
78-
-
79-
message: "#^Parameter \\#3 \\$type of class PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS constructor expects int, null given\\.$#"
80-
count: 1
81-
path: src/PhpSpreadsheet/Shared/OLE.php
82-
83-
-
84-
message: "#^Parameter \\#4 \\$prev of class PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS constructor expects int, null given\\.$#"
85-
count: 1
86-
path: src/PhpSpreadsheet/Shared/OLE.php
87-
88-
-
89-
message: "#^Parameter \\#5 \\$next of class PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS constructor expects int, null given\\.$#"
90-
count: 1
91-
path: src/PhpSpreadsheet/Shared/OLE.php
92-
93-
-
94-
message: "#^Parameter \\#6 \\$dir of class PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS constructor expects int, null given\\.$#"
95-
count: 1
96-
path: src/PhpSpreadsheet/Shared/OLE.php
97-
98-
-
99-
message: "#^Parameter \\#9 \\$data of class PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS constructor expects string, null given\\.$#"
100-
count: 1
101-
path: src/PhpSpreadsheet/Shared/OLE.php
102-
103-
-
104-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\:\\:\\$root \\(PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\\\Root\\) in isset\\(\\) is not nullable\\.$#"
105-
count: 1
106-
path: src/PhpSpreadsheet/Shared/OLE.php
107-
108-
-
109-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:\\$_data \\(string\\) in isset\\(\\) is not nullable\\.$#"
110-
count: 1
111-
path: src/PhpSpreadsheet/Shared/OLE/PPS.php
112-
113-
-
114-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:\\$startBlock \\(int\\) on left side of \\?\\? is not nullable\\.$#"
115-
count: 1
116-
path: src/PhpSpreadsheet/Shared/OLE/PPS.php
117-
118-
-
119-
message: "#^Parameter \\#1 \\$No of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects int, null given\\.$#"
120-
count: 1
121-
path: src/PhpSpreadsheet/Shared/OLE/PPS/File.php
122-
123-
-
124-
message: "#^Parameter \\#4 \\$prev of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects int, null given\\.$#"
125-
count: 1
126-
path: src/PhpSpreadsheet/Shared/OLE/PPS/File.php
127-
128-
-
129-
message: "#^Parameter \\#5 \\$next of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects int, null given\\.$#"
130-
count: 1
131-
path: src/PhpSpreadsheet/Shared/OLE/PPS/File.php
132-
133-
-
134-
message: "#^Parameter \\#6 \\$dir of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects int, null given\\.$#"
135-
count: 1
136-
path: src/PhpSpreadsheet/Shared/OLE/PPS/File.php
137-
138-
-
139-
message: "#^Parameter \\#1 \\$No of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects int, null given\\.$#"
140-
count: 1
141-
path: src/PhpSpreadsheet/Shared/OLE/PPS/Root.php
142-
143-
-
144-
message: "#^Parameter \\#4 \\$prev of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects int, null given\\.$#"
145-
count: 1
146-
path: src/PhpSpreadsheet/Shared/OLE/PPS/Root.php
147-
148-
-
149-
message: "#^Parameter \\#5 \\$next of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects int, null given\\.$#"
150-
count: 1
151-
path: src/PhpSpreadsheet/Shared/OLE/PPS/Root.php
152-
153-
-
154-
message: "#^Parameter \\#6 \\$dir of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects int, null given\\.$#"
155-
count: 1
156-
path: src/PhpSpreadsheet/Shared/OLE/PPS/Root.php
157-
158-
-
159-
message: "#^Parameter \\#9 \\$data of method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\:\\:__construct\\(\\) expects string, null given\\.$#"
160-
count: 1
161-
path: src/PhpSpreadsheet/Shared/OLE/PPS/Root.php
162-
163-
-
164-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\\\Root\\:\\:\\$bigBlockSize \\(int\\) in isset\\(\\) is not nullable\\.$#"
165-
count: 1
166-
path: src/PhpSpreadsheet/Shared/OLE/PPS/Root.php
167-
168-
-
169-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLE\\\\PPS\\\\Root\\:\\:\\$smallBlockSize \\(int\\) in isset\\(\\) is not nullable\\.$#"
170-
count: 1
171-
path: src/PhpSpreadsheet/Shared/OLE/PPS/Root.php
172-
173-
-
174-
message: "#^Parameter \\#1 \\$data of static method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLERead\\:\\:getInt4d\\(\\) expects string, string\\|false given\\.$#"
175-
count: 8
176-
path: src/PhpSpreadsheet/Shared/OLERead.php
177-
178-
-
179-
message: "#^Parameter \\#1 \\$string of function substr expects string, string\\|false given\\.$#"
180-
count: 2
181-
path: src/PhpSpreadsheet/Shared/OLERead.php
182-
183-
-
184-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLERead\\:\\:\\$data has no type specified\\.$#"
185-
count: 1
186-
path: src/PhpSpreadsheet/Shared/OLERead.php
187-
188-
-
189-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLERead\\:\\:\\$documentSummaryInformation has no type specified\\.$#"
190-
count: 1
191-
path: src/PhpSpreadsheet/Shared/OLERead.php
192-
193-
-
194-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLERead\\:\\:\\$summaryInformation has no type specified\\.$#"
195-
count: 1
196-
path: src/PhpSpreadsheet/Shared/OLERead.php
197-
198-
-
199-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\OLERead\\:\\:\\$wrkbook has no type specified\\.$#"
200-
count: 1
201-
path: src/PhpSpreadsheet/Shared/OLERead.php

‎src/PhpSpreadsheet/Shared/OLE.php‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ class OLE
116116
*/
117117
public function read($filename)
118118
{
119-
$fh = fopen($filename, 'rb');
119+
$fh = @fopen($filename, 'rb');
120120
if ($fh === false) {
121121
throw new ReaderException("Can't open file $filename");
122122
}
@@ -239,7 +239,12 @@ public function getStream($blockIdOrPps)
239239
$path .= '&blockId=' . $blockIdOrPps;
240240
}
241241

242-
return fopen($path, 'rb');
242+
$resource = fopen($path, 'rb');
243+
if ($resource === false) {
244+
throw new Exception("Unable to open stream $path");
245+
}
246+
247+
return $resource;
243248
}
244249

245250
/**
@@ -251,8 +256,7 @@ public function getStream($blockIdOrPps)
251256
*/
252257
private static function readInt1($fileHandle)
253258
{
254-
// @phpstan-ignore-next-line
255-
[, $tmp] = unpack('c', fread($fileHandle, 1));
259+
[, $tmp] = unpack('c', fread($fileHandle, 1) ?: '') ?: [0, 0];
256260

257261
return $tmp;
258262
}
@@ -266,23 +270,28 @@ private static function readInt1($fileHandle)
266270
*/
267271
private static function readInt2($fileHandle)
268272
{
269-
// @phpstan-ignore-next-line
270-
[, $tmp] = unpack('v', fread($fileHandle, 2));
273+
[, $tmp] = unpack('v', fread($fileHandle, 2) ?: '') ?: [0, 0];
271274

272275
return $tmp;
273276
}
274277

278+
private const SIGNED_4OCTET_LIMIT = 2147483648;
279+
280+
private const SIGNED_4OCTET_SUBTRACT = 2 * self::SIGNED_4OCTET_LIMIT;
281+
275282
/**
276-
* Reads an unsigned long (4 octets).
283+
* Reads long (4 octets), interpreted as if signed on 32-bit system.
277284
*
278285
* @param resource $fileHandle file handle
279286
*
280287
* @return int
281288
*/
282289
private static function readInt4($fileHandle)
283290
{
284-
// @phpstan-ignore-next-line
285-
[, $tmp] = unpack('V', fread($fileHandle, 4));
291+
[, $tmp] = unpack('V', fread($fileHandle, 4) ?: '') ?: [0, 0];
292+
if ($tmp >= self::SIGNED_4OCTET_LIMIT) {
293+
$tmp -= self::SIGNED_4OCTET_SUBTRACT;
294+
}
286295

287296
return $tmp;
288297
}
@@ -300,7 +309,7 @@ public function readPpsWks($blockId)
300309
$fh = $this->getStream($blockId);
301310
for ($pos = 0; true; $pos += 128) {
302311
fseek($fh, $pos, SEEK_SET);
303-
$nameUtf16 = fread($fh, 64);
312+
$nameUtf16 = (string) fread($fh, 64);
304313
$nameLength = self::readInt2($fh);
305314
$nameUtf16 = substr($nameUtf16, 0, $nameLength - 2);
306315
// Simple conversion from UTF-16LE to ISO-8859-1
@@ -330,15 +339,15 @@ public function readPpsWks($blockId)
330339
$pps->NextPps = self::readInt4($fh);
331340
$pps->DirPps = self::readInt4($fh);
332341
fseek($fh, 20, SEEK_CUR);
333-
$pps->Time1st = self::OLE2LocalDate(fread($fh, 8));
334-
$pps->Time2nd = self::OLE2LocalDate(fread($fh, 8));
342+
$pps->Time1st = self::OLE2LocalDate((string) fread($fh, 8));
343+
$pps->Time2nd = self::OLE2LocalDate((string) fread($fh, 8));
335344
$pps->startBlock = self::readInt4($fh);
336345
$pps->Size = self::readInt4($fh);
337346
$pps->No = count($this->_list);
338347
$this->_list[] = $pps;
339348

340349
// check if the PPS tree (starting from root) is complete
341-
if (isset($this->root) && $this->ppsTreeComplete($this->root->No)) {
350+
if (isset($this->root) && $this->ppsTreeComplete($this->root->No)) { //* @phpstan-ignore-line
342351
break;
343352
}
344353
}
@@ -448,7 +457,7 @@ public function getData($index, $position, $length)
448457
return '';
449458
}
450459
$fh = $this->getStream($this->_list[$index]);
451-
$data = stream_get_contents($fh, $length, $position);
460+
$data = (string) stream_get_contents($fh, $length, $position);
452461
fclose($fh);
453462

454463
return $data;
@@ -540,7 +549,7 @@ public static function OLE2LocalDate($oleTimestamp)
540549
}
541550

542551
// convert to units of 100 ns since 1601:
543-
$unpackedTimestamp = unpack('v4', $oleTimestamp);
552+
$unpackedTimestamp = unpack('v4', $oleTimestamp) ?: [];
544553
$timestampHigh = (float) $unpackedTimestamp[4] * 65536 + (float) $unpackedTimestamp[3];
545554
$timestampLow = (float) $unpackedTimestamp[2] * 65536 + (float) $unpackedTimestamp[1];
546555

‎src/PhpSpreadsheet/Shared/OLE/ChainedBlockStream.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class ChainedBlockStream
4848
*/
4949
public function stream_open($path, $mode, $options, &$openedPath) // @codingStandardsIgnoreLine
5050
{
51-
if ($mode != 'r') {
51+
if ($mode[0] !== 'r') {
5252
if ($options & STREAM_REPORT_ERRORS) {
5353
trigger_error('Only reading is supported', E_USER_WARNING);
5454
}
@@ -119,7 +119,7 @@ public function stream_read($count) // @codingStandardsIgnoreLine
119119
if ($this->stream_eof()) {
120120
return false;
121121
}
122-
$s = substr($this->data, $this->pos, $count);
122+
$s = substr($this->data, (int) $this->pos, $count);
123123
$this->pos += $count;
124124

125125
return $s;

‎src/PhpSpreadsheet/Shared/OLE/PPS.php‎

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class PPS
8888
/**
8989
* Starting block (small or big) for this PPS's data inside the container.
9090
*
91-
* @var int
91+
* @var ?int
9292
*/
9393
public $startBlock;
9494

@@ -104,7 +104,7 @@ class PPS
104104
*
105105
* @var string
106106
*/
107-
public $_data;
107+
public $_data = '';
108108

109109
/**
110110
* Array of child PPS's (only used by Root and Dir PPS's).
@@ -123,34 +123,30 @@ class PPS
123123
/**
124124
* The constructor.
125125
*
126-
* @param int $No The PPS index
127-
* @param string $name The PPS name
128-
* @param int $type The PPS type. Dir, Root or File
129-
* @param int $prev The index of the previous PPS
130-
* @param int $next The index of the next PPS
131-
* @param int $dir The index of it's first child if this is a Dir or Root PPS
126+
* @param ?int $No The PPS index
127+
* @param ?string $name The PPS name
128+
* @param ?int $type The PPS type. Dir, Root or File
129+
* @param ?int $prev The index of the previous PPS
130+
* @param ?int $next The index of the next PPS
131+
* @param ?int $dir The index of it's first child if this is a Dir or Root PPS
132132
* @param null|float|int $time_1st A timestamp
133133
* @param null|float|int $time_2nd A timestamp
134-
* @param string $data The (usually binary) source data of the PPS
134+
* @param ?string $data The (usually binary) source data of the PPS
135135
* @param array $children Array containing children PPS for this PPS
136136
*/
137137
public function __construct($No, $name, $type, $prev, $next, $dir, $time_1st, $time_2nd, $data, $children)
138138
{
139-
$this->No = $No;
140-
$this->Name = $name;
141-
$this->Type = $type;
142-
$this->PrevPps = $prev;
143-
$this->NextPps = $next;
144-
$this->DirPps = $dir;
139+
$this->No = (int) $No;
140+
$this->Name = (string) $name;
141+
$this->Type = (int) $type;
142+
$this->PrevPps = (int) $prev;
143+
$this->NextPps = (int) $next;
144+
$this->DirPps = (int) $dir;
145145
$this->Time1st = $time_1st ?? 0;
146146
$this->Time2nd = $time_2nd ?? 0;
147-
$this->_data = $data;
147+
$this->_data = (string) $data;
148148
$this->children = $children;
149-
if ($data != '') {
150-
$this->Size = strlen($data);
151-
} else {
152-
$this->Size = 0;
153-
}
149+
$this->Size = strlen((string) $data);
154150
}
155151

156152
/**
@@ -160,9 +156,9 @@ public function __construct($No, $name, $type, $prev, $next, $dir, $time_1st, $t
160156
*/
161157
public function getDataLen()
162158
{
163-
if (!isset($this->_data)) {
164-
return 0;
165-
}
159+
//if (!isset($this->_data)) {
160+
// return 0;
161+
//}
166162

167163
return strlen($this->_data);
168164
}

0 commit comments

Comments
 (0)