TECLIB / CFPropertyList

PHP Implementation of Apple's PList (binary and xml)
http://teclib.github.io/CFPropertyList/
MIT License
454 stars 111 forks source link

Output of toXML() for <real>s is locale-dependent and causes creation of invalid PList-XML #67

Open stefanschramm opened 3 years ago

stefanschramm commented 3 years ago

Describe the bug When using a locale that uses commas (,) instead of points (.) as decimal delimiter, the output of CFPropertyList::toXML() uses this charater for <real>s which is invalid PList-Data. The cause is probably some implicit toString conversion that will use the current locale.

To Reproduce Execute this script:

<?php
use CFPropertyList\CFPropertyList;
require_once('vendor/autoload.php');
$example = <<<EOF
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0"><dict><key>ExampleReal</key><real>1.5</real></dict></plist>
EOF;
setlocale(LC_ALL, 'de_DE.UTF-8');
$plist = new CFPropertyList();
$plist->parse($example);
echo $plist->toXML();

It's output will be:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0"><dict><key>ExampleReal</key><real>1,5</real></dict></plist>

Expected behavior This output:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0"><dict><key>ExampleReal</key><real>1.5</real></dict></plist>

(1.5 instead of 1,5)

Desktop

btry commented 3 years ago

Hi

I reproduce. The problem is caused by the use of the function floatval() which outputs a float in a format dependent on your locale.

btry commented 3 years ago

Please test the patch above, and feedback.

stefanschramm commented 3 years ago

The patch adds the dependency to install the php-intl-Extension.

Using bigger numbers like 1234.5 will add thousands-separators: 1,234.5. Adding $formatter->setAttribute(\NumberFormatter::GROUPING_USED, false); solves this for me.

btry commented 3 years ago

Hi

Yes, you're right. I add this extra attribute to the PR.

However I cannot merge the fix right now : I need to add an unit test for your issue, and migrate test environment to github actions. I need some time to do this, merge and build a new release.

EDIT : I added the dependency to php-intl. I think it is better to rely on NumberFormatter instead of a filthy string manipulation, which may break for some foreign language.