Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100759
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmx.de designates 212.227.17.21 as permitted sender)
To: 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>
Message-ID: <c2b8b875-f98e-1745-2ba8-ef7289a9c15f@gmx.de>
Date: Sat, 23 Sep 2017 00:09:23 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <CA+kxMuQRHq8HfuicqSFm8_Xcxw7VD6Aaqoi5TZa7m7Tofh1YKA@mail.gmail.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 21.09.2017 at 23:08, Dan Ackroyd wrote:

> On 21 September 2017 at 12:43, 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.

I agree that php_fgetcsv() has serious issues, and it might not be
possible to fix it without causing severe BC breaks.  php_fputcsv(), on
the other hand, is less of a problem, though.

Overall, the most demanding issue is that both functions try to regard
the current locale, but already fail that generally, since several
parameters are declared as char, which can't work for (some) multibyte
encodings.  For instance, it is impossible to generate proper UTF-16
encoded CSV files, or to read them.  This issue continues, because
several (mostly?) whitespace characters are hard-coded assuming an ASCII
compatible character encoding.

A minor issue are the hard-coded record terminators, which are currently
LF (RFC 4180 specifies CRLF).  Apparently, this isn't a real issue
nowadays (besides the missing support for non ASCII compatible character
encodings).

Another issue concerns the escape character.  Frankly, I don't even have
the slightest clue how that is supposed to work, and why it even has
been introduced in the first place.  Maybe it has been introduced for
compatibility with some application requiring it; maybe it has been
introduced to support "DSV style"[1].  If the latter, at least
php_fputcsv() doesn't support it (anymore).  Unless it's clear what the
escape character exactly is supposed to do, we *cannot* even *hope* to
fix the implementation.

Introducing new functions with a clearly defined behavior would be nice,
but appears to me somewhat as pie in the sky (somebody would have to do
the actual work!).  But even if we do so, at least the actual behavior
of the existing functions would have to be documented.

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.  I guess that
fgetcsv() and fputcsv() are mostly used for importing from and exporting
to CSV, respectively, but not as replacement for an SQL database engine.

[1] <http://www.catb.org/~esr/writings/taoup/html/ch05s02.html>

-- 
Christoph M. Becker