zio / zio-ftp

A simple, idiomatic (S)FTP client for ZIO
https://zio.dev/zio-ftp
Apache License 2.0
29 stars 13 forks source link

UnsecureFtp: improve `readFile` Stream finalization #410

Open mberndt123 opened 1 month ago

mberndt123 commented 1 month ago

Hi there,

I recently noticed a problem with zio-ftp. I was trying to read a zip file from the server using java.util.zip.ZipInputStream. So I was just iterating over the entries in the zip file, reading the data from the entries that I'm interested in and it all worked beautifully… until I tried to read another file from the FTP server, which resulted in a file not found error, even though I knew the file was there. It turns out that iterating over a zip archive with ZipInputStream never results in an attempt to read beyond the end of the file. But due to the way that finalization of the stream is implemented (using the ++ operator on a stream), this means that completePendingCommand isn't called, resulting in the problem I've seen. This might also be the reason for bug #227.

The solution is to use .ensuring for the finalizer. This has the added benefit that it's now possible to read only part of the file and stop processing once you've found the bit you're interested in. But it leads to another problem: finalizers aren't allowed to throw errors, but completePendingCommand can fail. I could die in that situation, but that doesn't seem right, because die indicates a defect in the program, and when the FTP server decides to be weird, that's not the program's fault. Therefore, I decided to not throw the error from the finalizer but instead delay the error until the next execute call. I've also improved the error handling so that it doesn't just assume a "File doesn't exist" error but instead exposes the error message given by the FTP server to make it more transparent what's going on.

mberndt123 commented 1 month ago

There's something wrong with the CI setup (docker-compose seems to be missing), but UnsecureFtpSpec and UnsecureDownloadFinalizeSpec pass, so… I hope it's OK :-)

regis-leray commented 1 month ago

There's something wrong with the CI setup (docker-compose seems to be missing), but UnsecureFtpSpec and UnsecureDownloadFinalizeSpec pass, so… I hope it's OK :-)

i have a PR to fix it https://github.com/zio/zio-ftp/pull/411

regis-leray commented 1 month ago

CI build is fixed, please rebase your branch