Fixes bug #69181 (csv parsing drops newlines within fields with SplFileObject::DROP_NEW_LINE)

[g105b]

This pull request fixes buggy behaviour when using SplFileObject to work with CSV files.

According to the documentation, the DROP_NEW_LINE flag drops newlines at the end of a line. http://php.net/manual/en/class.splfileobject.php#splfileobject.constants.drop-new-line

However, even though CSV files are obviously stored with new lines at the end of each line, the fields within the CSV can also contain any number of newline characters, as long as they are quoted. In this case, the bug appears, as the DROP_NEW_LINE flag causes c function strcspn to remove the first, and only the first, newline from the entire row of fields.

My solution is to ignore the strcspn function when parsing CSV files, because if there are any newlines within fields, the fields MUST be delimited, therefore no newlines should be dropped within the delimiters.

[g105b]

The test that fails in the Travis build is ext/phar/tests/phar_oo_009.phpt.

This test is titled "Phar object: iterating via SplFileObject and reading csv", so it seems to me that on line 19 the SplFileObject::READ_CSV flag should be set, as on line 29?

My PR ignores dropping new lines within CSV rows (keeping the original data, as it is stored in CSV), so the outcome is that when SplFileObject::READ_CSV is set, no new lines are dropped (affecting the expected result from the now-failing test).

@laruence and @m6w6 may want to give input to this, regarding the Phar test?

[ghost]

Can one of the admins verify this patch?

[smalyshev]

I'm not sure the result you're getting in ext/phar/tests/phar_oo_009.phpt is correct. Linefeed within delimiters may be passed as is, true, but the linefeed you have there is not within the delimiters. I think you need to handle this case more carefully.

[g105b]

@smalyshev I'm not sure what the phar test is doing, and I have not managed to run it in isolation.

I think you need to handle this case more carefully

are you addressing the functionality supplied by my patch, or the test in ext/phar/tests/phar_oo_009.phpt ?

[cmb69]

The test is failing, because there is a empty line in the CSV, which is not skipped, even though SplFileObject::SKIP_EMPTY is given. That is because the modified code puts a non zero length into intern->u.file.current_line_len, but the line would only be skipped if intern->u.file.current_line_len was zero.

Actually, it seems to me that unrelated whether SPL_FILE_OBJECT_READ_CSV is given, the buffer should be investigated from the end, replacing all \n and \r with \0 (decreasing line_len as well), and stopping as soon as another character is detected.

IOW: don't make a case differentiation, but replace the strcspn() with something that works from right-to-left.

[cmb69]

Diregard my suggested solution; that can't work.

[krakjoe]

Since this targets an unsupported branch of PHP, and since this seems to introduce inconsistencies of it's own, and since the author seems to have abandoned working on it, I'm closing this PR.

Please take this action as encouragement to open a clean PR against a supported branch of PHP.

[g105b]

Thanks, I'll look into the affects on 7 and whether it is still valid.