sabre-io / xml

sabre/xml is an XML library that you may not hate.
http://sabre.io/xml/
BSD 3-Clause "New" or "Revised" License
516 stars 77 forks source link

libxml_disable_entity_loader is deprecated as of PHP 8.0 #203

Closed blackstar0169 closed 2 years ago

blackstar0169 commented 3 years ago

I got this error when I send a PROPFIND request.

Deprecated:  Function libxml_disable_entity_loader() is deprecated in /vendor/sabre/xml/lib/Reader.php on line 59

This function is deprecated as of PHP 8.0 https://www.php.net/manual/en/function.libxml-disable-entity-loader.php

My request timeline :

* Preparing request to https://myhost.com/server.php
* Current time is 2021-09-15T15:53:25.044Z
* Using libcurl/7.73.0-DEV OpenSSL/1.1.1g zlib/1.2.11 brotli/1.0.9 WinIDN libssh2/1.9.0_DEV nghttp2/1.41.0
* Using default HTTP version
* Disable timeout
* Enable automatic URL encoding
* Disable SSL validation
* Using Stream ID: 3 (easy handle 0x1a6e02ed090)

> PROPFIND /server.php HTTP/2
> Host: myhost.com
> user-agent: insomnia/2021.5.2
> content-type: application/xml
> accept: */*
> content-length: 102

| <?xml version="1.0" encoding="UTF-8"?>
| <propfind xmlns="DAV:"><prop><resourcetype/></prop></propfind>

Thanks for sharing this nice library !

evert commented 3 years ago

Ok, I think it makes sense to do a new major version of sabre/xml that removes libxml_disable_entity_loader(), and has PHP 8 as a minimum version.

blackstar0169 commented 3 years ago

On this link https://php.watch/versions/8.0/libxml_disable_entity_loader-deprecation the author said that it is safe to do this for backward compatibility :

- libxml_disable_entity_loader(true);
+ if (\PHP_VERSION_ID < 80000) {
+      libxml_disable_entity_loader(true);
+ }

Or this in case you want to rely on libxml version :

- libxml_disable_entity_loader(true);
+ if (\LIBXML_VERSION < 20900) {
+      libxml_disable_entity_loader(true);
+ }

I think this could avoid making a major version, but my knowledge on XML parsing is limited, so I don't know what consequences those changes may have in this library.

evert commented 3 years ago

That sounds like a better idea =)

phil-davis commented 2 years ago

IMO this was fixed in commit https://github.com/sabre-io/xml/pull/190/commits/73040eda220955a9ab3721c25483da1192c36242

And then the logic was improved in #204

Please feel free to ping here if it is not working.