ssh: ignore io.EOF from sftp.Server.Serve#8670
Merged
Merged
Conversation
If the connection provided to sftp.NewServer is closed, Serve returns the io.EOF error verbatim from io.Reader.Read. This is an odd error since this is an expected situation, so we manually ignore io.EOF. This is somewhat buggy since the sftp package itself incorrectly reports io.EOF in cases where it should actually be reporting io.ErrUnexpectedEOF. See pkg/sftp#554 which patches Serve to return nil on clean closes and fixes buggy uses of io.ReadFull. Fixes #8592 Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Member
Author
|
I tested this manually and then started to read the "sftp" package code to understand why it returns I couldn't figure out an easy way to write a test. The entire |
bradfitz
approved these changes
Jul 21, 2023
bradfitz
left a comment
Member
There was a problem hiding this comment.
There's a bunch of stuff in that sftp package I want to fix. Maybe it's time we fork it?
Member
|
Should we cherry-pick this back to the 1.46 branch? Probably? |
Contributor
|
Putting it in 1.46.1 seems reasonable. |
Member
|
Then I'll merge this & include it in my cherry-pick PR I'm about to send. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If the connection provided to sftp.NewServer is closed, Serve returns the io.EOF error verbatim from io.Reader.Read. This is an odd error since this is an expected situation, so we manually ignore io.EOF.
This is somewhat buggy since the sftp package itself incorrectly reports io.EOF in cases where it should actually be reporting io.ErrUnexpectedEOF.
See pkg/sftp#554 which patches Serve to return nil on clean closes and fixes buggy uses of io.ReadFull.
Fixes #8592