Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:103156
To: Theodore Brown <theodorejb@outlook.com>,
 Dan Ackroyd <danack@basereality.com>
Cc: PHP internals <internals@lists.php.net>
References: <7cf5adb8-0738-259e-6d1e-f966722fdae2@gmx.de>
 <CA+kxMuQRHq8HfuicqSFm8_Xcxw7VD6Aaqoi5TZa7m7Tofh1YKA@mail.gmail.com>
 <c2b8b875-f98e-1745-2ba8-ef7289a9c15f@gmx.de>
 <CY4PR18MB10489E50B61CC480A83B5853DE060@CY4PR18MB1048.namprd18.prod.outlook.com>
Message-ID: <5ca1b295-6f43-794c-f2a9-f672155cc84c@gmx.de>
Date: Thu, 13 Sep 2018 14:30:50 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.0
MIME-Version: 1.0
In-Reply-To: <CY4PR18MB10489E50B61CC480A83B5853DE060@CY4PR18MB1048.namprd18.prod.outlook.com>
Content-Type: text/plain; charset=utf-8
Content-Language: de-DE
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] fputcsv() and $escape character
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 09.09.2018 at 16:55, Theodore Brown wrote:

> On Friday, Sep. 22, 2017 at 5:09 PM Christoph M. Becker <cmbecker69@gmx.de> wrote:
> 
>>>> There are several bug reports regarding "broken" `fputcsv()` behavior
>>>> in our tracker, namely, because the $escape parameter causes unexpected
>>>> results. For instance:
>>>
>>> I looked at fixing some of the CSV related bugs about a year or so
>>> ago. My conclusions were:
>>>
>>> i) There is no way to fix the problems that wouldn't cause horrible BC
>>> breaks for code that is only coincidentally working currently.
>>>
>>> ii) Handling strings in C is much more error prone than handling them in PHP.
>>>
>>> I'm reasonably certain that trying to fix the current functions is the
>>> wrong approach, and one of the following would be much better.
>>>
>>> Either, find a C library that has already been proven to handle CSV
>>> parsing/generating 'correctly' and bring that into PHP core under
>>> either new function names or namespace.
>>>
>>> Or, write the code in PHP (or just use https://github.com/thephpleague/csv)
>>> and find a way to make that fast enough for people to use.
>>>
>>> Touching the existing code is pretty certain to bring a lot of pain,
>>> without resulting in a fully compliant csv parser/generator.
>>
>> And frankly, I don't see why it would be a problem to allow to use no
>> escape character for `fputcsv()`. That certainly wouldn't be a BC break,
>> since currently the function bails out if `escape_str_len < 1`. Of
>> course, that wouldn't fix all issues, but it appears to make the
>> function work as expected for ASCII compatible character encodings (for
>> other character encodings the function appears to be broken anyway).
>>
>> Ad league/csv: rather impressive! However, including this functionality
>> into ext/standard is totally over the top, in my opinion.
> 
> Ironically, the League/Csv library uses `fputcsv()` behind the scenes, and
> thus suffers from the same escape character problem
> (see https://github.com/thephpleague/csv/issues/307).
> 
> Is there any chance `fputcsv()` can be updated to allow setting a blank
> string as the escape character? As Christopher pointed out, this would not
> be a BC break, and while it wouldn't fix every issue with the function it
> would still be a step forward. Frankly I found it very surprising that the
> function *didn't* support passing a blank string.

I've submitted <https://github.com/php/php-src/pull/3515>.

-- 
Christoph M. Becker