-
-
Notifications
You must be signed in to change notification settings - Fork 333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Infinite looping timeout #325
Comments
@Zyles thanks for the bug report but I fail to reproduce your issue using your code. Did you manage to solve it in the meantime ? because looking at your code I don't get why nginx would be affected by the library... I need more detail to investigate |
Well I don't know, it just keeps timing out. Not sure how to debug it any further. I went with the statement solution instead which works for me. |
@Zyles is it possible to get the command used only those related to League\Csv so I get try to reproduce the bug. The bug is about getContent and you seems to have resolve it using Statement but internally both method uses the same code so I have to wonder 🤔 .
Having acces to the script would help in knowing if it is a bug or a misconfiguration of the library. |
closing since no feedback was provided |
@nyamsprod - I just happened to see a very similar issue - maybe I misunderstand something but the following lines just don't work: // it does not matter if we load it from a file or from string
// $csv = Reader::createFromPath('./input/test.csv', 'r');
$csv = Reader::createFromString("col1,col2\nv1a,v2a\nv1b,v2b");
// but it does matter if we call the getRecords. Because if we do, the
// ::getContent will never end, it will go into an infinite loop.
$records = $csv->getRecords();
foreach($records as $r){
print_r($r);
}
echo $csv->getContent(); //returns the CSV document as a string I'm sure I'm missing something? But can't see what that would be o.O I think that even if I missued it, it shouldn't behave like this :) Once again - everything works fine if you'll remove the On apache, PHP 7.2.15-1+ubuntu16.04.1+deb.sury.org+1 (cli) (built: Feb 8 2019 15:37:29) ( NTS ) Oh, here is an edit:
nope, a regular, very normal file, but it also fails with a string
nope
it's apache, but I also tried on cli, same result. |
@kpion thanks for the code could you try your snippet with |
@nyamsprod When using $fo = new SplFileObject('./input/tests.csv');
$csv = Reader::createFromFileObject($fo);
//now everything's cool, even when I do this stuff:
$records = $csv->getRecords();
foreach($records as $r){
print_r($r);
}
echo $csv->getContent(); //returns the CSV document as a string The csv file is the same as in the above examples. It doesn't matter as well though, because when I load this file with |
@kpion then I know what is the issue 🎉 but It will take time for me to resolve it 😢 Basically there's an issue in the |
OK, well, thanks. Although - two things - it's cli, and also apache. Didn't actually test it under nginx. The OP did :) And second thing - when it comes to me, I did "solve" by not calling the I made this comment to just let you know. Thanks for the stuff, and keep the good work! |
@kpion turns out that with your last comment I was able to quickly resolve the issue 👍 🎉 . If you could test the branch in PR I'll merge it and release the bugfixed version ASAP. |
Hmm, yes and no :) Seems like we test it differently, or... IDK, maybe I'm doing something wrong. But no, it did not work. But I did take a look at your changes, you're playing with the $this->document->setFlags(0); But in my scenario, this method (output) isn't even called. The So I modified the above method, i.e. I added this setFlags to it and yeah, now it works:) Here is the complete method now: public function getContent(): string
{
$this->document->setFlags(0);// here it is
$raw = '';
foreach ($this->chunk(8192) as $chunk) {
$raw .= $chunk;
}
return $raw;
} My guess is that it's wrong, for example because the flags should possibly be restored or something, but ... it proves it works :) |
Sorry yeah wrong patched method it should have been in the getContent like you did... I'll fix this asap. Good catch |
…eout Bugfix/infinite looping timeout issue #325
fix merge into master |
Yeah, tested, and it works. Thank you! |
Bug Report
Summary
echo $csv->getContent(); causes an infinite loop and eventually nginx gateway timeout.
The CSV file consists of:
Expected result
Stop timing out.
Actual result
Timeout.
The text was updated successfully, but these errors were encountered: