Skip to content

Commit 4c24f17

Browse files
axiaccmb69
axiac
authored and
cmb69
committed
Fix bug #55701: GlobIterator throws LogicException
GlobIterator throws LogicException with message 'The parent constructor was not called' on its first operation when the glob expression doesn't match any file. It also throws on the first operation after the iteration completes, when the glob expression matches some files. # Resolved conflicts: # ext/spl/spl_directory.c
1 parent 08777e9 commit 4c24f17

File tree

3 files changed

+341
-1
lines changed

3 files changed

+341
-1
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ PHP NEWS
3030
. Fixed bug #72336 (openssl_pkey_new does not fail for invalid DSA params).
3131
(Jakub Zelenka)
3232

33+
- SPL:
34+
. Fixed bug #55701 (GlobIterator throws LogicException). (Valentin VĂLCIU)
35+
3336
- SQLite3:
3437
. Fixed bug #70628 (Clearing bindings on an SQLite3 statement doesn't work).
3538
(cmb)

‎ext/spl/spl_directory.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,7 @@ zend_function *spl_filesystem_object_get_method_check(zval **object_ptr, char *m
666666
{
667667
spl_filesystem_object *fsobj = zend_object_store_get_object(*object_ptr TSRMLS_CC);
668668

669-
if (fsobj->u.dir.entry.d_name[0] == '\0' && fsobj->orig_path == NULL) {
669+
if (fsobj->u.dir.dirp == NULL && fsobj->orig_path == NULL) {
670670
method = "_bad_state_ex";
671671
method_len = sizeof("_bad_state_ex") - 1;
672672
key = NULL;

‎ext/spl/tests/bug55701.phpt

Lines changed: 337 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,337 @@
1+
--TEST--
2+
Bug #55701 (GlobIterator throws LogicException with message 'The parent constructor was not called')
3+
--FILE--
4+
<?php
5+
6+
//
7+
// Some methods of GlobIterator do not throw a RuntimeException when the glob pattern doesn't match any file.
8+
// Most methods of GlobIterator throw a RuntimeException when the glob pattern does't match any file
9+
// because they get the properties of the current file
10+
function testBaseClass($f) {
11+
// The tested iterator is in an invalid state; the behaviour of most of its methods is undefined
12+
try {
13+
$f();
14+
echo "ran normally (expected)\n";
15+
} catch (RuntimeException $e) {
16+
// Throwing a RuntimeException is the correct behaviour for some methods
17+
echo "ran normally (expected)\n";
18+
} catch (LogicException $e) {
19+
// Throwing a LogicException is not correct
20+
echo "threw LogicException (unexpected)\n";
21+
}
22+
}
23+
24+
//
25+
// The derived classes must throw LogicException if the parent class constructor was not called
26+
function testChildClass($f) {
27+
try {
28+
$f();
29+
echo "didn't throw (unexpected)\n";
30+
} catch (LogicException $e) {
31+
echo "threw LogicException (expected)\n";
32+
} catch (Exception $e) {
33+
echo "threw other exception (unexpected)\n";
34+
}
35+
}
36+
37+
38+
39+
//
40+
// It must not throw LogicException when the iterator is not valid
41+
echo "->count()... ";
42+
testBaseClass( function() {
43+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
44+
$o->count();
45+
} );
46+
47+
echo "->rewind()... ";
48+
testBaseClass( function() {
49+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
50+
$o->rewind();
51+
} );
52+
53+
echo "->getFlags()... ";
54+
testBaseClass( function() {
55+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
56+
$o->getFlags();
57+
} );
58+
59+
echo "->setFlags()... ";
60+
testBaseClass( function() {
61+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
62+
$o->setFlags(FilesystemIterator::KEY_AS_PATHNAME);
63+
} );
64+
65+
echo "->valid()... ";
66+
testBaseClass( function() {
67+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
68+
$o->valid();
69+
} );
70+
71+
72+
73+
//
74+
// When the iterator is not valid, the behaviour of the next methods is undefined
75+
// Some of them throw a RuntimeException while others just return an invalid value
76+
// However, they must not throw LogicException
77+
echo "->current()... ";
78+
testBaseClass( function() {
79+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
80+
$o->current();
81+
} );
82+
83+
echo "->key()... ";
84+
testBaseClass( function() {
85+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
86+
$o->key();
87+
} );
88+
89+
echo "->next()... ";
90+
testBaseClass( function() {
91+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
92+
$o->next();
93+
} );
94+
95+
echo "->getATime()... ";
96+
testBaseClass( function() {
97+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
98+
$o->getATime();
99+
} );
100+
101+
echo "->getBasename()... ";
102+
testBaseClass( function() {
103+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
104+
$o->getBasename();
105+
} );
106+
107+
echo "->getCTime()... ";
108+
testBaseClass( function() {
109+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
110+
$o->getCTime();
111+
} );
112+
113+
echo "->getExtension()... ";
114+
testBaseClass( function() {
115+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
116+
$o->getExtension();
117+
} );
118+
119+
echo "->getFilename()... ";
120+
testBaseClass( function() {
121+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
122+
$o->getFilename();
123+
} );
124+
125+
echo "->getGroup()... ";
126+
testBaseClass( function() {
127+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
128+
$o->getGroup();
129+
} );
130+
131+
echo "->getInode()... ";
132+
testBaseClass( function() {
133+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
134+
$o->getInode();
135+
} );
136+
137+
echo "->getMTime()... ";
138+
testBaseClass( function() {
139+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
140+
$o->getMTime();
141+
} );
142+
143+
echo "->getOwner()... ";
144+
testBaseClass( function() {
145+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
146+
$o->getOwner();
147+
} );
148+
149+
echo "->getPath()... ";
150+
testBaseClass( function() {
151+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
152+
$o->getPath();
153+
} );
154+
155+
echo "->getPathname()... ";
156+
testBaseClass( function() {
157+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
158+
$o->getPathname();
159+
} );
160+
161+
echo "->getPerms()... ";
162+
testBaseClass( function() {
163+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
164+
$o->getPerms();
165+
} );
166+
167+
echo "->getSize()... ";
168+
testBaseClass( function() {
169+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
170+
$o->getSize();
171+
} );
172+
173+
echo "->getType()... ";
174+
testBaseClass( function() {
175+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
176+
$o->getType();
177+
} );
178+
179+
echo "->isDir()... ";
180+
testBaseClass( function() {
181+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
182+
$o->isDir();
183+
} );
184+
185+
echo "->isDot()... ";
186+
testBaseClass( function() {
187+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
188+
$o->isDot();
189+
} );
190+
191+
echo "->isExecutable()... ";
192+
testBaseClass( function() {
193+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
194+
$o->isExecutable();
195+
} );
196+
197+
echo "->isFile()... ";
198+
testBaseClass( function() {
199+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
200+
$o->isFile();
201+
} );
202+
203+
echo "->isLink()... ";
204+
testBaseClass( function() {
205+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
206+
$o->isLink();
207+
} );
208+
209+
echo "->isReadable()... ";
210+
testBaseClass( function() {
211+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
212+
$o->isReadable();
213+
} );
214+
215+
echo "->isWritable()... ";
216+
testBaseClass( function() {
217+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
218+
$o->isWritable();
219+
} );
220+
221+
echo "->seek()... ";
222+
testBaseClass( function() {
223+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
224+
$o->seek(0);
225+
} );
226+
227+
echo "->__toString()... ";
228+
testBaseClass( function() {
229+
$o = new GlobIterator(__DIR__.'/*.abcdefghij');
230+
$o->__toString();
231+
} );
232+
233+
234+
//
235+
// Supplemental test: no method should throw LogicException if it is invoked
236+
// after a successful iteration over a non-empty list of files.
237+
echo "non-empty GlobIterator... ";
238+
testBaseClass( function() {
239+
$o = new GlobIterator(__DIR__.'/*.phpt');
240+
foreach ($o as $file) {
241+
// nothing here, just consume all the items
242+
}
243+
// This must not throw an exception
244+
$o->count();
245+
} );
246+
247+
248+
//
249+
// The correct existing behaviour must not change
250+
// The classes SplFileObject and SplTempFileObject are not affected by the bug
251+
echo "======================= test there are no regressions =======================\n";
252+
253+
echo "SplFileObject existent file... ";
254+
testBaseClass( function() {
255+
$o = new SplFileObject(__FILE__);
256+
$o->fread(1);
257+
} );
258+
259+
echo "SplFileObject non-existent file... ";
260+
testBaseClass( function() {
261+
$o = new SplFileObject('/tmp/abcdefghij.abcdefghij');
262+
$o->fread(1);
263+
} );
264+
265+
266+
267+
//
268+
// Check that when derived classes do not call GlobIterator::__construct()
269+
// the LogicException is thrown (don't break the behaviour introduced to fix bug #54384)
270+
echo "extends GlobIterator... ";
271+
class GlobIteratorChild extends GlobIterator {
272+
public function __construct() {}
273+
}
274+
testChildClass( function() {
275+
$o = new GlobIteratorChild();
276+
$o->count();
277+
} );
278+
279+
echo "extends SplFileObject... ";
280+
class SplFileObjectChild extends SplFileObject {
281+
public function __construct() {}
282+
}
283+
testChildClass( function() {
284+
$o = new SplFileObjectChild();
285+
$o->count();
286+
} );
287+
288+
echo "extends SplTempFileObject... ";
289+
class SplTempFileObjectChild extends SplTempFileObject {
290+
public function __construct() {}
291+
}
292+
testChildClass( function() {
293+
$o = new SplTempFileObjectChild();
294+
$o->count();
295+
} );
296+
297+
298+
299+
--EXPECT--
300+
->count()... ran normally (expected)
301+
->rewind()... ran normally (expected)
302+
->getFlags()... ran normally (expected)
303+
->setFlags()... ran normally (expected)
304+
->valid()... ran normally (expected)
305+
->current()... ran normally (expected)
306+
->key()... ran normally (expected)
307+
->next()... ran normally (expected)
308+
->getATime()... ran normally (expected)
309+
->getBasename()... ran normally (expected)
310+
->getCTime()... ran normally (expected)
311+
->getExtension()... ran normally (expected)
312+
->getFilename()... ran normally (expected)
313+
->getGroup()... ran normally (expected)
314+
->getInode()... ran normally (expected)
315+
->getMTime()... ran normally (expected)
316+
->getOwner()... ran normally (expected)
317+
->getPath()... ran normally (expected)
318+
->getPathname()... ran normally (expected)
319+
->getPerms()... ran normally (expected)
320+
->getSize()... ran normally (expected)
321+
->getType()... ran normally (expected)
322+
->isDir()... ran normally (expected)
323+
->isDot()... ran normally (expected)
324+
->isExecutable()... ran normally (expected)
325+
->isFile()... ran normally (expected)
326+
->isLink()... ran normally (expected)
327+
->isReadable()... ran normally (expected)
328+
->isWritable()... ran normally (expected)
329+
->seek()... ran normally (expected)
330+
->__toString()... ran normally (expected)
331+
non-empty GlobIterator... ran normally (expected)
332+
======================= test there are no regressions =======================
333+
SplFileObject existent file... ran normally (expected)
334+
SplFileObject non-existent file... ran normally (expected)
335+
extends GlobIterator... threw LogicException (expected)
336+
extends SplFileObject... threw LogicException (expected)
337+
extends SplTempFileObject... threw LogicException (expected)

0 commit comments

Comments
 (0)