Fix #61597: SXE properties may lack attributes and content #5246
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We must not treat a node as string if it has attributes, unless it is an entity declaration which is always treated as string by simplexml.
nikic
reviewed
Mar 9, 2020
| } | ||
| ["elem1"]=> | ||
| string(10) "Bla bla 1." | ||
| object(SimpleXMLElement)#%d (3) { |
Member
There was a problem hiding this comment.
Looks like the Bla bla 1. text got lost completely here?
Member
Author
There was a problem hiding this comment.
That is actually to be expected for mixed content; text nodes are dropped. That is already the case if these elements are var_dump()ed directly (in this case var_dump($sxe->elem1); only children of the SimpleXMLElement are treated wrong without this patch.
Member
There was a problem hiding this comment.
That seems pretty weird ... but it's SimpleXML so 🤷♂️
nikic
approved these changes
Mar 12, 2020
Member
Author
|
Thanks! Applied as 7c081db. |
wmfgerrit
pushed a commit
to wikimedia/mediawiki
that referenced
this pull request
Apr 30, 2020
Upstream bug reports of the behavior change introduced in PHP 7.3.17 (and applied to PHP 7.4 branch as well): https://bugs.php.net/bug.php?id=79528 https://bugs.php.net/bug.php?id=79485 The reponsible commit in PHP was php/php-src#5246 This was a "bug fix" in the sense that SimpleXML used to discard the attributes on the namespace elements, which look like this: <namespace key="-2" case="first-letter">Media</namespace> SimpleXML used to return this as a string "Media" instead of a SimpleXMLElement... but ExportTest (inadvertently?) depended on that behavior. In any case, if we iterate over SimpleXMLElement::children() we always get SimpleXMLElements, not "sometimes strings", and so our code will correct correctly on PHP below 7.3.17 and above, regardless of how PHP decides to handle this "bug". Bug: T250568 Change-Id: I9c2cb6a86fd6e8023c1979ec6838071a87a7bcea
wmfgerrit
pushed a commit
to wikimedia/mediawiki
that referenced
this pull request
Apr 30, 2020
Upstream bug reports of the behavior change introduced in PHP 7.3.17 (and applied to PHP 7.4 branch as well): https://bugs.php.net/bug.php?id=79528 https://bugs.php.net/bug.php?id=79485 The reponsible commit in PHP was php/php-src#5246 This was a "bug fix" in the sense that SimpleXML used to discard the attributes on the namespace elements, which look like this: <namespace key="-2" case="first-letter">Media</namespace> SimpleXML used to return this as a string "Media" instead of a SimpleXMLElement... but ExportTest (inadvertently?) depended on that behavior. In any case, if we iterate over SimpleXMLElement::children() we always get SimpleXMLElements, not "sometimes strings", and so our code will correct correctly on PHP below 7.3.17 and above, regardless of how PHP decides to handle this "bug". Bug: T250568 Change-Id: I9c2cb6a86fd6e8023c1979ec6838071a87a7bcea (cherry picked from commit 7f1ad7d)
wmfgerrit
pushed a commit
to wikimedia/mediawiki
that referenced
this pull request
Apr 30, 2020
Upstream bug reports of the behavior change introduced in PHP 7.3.17 (and applied to PHP 7.4 branch as well): https://bugs.php.net/bug.php?id=79528 https://bugs.php.net/bug.php?id=79485 The reponsible commit in PHP was php/php-src#5246 This was a "bug fix" in the sense that SimpleXML used to discard the attributes on the namespace elements, which look like this: <namespace key="-2" case="first-letter">Media</namespace> SimpleXML used to return this as a string "Media" instead of a SimpleXMLElement... but ExportTest (inadvertently?) depended on that behavior. In any case, if we iterate over SimpleXMLElement::children() we always get SimpleXMLElements, not "sometimes strings", and so our code will correct correctly on PHP below 7.3.17 and above, regardless of how PHP decides to handle this "bug". Bug: T250568 Change-Id: I9c2cb6a86fd6e8023c1979ec6838071a87a7bcea (cherry picked from commit 7f1ad7d)
wmfgerrit
pushed a commit
to wikimedia/mediawiki
that referenced
this pull request
Apr 30, 2020
Upstream bug reports of the behavior change introduced in PHP 7.3.17 (and applied to PHP 7.4 branch as well): https://bugs.php.net/bug.php?id=79528 https://bugs.php.net/bug.php?id=79485 The reponsible commit in PHP was php/php-src#5246 This was a "bug fix" in the sense that SimpleXML used to discard the attributes on the namespace elements, which look like this: <namespace key="-2" case="first-letter">Media</namespace> SimpleXML used to return this as a string "Media" instead of a SimpleXMLElement... but ExportTest (inadvertently?) depended on that behavior. In any case, if we iterate over SimpleXMLElement::children() we always get SimpleXMLElements, not "sometimes strings", and so our code will correct correctly on PHP below 7.3.17 and above, regardless of how PHP decides to handle this "bug". Bug: T250568 Change-Id: I9c2cb6a86fd6e8023c1979ec6838071a87a7bcea (cherry picked from commit 7f1ad7d)
|
This "fix" is a breaking change from PHP 7.3.16 to 7.3.17. Differences: This can kill running systems that are uses XML. Please don't release "fixes" like this as an bugfix release. |
Member
Author
|
Sorry, that this happened; the bad fix has already been reverted, and PHP 7.3.18 (which will be released tomorrow) should be working again like before (not that the former behavior makes sense, though). |
|
Reverting is also a bad idea (for us), because we have fixed our bugs with version 7.3.17. |
|
🤷♂️ |
mmeller-wikia
pushed a commit
to Wikia/mediawiki
that referenced
this pull request
Feb 16, 2021
Upstream bug reports of the behavior change introduced in PHP 7.3.17 (and applied to PHP 7.4 branch as well): https://bugs.php.net/bug.php?id=79528 https://bugs.php.net/bug.php?id=79485 The reponsible commit in PHP was php/php-src#5246 This was a "bug fix" in the sense that SimpleXML used to discard the attributes on the namespace elements, which look like this: <namespace key="-2" case="first-letter">Media</namespace> SimpleXML used to return this as a string "Media" instead of a SimpleXMLElement... but ExportTest (inadvertently?) depended on that behavior. In any case, if we iterate over SimpleXMLElement::children() we always get SimpleXMLElements, not "sometimes strings", and so our code will correct correctly on PHP below 7.3.17 and above, regardless of how PHP decides to handle this "bug". Bug: T250568 Change-Id: I9c2cb6a86fd6e8023c1979ec6838071a87a7bcea (cherry picked from commit 7f1ad7d)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We must not treat a node as string if it has attributes, unless it is
an entity declaration which is always treated as string by simplexml.