Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Few tests failed building with new libxml 2.11.0 #11160

Closed
andypost opened this issue Apr 29, 2023 · 5 comments · Fixed by ThePHPF/thephp.foundation#90
Closed

Few tests failed building with new libxml 2.11.0 #11160

andypost opened this issue Apr 29, 2023 · 5 comments · Fixed by ThePHPF/thephp.foundation#90
Labels
Bug Extension: dom Extension: libxml Extension: xml Status: Verified

Comments

@andypost
Copy link
Contributor

andypost commented Apr 29, 2023 •
edited
Loading

Description

The following tests fail:with https://gitlab.gnome.org/GNOME/libxml2/-/tags/v2.11.0#deprecations

Test DOMDocument::loadXML() detects not-well formed XML [ext/dom/tests/DOMDocument_loadXML_error2.phpt]
Test DOMDocument::load() detects not-well formed XML [ext/dom/tests/DOMDocument_load_error2.phpt]
Bug #61367: open_basedir bypass in libxml RSHUTDOWN: read test [ext/libxml/tests/bug61367-read_2.phpt]
libxml_disable_entity_loader() [ext/libxml/tests/libxml_disable_entity_loader_2.phpt]
Bug #26614 (CDATA sections skipped on line count) [ext/xml/tests/bug26614_libxml.phpt]

Diffs in first comment

PHP Version

PHP 8.1.18

Operating System

Alpinelinux
@andypost
Copy link
Contributor Author

Diff is

TEST 5588/15656 [ext/dom/tests/DOMDocument_loadXML_error2.phpt]
========DIFF========
     Warning: DOMDocument::load%r(XML){0,1}%r(): AttValue: " or ' expected %s
     
�[1;32m003+ Warning: DOMDocument::loadXML(): internal error: xmlParseStartTag: problem parsing attributes in Entity, line: 4 in /builds/alpine/aports/community/php81/src/php-8.1.19RC1/ext/dom/tests/DOMDocument_loadXML_error2.php on line 7�[0m
�[1;31m003- Warning: DOMDocument::load%r(XML){0,1}%r(): attributes construct error %s�[0m
     
     Warning: DOMDocument::load%r(XML){0,1}%r(): Couldn't find end of Start Tag book %s
     
--
========DONE========
...
TEST 5599/15656 [ext/dom/tests/DOMDocument_load_error2.phpt]
========DIFF========
     Warning: DOMDocument::load%r(XML){0,1}%r(): AttValue: " or ' expected %s
     
�[1;32m003+ Warning: DOMDocument::load(): internal error: xmlParseStartTag: problem parsing attributes in /builds/alpine/aports/community/php81/src/php-8.1.19RC1/ext/dom/tests/not_well_formed2.xml, line: 4 in /builds/alpine/aports/community/php81/src/php-8.1.19RC1/ext/dom/tests/DOMDocument_load_error2.php on line 8�[0m
�[1;31m003- Warning: DOMDocument::load%r(XML){0,1}%r(): attributes construct error %s�[0m
     
     Warning: DOMDocument::load%r(XML){0,1}%r(): Couldn't find end of Start Tag book %s
     
--
========DONE========
...
TEST 7451/15656 [ext/libxml/tests/bug61367-read_2.phpt]
========DIFF========
--
     int(4)
     bool(true)
     
�[1;31m006- Warning: DOMDocument::loadXML(): I/O warning : failed to load external entity "file:///%s/test_bug_61367-read/bad" in %s on line %d�[0m
�[1;31m007- �[0m
     Warning: Attempt to read property "nodeValue" on null in %s on line %d
========DONE========
...
TEST 7459/15656 [ext/libxml/tests/libxml_disable_entity_loader_2.phpt]
========DIFF========
--
     
     Deprecated: Function libxml_disable_entity_loader() is deprecated in %s on line %d
     bool(false)
�[1;31m005- �[0m
�[1;31m006- Warning: DOMDocument::loadXML(): I/O warning : failed to load external entity "%s" in %s on line %d�[0m
     bool(true)
     Done
========DONE========
...
TEST 14990/15655 [ext/xml/tests/bug26614_libxml.phpt]
========DIFF========
�[1;32m002+ <DATA> at line 2, col 6 (byte 50)�[0m
     <DATA> at line 2, col %d (byte 9)
�[1;32m003+ </DATA> at line 9, col 8 (byte 96)�[0m
�[1;31m002- <DATA> at line 2, col %d (byte 9)�[0m
�[1;31m003- </DATA> at line 9, col %d (byte 55)�[0m
�[1;32m005+ <DATA> at line 2, col 6 (byte 50)�[0m
     <DATA> at line 2, col %d (byte 9)
�[1;32m006+ </DATA> at line 9, col 8 (byte 96)�[0m
�[1;31m005- <DATA> at line 2, col %d (byte 9)�[0m
�[1;31m006- </DATA> at line 9, col %d (byte 55)�[0m
     Text
�[1;32m008+ <DATA> at line 2, col 6 (byte 50)�[0m
�[1;32m009+ </DATA> at line 9, col 8 (byte 96)�[0m
�[1;31m008- <DATA> at line 2, col %d (byte 9)�[0m
�[1;31m009- </DATA> at line 9, col %d (byte 55)�[0m
========DONE========

@andypost andypost changed the title Few test failed building with libxml 2.11.0 Few tests failed building with libxml 2.11.0 Apr 29, 2023
@andypost andypost changed the title Few tests failed building with libxml 2.11.0 Few tests failed building with new libxml 2.11.0 Apr 29, 2023
@nielsdos
Copy link
Member

nielsdos commented Apr 29, 2023 •
edited
Loading

The tests whose message differs can be fixed by duplicating the test: have a version for <2.11 and a version for >=2.11
Some other tests have a missing warning, php_libxml_ctx_error_level is actually called but the parser->input pointer is NULL, which means that PHP will not print the warning. I'll try to figure out why the input pointer is NULL.

EDIT: it seems that the actual problem is the error_type being 2 instead of 0 ?

@andypost
Copy link
Contributor Author

andypost commented Apr 29, 2023 •
edited
Loading

Thank you! meantime this tests are excluded and the rest going good alpinelinux/aports@2b8cddf and alpinelinux/aports@482b784

@nielsdos
Copy link
Member

I just performed a bisect on libxml2 and found this commit: https://gitlab.gnome.org/GNOME/libxml2/-/commit/9a82b94a94bd310db426edd453b0f38c6c8f69f5 is the first commit that breaks the PHP behaviour wrt the parser->input state.

@andypost
Copy link
Contributor Author

As I got lots of old parser-related APIs are deprecated, that's why mentioned in description

nielsdos added a commit to nielsdos/php-src that referenced this issue Apr 29, 2023
@nielsdos
Fix phpGH-11160: Few tests failed building with new libxml 2.11.0
1b562ba 
It's possible to categorise the failures into 2 categories:
  - Changed error message. In this case we either duplicate the test and
    modify the error message. Or if the change in error message is
    small, we use the EXPECTF matchers to make the test compatible with both
    old and new versions of libxml2.
  - Missing warnings. This is caused by a change in libxml2 where the
    parser started using SAX APIs internally [1]. In this case the
    error_type passed to php_libxml_internal_error_handler() changed from
    PHP_LIBXML_ERROR to PHP_LIBXML_CTX_WARNING because it internally
    started to use the SAX handlers instead of the generic handlers.
    However, for the SAX handlers the current input stack is empty, so
    nothing is actually printed. I fixed this by falling back to a
    regular warning without a filename & line number reference, which
    mimicks the old behaviour. Furthermore, this change now also shows
    an additional warning in a test which was previously hidden.

[1] https://gitlab.gnome.org/GNOME/libxml2/-/commit/9a82b94a94bd310db426edd453b0f38c6c8f69f5
@nielsdos nielsdos linked a pull request Apr 29, 2023 that will close this issue
Fix GH-11160: Few tests failed building with new libxml 2.11.0 #11162
Closed
nielsdos added a commit that referenced this issue May 6, 2023
@nielsdos
Merge branch 'PHP-8.1' into PHP-8.2
dc1a70c 
* PHP-8.1:
  Fix GH-11160: Few tests failed building with new libxml 2.11.0
nielsdos added a commit that referenced this issue May 6, 2023
@nielsdos
Merge branch 'PHP-8.2'
82b0537 
* PHP-8.2:
  Fix GH-11160: Few tests failed building with new libxml 2.11.0
@pronskiy pronskiy mentioned this issue Jun 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Extension: dom Extension: libxml Extension: xml Status: Verified
Projects
None yet
Milestone
No milestone
3 participants
@andypost @nielsdos and others