Skip to content
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

UnsecureFtp: improve readFile Stream finalization #420

Closed
regis-leray opened this issue Oct 10, 2024 · 0 comments · Fixed by #421
Closed

UnsecureFtp: improve readFile Stream finalization #420

regis-leray opened this issue Oct 10, 2024 · 0 comments · Fixed by #421

Comments

@regis-leray
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant