[1.5] Prevent infinite loop on empty xml elements #159
Conversation
DeepDiver1975
force-pushed
the
bugfix/infinite-loop-1.5
branch
from
909e850 to
045dfc1
Compare
lib/Deserializer/functions.php
Outdated
| } | ||
| } while ($reader->nodeType !== Reader::END_ELEMENT); | ||
|
|
||
| $reader->read(); | ||
| if (!$reader->read()) { |
There was a problem hiding this comment.
Why was the if statement added here? This duplicates also the return of this method.
There was a problem hiding this comment.
this is true - thx for having a closer look.
I added this for debugging purpose to see if read fails at this point under any condition .....
| @@ -123,6 +125,40 @@ function testExpect() { | |||
| ); | |||
| } | |||
|
|
|||
| /** | |||
| * @expectedException \Sabre\Xml\LibXMLException | |||
There was a problem hiding this comment.
The support for this annotation will be removed with version 9 of PHPUnit. This would not be a problem if this package did not use the "death star version constraint" on PHPUnit.
- Remove annotation(s) for expecting exceptions sebastianbergmann/phpunit#3333
- https://thephp.cc/news/2017/02/death-star-version-constraint
My suggestion: use the expectException method instead and define a version for PHPUnit in the Composer configuration – version 8 is around the corner.
There was a problem hiding this comment.
thx for noticing.
at first we should get rid of the "death star version constraint".
changes to our unit tests itself are welcome via separate pull requets
There was a problem hiding this comment.
There was a problem hiding this comment.
This is the 1.5 branch - it is on pure maintenance mode. No idea if we should put any further man power into this.
There was a problem hiding this comment.
This is the 1.5 branch - it is on pure maintenance mode. No idea if we should put any further man power into this.
the mentioned issues should be fixed on master.
in 1.5 I would only fix the "death star version constraint"
DeepDiver1975
force-pushed
the
bugfix/infinite-loop-1.5
branch
from
045dfc1 to
fd77b16
Compare
DeepDiver1975
changed the title
|
feel free to merge, in case this is ready. |
I'm preparing the release right in here as well ... then merge .... then repeat on all other PRs .... |
Brings #158 to the 1.5 branch