Bug #55413 [Com]: str_getcsv doesnt remove escape characters

Edit report at https://bugs.php.net/bug.php?id=55413&edit=1 ID: 55413 Comment by: colinodell@php.net Reported by: mathielen at gmail dot com Summary: str_getcsv doesnt remove escape characters Status: Open Type: Bug Package: Strings related Operating System: ubuntu 11.04 PHP Version: 5.3.6 Block user comment: N Private report: N New Comment: According to RFC 4180 Common Format and MIME Type for CSV Files: > 7. If double-quotes are used to enclose fields, then a double-quote > appearing inside a field must be escaped by preceding it with > another double quote. For example: > > "aaa","b""bb","ccc" PHP does indeed support this escaping method which is common amongst most other CSV implementations: https://3v4l.org/e0aX8 Previous Comments: ------------------------------------------------------------------------ [2014-10-24 17:08:16] desertshadow at gmail dot com Has this bug really been open for 3 years? This is a pretty big bug, 1) The docs are incorrect 2) The CSV parser isn't working correctly I'm trying to escape a comma in a CSV string but it doesn't appear to be escaping correctly. ------------------------------------------------------------------------ [2012-07-15 04:07:21] darren at dcook dot org Yes, agree: 1. change docs to say $escape defaults to '"' 2. Change code to use $escape when it is something else (NB. IIUC, this won't break backwards compatibility.) ------------------------------------------------------------------------ [2012-07-14 07:39:46] dan dot libby at gmail dot com I just ran into this bug also. I don't know the history, and haven't reviewed the str_getcsv() source yet but I am guessing that *getcsv() were originally implemented with excel style double-quote escaping. Somehow the escape='\\' param got added to the documentation, but seemingly not the code. Defaulting escape='\\' as the documentation says would potentially break apps depending on escape='"'. So that would be a breaking change, and a bad idea. But leaving it as supporting only escape='"' is also bad, because it limits the utility of the function. For example, I need to parse apache logs, and apache only supports escaping with \. whoops. So I believe the correct fix would be to default to escape='"' so we don't break apps using it with defaults, but still support explicit use of escape='\\'. agree? disagree? ------------------------------------------------------------------------ [2012-05-14 14:30:33] spidgorny at gmail dot com 5.3.10 is affected too. A bug in a primitive function like this after years of evolution should be embarrassing. ------------------------------------------------------------------------ [2012-04-27 03:08:46] darren at dcook dot org Another way of looking at the code in comment 1 is that the behaviour is correct (for parsing Excel-style csv), but the documentation is confusing. In my testing the "" within quotes is being handled correctly (and the $escape parameter is either not being used, or has not got in my way yet). But as another viewpoint, if we take the original bug report example and do: $line = '"A";"Some \"Stuff\"";"C"' print_r(str_getcsv($line, ';', '"', 'x')); (BTW, I'm using 'x' to mean no escaping; using a '' uses the default instead!!) Output is: Array ( [0] => A [1] => Some \Stuff\"" [2] => C ) This almost makes sense if you consider it treated the second field as three sub-strings: "Some \" Stuff\ "" The problem is, if that was true, the 3rd sub-string got parsed wrongly. The 3rd sub-string should have evaluated to a blank string. Summary: something is wrong. Either there is a bug to fix, or the $escape parameter should be removed completely, or the function needs to document the intended behaviour for corner cases like these. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=55413 -- Edit this bug report at https://bugs.php.net/bug.php?id=55413&edit=1