pkg/sftp

SFTP support for the go.crypto/ssh package

sftp

The sftp package provides support for file system operations on remote ssh servers using the SFTP subsystem. It also implements an SFTP server for serving files from the filesystem.

usage and examples

See https://pkg.go.dev/github.com/pkg/sftp for examples and usage.

The basic operation of the package mirrors the facilities of the os package.

The Walker interface for directory traversal is heavily inspired by Keith Rarick's fs package.

roadmap

• There is way too much duplication in the Client methods. If there was an unmarshal(interface{}) method this would reduce a heap of the duplication.

contributing

We welcome pull requests, bug fixes and issue reports.

Before proposing a large change, first please discuss your change by raising an issue.

For API/code bugs, please include a small, self contained code example to reproduce the issue. For pull requests, remember test coverage.

We try to handle issues and pull requests with a 0 open philosophy. That means we will try to address the submission as soon as possible and will work toward a resolution. If progress can no longer be made (eg. unreproducible bug) or stops (eg. unresponsive submitter), we will close the bug.

Thanks.

sftp's Issues

corruption when reading files concurrently

When opening and reading files concurrently from the same client, files are not written correctly. Using io.Copy, the func (f *File) WriteTo(w io.Writer) (int64, error) method is invoked. The correct number of bytes 'written' is returned but the size of the local file does not match the remote file.

To reproduce:

1. open 2 files with the same client.
2. read both files concurrently.
3. check the size of the locally written files against the size of the remote files.

This was tested with an openssh server running sftp version 3.

Missing 'error message' in SSH_FXP_STATUS causing 'index out of range' panic

The SSH-2.0-Cisco-2.0 server implementation does not return the SSH_FXP_STATUS 'error message' in response to a ReadDir packet causing an "index out of range" panic. The sftp unix utility gracefully handles this case. I propose that the 'error message' is made optional in unmarshalStatus to handle this case.

Lenience with non-compliant servers?

I recently used this library for a client sftp setup. Their server software (unknown to us at the moment) disregards the relevant IETF document by not including the language tag in all its status messages. For this use case, I patched unmarshalStatus to use unmarshalStringSafe when reading the language tag from the status message which works fine.

The question now is whether this project is aiming for strict compliance, or if the patch I made is relevant to the project. sftp from OpenSSH seems to ignore that the language tag is missing, and I found that WinSCP had patched for similar issues with broken server software (http://winscp.net/tracker/show_bug.cgi?id=770).

Versioning would be wise

Hi guys,

Probably it would be wise to have some kind of versioning (branches/tags). I recently wrote a extension of the Afero filesystem (spf13/afero#59) project and it seemed it broken exactly 6 days ago after preparing for the new architecture. Probably it could be wise to have some versioning like this: http://labix.org/gopkg.in. The gopkg.in service may be used as a tag/branch to url which is go-gettable, people may use or not use this service but can also rely on the tags/branches itself so stuff wont break as development goes forward.

Then people can still use the tag/branch which includes hotfixes and dont have to vendor the sftp source into their own repo.

Let me know what you think.

TestClientRemoveDir failure on macosx 10.11.2

When running the integration tests against macosx's OpenSSH sftp server, the TestClientRemoveDir test fails. The server returns an SSH_FX_PERMISSION_DENIED error when attempting to remove the empty directory and, since this error is not SSH_FX_FAILURE, the remove directory path in Client#Remove is never attempted.

It appears that unlink on macosx will fail with EPERM when attempting to delete an empty directory and the sftp server is simply propagating that up.

$ mkdir empty-dir; sudo dtruss -f -t unlink unlink empty-dir

unlink: cannot unlink ‘empty-dir’: Operation not permitted
    PID/THRD  SYSCALL(args)          = return
38284/0x6b422:  unlink("empty-dir\0", 0x7F802240387C, 0x12)      = -1 Err#1

In practice this means that the Client#Remove can't be used to delete an empty directory on macosx as the sftp rmdir function isn't surfaced and the current implementation does not attempt a removeDirectory when removeFile fails with a permission error.

Seems like Remove should be attempting a removeDirectory when the path represents a directory regardless of the error from removeFile. Short of that, it may make sense to expose RemoveDirectory on the client.

Panic on file close

I was able to cause this panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x6baef4]

goroutine 174 [running]:
github.com/pkg/sftp.(*File).Close(0x0, 0x0, 0x0)
    /home/james/code/gopath/src/github.com/pkg/sftp/client.go:638 +0x24

The line in question is:
return f.c.close(f.handle)
https://github.com/pkg/sftp/blob/master/client.go#L638

and it seems in this case f.c was nil. woops!
I'm sure this was my fault for having a race in my code, but the panic actually happened in the library, not in my code, so perhaps there should be a lock around this check so that the close is always a safe operation?

If you agree, I'm happy to write this patch.

Can't remove directories on a Serv-U server

Commit e8b1445 merged RemoveDirectory() into the general Remove() call and tries to be smart by attempting an RMDIR call if REMOVE fails with an a FAILURE code.

This works ok for compliant servers, but Serv-U returns 0x18 when issuing a REMOVE call on a directory and Remove() fails as a result.

I'm not sure it really makes sense to merge RemoveFile and RemoveDirectory into one call anyway; it doesn't match the underlying protocol - Could we consider reversing that? Or at least adding an explicit RemoveDirectory() call if backwards compatibility is a concern.

client.Close hangs on channel recvClosed

When closing clients that are accessed concurrently from multiple goroutines, the client Close() method hangs waiting on a receive from c.recvClosed. This started occurring when a change was made in the client code to use clients concurrently from more than one goroutine. I suspect there is a deadlock occurring in the client recv() function, since this is the only function that closes the c.recvClosed channel. Requests may still be in progress when client Close is called, but this shouldn't cause the method to block indefinitely. I think the Close() method may need a way to cancel inflight requests. I'm happy to take a stab at a PR or do additional debugging on this but if anyone with more familiarity with the sftp package code can comment on why this is occurring I'd appreciate it.

partial stacktrace showing blocking on recvClosed:

goroutine 5 [chan receive, 1013 minutes]:
github.com/pkg/sftp.(*Client).Close(0xc25d62f590, 0x0, 0x0)
        /Users/rmcpherson/dev/go/src/github.com/pkg/sftp/client.go:100 +0x8d
github.com/codeguard/lister/client.(*SftpClient).Close(0xc208945e60, 0x0, 0x0)

Retrieve owner/group as per v03+ spec?

We are currently using this package with providers who do not offer meaningful GID's/UID's, but instead offer owners and groups via the v03+ protocol drafts. It would be really nice to be able to fetch those extended attributes. Has adding that functionality to this package been investigated?

client integration tests broken

Probably broken after #18

go test -integration -v 

shows unknown message 0 from sftp server

Proposal: remove all named and naked returns

Named returns can be useful for documentation purposes, but are often unneeded. Naked returns go hand-in-hand with named returns, and in my personal experience, only lead to confusing code.

I propose removing every named and naked return from this repository, with a few exceptions:

1. interface declarations, e.g.

type Filesystem interface {
    Open(path string) (info *os.FileInfo, err error)
}

2. functions which return several values of the same type, hypothetical example:

func Stat(path string) (freeSpace uint64, usedSpace uint64, totalSpace uint64, err error) { }

3. capturing panic recovery value into an error (haven't seen this at all in this repository, but it's occasionally useful)

Thoughts?

Cannot compile package with gccgo

I am trying to compile hugo, getting this:

github.com/pkg/sftp
# github.com/pkg/sftp

code/go/src/github.com/pkg/sftp/server_statvfs_linux.go:17:35: error: reference to undefined field or method ‘X__val’
   Fsid:    uint64(uint64(stat.Fsid.X__val[1])<<32 | uint64(stat.Fsid.X__val[0])), // endianness?
                                   ^
code/go/src/github.com/pkg/sftp/server_statvfs_linux.go:17:69: error: reference to undefined field or method ‘X__val’
   Fsid:    uint64(uint64(stat.Fsid.X__val[1])<<32 | uint64(stat.Fsid.X__val[0])), // endianness?

Severe performance regression

In the process of upgrading an application from 873d14f to 57fcf4a, we noticed that our application performed SFTP uploads roughly 5x slower than before (download speed, oddly enough, was greatly improved!)

Doing a full bisection against production is impractical for us, but I believe I've found a microbenchmark in this package that is indicative of this performance regression: BenchmarkWrite1k. Prior to 2960014, I got the following results on my laptop (Early 2013 15" MBP; Go 1.6.2):

$ go test -run NONE -bench BenchmarkWrite1k -integration -sftp /usr/libexec/sftp-server -count 10
PASS
BenchmarkWrite1k-8         5     257684948 ns/op      40.69 MB/s
BenchmarkWrite1k-8         5     258874739 ns/op      40.51 MB/s
BenchmarkWrite1k-8         5     257781516 ns/op      40.68 MB/s
BenchmarkWrite1k-8         5     258480088 ns/op      40.57 MB/s
BenchmarkWrite1k-8         5     254071386 ns/op      41.27 MB/s
BenchmarkWrite1k-8         5     258690412 ns/op      40.53 MB/s
BenchmarkWrite1k-8         5     298626463 ns/op      35.11 MB/s
BenchmarkWrite1k-8         5     258688959 ns/op      40.53 MB/s
BenchmarkWrite1k-8         5     255352022 ns/op      41.06 MB/s
BenchmarkWrite1k-8         5     262082224 ns/op      40.01 MB/s
ok      github.com/pkg/sftp 23.966s

After 2960014, I get:

$ go test -run NONE -bench BenchmarkWrite1k -integration -sftp /usr/libexec/sftp-server -count 10
PASS
BenchmarkWrite1k-8         1    2363737801 ns/op       4.44 MB/s
BenchmarkWrite1k-8         1    2401106692 ns/op       4.37 MB/s
BenchmarkWrite1k-8         1    2373019226 ns/op       4.42 MB/s
BenchmarkWrite1k-8         1    2316647765 ns/op       4.53 MB/s
BenchmarkWrite1k-8         1    1319017375 ns/op       7.95 MB/s
BenchmarkWrite1k-8         1    2357954728 ns/op       4.45 MB/s
BenchmarkWrite1k-8         1    2379846631 ns/op       4.41 MB/s
BenchmarkWrite1k-8         1    1364000038 ns/op       7.69 MB/s
BenchmarkWrite1k-8         1    1186599504 ns/op       8.84 MB/s
BenchmarkWrite1k-8         1    1259262555 ns/op       8.33 MB/s
ok      github.com/pkg/sftp 19.487s

Note that in each case I've cherry-picked 587af18cfc86e31167f25d5e12b9a3e588b8775b to get the package to compile on Darwin.

I've tried all my usual debugging and profiling tricks, and it's not obviously any of the usual suspects. Any tips about where to look next? I'm also happy to provide more information about our production application, or to test a small number of patches against production (although the microbenchmark seems pretty damning on its own)

Root directory passed to NewServer is never used or acted on

The NewServer constructor accepts and propagates a rootDir argument to the Server but the location is never used after construction. What was the intent of the argument? Could/should it be treated as the default working directory for the server instance?

File.Read work incorrectly

Hi, i trying to read file by sftp and it does not work.
On small files Read returns io.EOF, on large not full file content.
Here code example

Incoming packet was garbled on decryption w/ winscp

I'm using this code. It mostly works, except when copying a large file with WinSCP I get the error "Incoming packet was garbled on decryption" and I have to keep reconnecting.

According to the WinSCP docs this could be because of miscomputing SSH-2 encryption keys, or ignoring max packet lengths. I tried enabling both workarounds in WinSCP but to no avail.

Entirely possible my code is doing something wrong (I really have very little idea how SSH works; it's mostly copy-pasta). Buuut it could also be a bug in this code, so .... any ideas?

`fatal error: concurrent map writes' when uploading to SFTP

I have encountered the following error. I am on Go 1.6.2

Looking at the code it seems https://github.com/pkg/sftp/blob/master/conn.go#L32 should have protected this. However at the time on panic, the following trace is generated by 2 goroutines in this package.

https://github.com/pkg/sftp/blob/master/conn.go#L795

fatal error: concurrent map writes
goroutine 1 [running]:
runtime.throw(0x8df830, 0x15)
    /.BUILD/go/src/runtime/panic.go:547 +0x90 fp=0xc8204f91b0 sp=0xc8204f9198
runtime.mapassign1(0x7548c0, 0xc8200fe8d0, 0xc8204f9284, 0xc8204f9288)
    /.BUILD/go/src/runtime/hashmap.go:445 +0xb1 fp=0xc8204f9258 sp=0xc8204f91b0
gorpt/vendor/github.com/pkg/sftp.(*clientConn).dispatchRequest(0xc82001d1a0, 0xc8202d2300, 0x7f74dc258100, 0xc82064a300)
    /.BUILD/go-work/src/gorpt/vendor/github.com/pkg/sftp/conn.go:95 +0x9c fp=0xc8204f92d8 sp=0xc8204f9258
gorpt/vendor/github.com/pkg/sftp.(*File).ReadFrom(0xc8200ffb00, 0x7f74dc257b68, 0xc8200143f0, 0xc8204f9508, 0x0, 0x0)
    /.BUILD/go-work/src/gorpt/vendor/github.com/pkg/sftp/client.go:970 +0x333 fp=0xc8204f9488 sp=0xc8204f92d8
io.copyBuffer(0x7f74dc257b40, 0xc8200ffb00, 0x7f74dc257b68, 0xc8200143f0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /.BUILD/go/src/io/io.go:374 +0x180 fp=0xc8204f9548 sp=0xc8204f9488
io.Copy(0x7f74dc257b40, 0xc8200ffb00, 0x7f74dc257b68, 0xc8200143f0, 0xc8200143f0, 0x0, 0x0)
    /.BUILD/go/src/io/io.go:350 +0x64 fp=0xc8204f95a0 sp=0xc8204f9548
gorpt/merger.MergeSrcToTarget(0x7ffde5d618c6, 0x13, 0x7ffde5d61918, 0x16, 0x7ffde5d6193c, 0x12, 0xc8204a8730, 0x5, 0xc8, 0x7f74dc256988, ...)
...
...
    /.BUILD/go-work/src/gorpt/merger/main/main.go:101 +0x10aa fp=0xc8204f9f40 sp=0xc8204f9c60
runtime.main()
    /.BUILD/go/src/runtime/proc.go:188 +0x2b0 fp=0xc8204f9f90 sp=0xc8204f9f40
runtime.goexit()
    /.BUILD/go/src/runtime/asm_amd64.s:1998 +0x1 fp=0xc8204f9f98 sp=0xc8204f9f90

and

https://github.com/pkg/sftp/blob/master/conn.go#L70

goroutine 78 [chan send]:
gorpt/vendor/github.com/pkg/sftp.(*clientConn).recv(0xc82001d140, 0x0, 0x0)
    /.BUILD/go-work/src/gorpt/vendor/github.com/pkg/sftp/conn.go:70 +0x461
gorpt/vendor/github.com/pkg/sftp.(*clientConn).loop(0xc82001d140)
    /.BUILD/go-work/src/gorpt/vendor/github.com/pkg/sftp/conn.go:44 +0x5a
created by gorpt/vendor/github.com/pkg/sftp.NewClientPipe
    /.BUILD/go-work/src/gorpt/vendor/github.com/pkg/sftp/client.go:77 +0x2c1

Mkdir returns SSH_FX_FAILURE

Great little library, thanks, however I seem to be hitting a strange error. I have a script that generates a path for a directory in /tmp/ and then tries to Mkdir it. It often fails with:

sftp: "Failure" (SSH_FX_FAILURE)

I've had to run it in a loop, and it eventually succeeds after between 0 and 3 or so tries. What's going on? I'm running current git master of the code, and connecting to an F23 ssh host and using golang 1.5.4

Thanks!

conn.Open(file) opens the file read only, writing to the file causes the client to go into an infinite loop

Proposal: improve flexibility of Server type

I've been mulling over a few ideas to make the Server type a bit more flexible. I'd like to use a SFTP server in a project I'm doing, but unfortunately, it provides no transparency as to what the server is actually doing.

1. A simple approach would be to create a Filesystem interface for calls such as creating, reading, or writing a file, and making the default implementation use the os calls. This would provide flexibility to perform SFTP operations on top of any arbitrary Filesystem.

type Filesystem interface {
    Open(path string) (file *os.File, err error)
    Close() error
    // etc...
}

2. Another idea would be to add "callback" functions for some of the SFTP server's actions. If non-nil, the callback would be invoked whenever the action was performed.

s := &sftp.Server{
    OnOpen: func(path string) {
        log.Println("opened file:", path)
    },
}

3. Finally, perhaps it would be feasible to provide a similar interface to net/http, so that arbitrary sftp.Handler functions can be created to handle various request types.

type Handler struct {}

func (h *Handler) ServeSFTP(w sftp.Responder, r *sftp.Request) {
    log.Println("request:", r.Type)
    _, _ = sftp.PermissionDenied(w)
}

func main() {
    s := &sftp.Server{
        Handler: &Handler{},
    }

    _ = s.Serve()
}

I imagine that #1 and/or #3 would probably be the best ideas, and the most idiomatic as well. What do you think, @davecheney ? I'd be happy to try to spike out some pull requests if you're interested. Thanks for your time.

ReadFrom() deadlock

File: client.go, conn.go
References to #55 and #63

Checking the commited fix for issue #55 today I found out that the ReadFrom() deadlock is still existing. I digged a bit deeper and found the problem.

At the beginning of ReadFrom() function the ch channel is buffered with capacity of 1 result struct object. This was the fix introduced with #63.

func (f *File) ReadFrom(r io.Reader) (int64, error) {
    // ...
    offset := f.offset
    ch := make(chan result, 1) // this was fixed with #63, making the channel non-blocking
    // ...

Then data is read and packed into sshFxpWritePacket struct and dispatched by calling dispatchRequest().

       // ...
    for inFlight > 0 || firstErr == nil {
        for inFlight < desiredInFlight && firstErr == nil {
            n, err := r.Read(b)
            if err != nil {
                firstErr = err
            }
            f.c.dispatchRequest(ch, sshFxpWritePacket{
                ID:     f.c.nextID(),
                Handle: f.handle,
                Offset: offset,
                Length: uint32(n),
                Data:   b[:n],
            })
            inFlight++
                       //...

dispatchRequest() will send the packet data and returns to ReadFrom() for waiting on a result channel value. In case an error occures dispatchRequest() will return an error result to channel ch.

func (c *clientConn) dispatchRequest(ch chan<- result, p idmarshaler) {
    c.Lock()
    c.inflight[p.id()] = ch
    if err := c.conn.sendPacket(p); err != nil {
        delete(c.inflight, p.id())
        ch <- result{err: err} 
    }
    c.Unlock()
}

Back to ReadFrom() the function waits for a result on channel ch by using a blocking select.

select {
        case res := <-ch:
            inFlight--
            if res.err != nil {
                firstErr = res.err
                break
            }
            switch res.typ {
            case ssh_FXP_STATUS:
                id, _ := unmarshalUint32(res.data)
                err := normaliseError(unmarshalStatus(id, res.data))
                if err != nil && firstErr == nil {
                    firstErr = err
                    break
                }
                if desiredInFlight < maxConcurrentRequests {
                    desiredInFlight++
                }

In case there was no tx error inFlight is decreased (back to 0), but on the other hand desiredInFlight increased once per loop until maxConcurrentRequests are reached (which are by default 64).

As long as no error occures or EOF is reached, the outer for-loop (for inflight > 0 || firstErr == nil) continues. The inner for-loop however will now call dispatchRequest() with the previously assigned ch channel as long as inFlight < desiredInFlight.

Lets think about that you have a wonky SSH connection, but a few megabytes had already been transferred so that desiredInFlight var has reached the maximum of 64 concurrent requests. Suddenly the remote devices connection drops, i.e. dispatchRequest() queues an error result to ch, but the inner for-loop has not yet reached desiredInFlight count. The next dispatchRequest() call will also result in an error.

Now the bad thing that will happen here is since ch was made with size one the second dispatchRequest() error handling ch <- result{} assignment will block dispatchRequest call (ch is already allocated with the first error from the previous loop) and therefor deadlock ReadFrom() since the select{} where the channel is read will never be reached.

support client.OpenFile()

please support client.OpenFile() so that operations other than read are possible.

Doesn't compile on linux/s390x

Seems to be a similar problem to #121.

The problem is that the X__val member of syscall.Fsid is not exported on s390x. I've created an issue for this in go (golang/go#17298), but technically the syscall package is frozen, deprecated and marked dragons-be-here...

The line which doesn't compile is:

sftp/server_statvfs_linux.go

Line 19 in 8197a2e

Fsid: uint64(uint64(stat.Fsid.X__val[1])<<32 | uint64(stat.Fsid.X__val[0])), // endianness?

.

Would it be possible to do one of the following:

1. delete the line of code,
2. use the unix package instead of the syscall package (which I can then fix),
3. or add a gccgo-like exception for s390x?

Thanks!

No mechanism exists to provide a `start_directory` for an sftp server instance

I have a golang ssh daemon that includes the sftp server. When an sftp subsystem request comes in, an instance of the sftp server is created, wired to the session, and started. The sftp server runs in the same OS process as the ssh daemon so it inherits the working directory of the daemon. Since the sftp server is running in the working directory of the daemon, the initial working directory of sftp clients becomes the working directory of the daemon.

For user experience reasons (not for security or isolation reasons), when a user initiates an sftp session with the server, I would like the client to start in a "remote working directory" that is different from the working directory of the ssh daemon.

---

The openssh sftp server implementation has a start_directory option that can be specified on launch. When specified, a client of the server will be placed in the start_directory as their initial working directory.

For example, using the following script as my sftp-server wrapper:

#!/bin/sh

exec /usr/libexec/sftp-server $SFTP_SERVER_ARGS

I can change the starting directory that clients see:

$ export SFTP_SERVER_ARGS="-d /usr"; echo pwd | sftp -D $PWD/sftp-wrapper -b-
sftp> pwd
Remote working directory: /usr

$ export SFTP_SERVER_ARGS="-d /"; echo pwd | sftp -D $PWD/sftp-wrapper -b-
sftp> pwd
Remote working directory: /

$ export SFTP_SERVER_ARGS="-d /etc"; echo pwd | sftp -D $PWD/sftp-wrapper -b-
sftp> pwd
Remote working directory: /private/etc

The client observes the initial working directory by sending a realpath "." to the server:

$ export SFTP_SERVER_ARGS="-d /etc"; echo pwd | sftp -vvv -D $PWD/sftp-wrapper -b-
debug2: Remote version: 3
...
debug3: Sent message fd 3 T:16 I:1
debug3: SSH_FXP_REALPATH . -> /private/etc size 0
sftp> pwd
Remote working directory: /private/etc

A similar mechanism would be useful for pkg/sftp's server. Ideally the start_directory could be specified as a server option with the following characteristics:

1. The start directory does not rely on the OS level current working directory of the sftp process.
2. Client operations can rely on relative paths being treated as relative to the starting directory and not the OS current working directory of the process.

Enable Sourcegraph

I want to use Sourcegraph code search and code review with sftp. A project maintainer needs to enable it to set up a webhook so the code is up-to-date there.

Could you please enable sftp on @sourcegraph by going to https://sourcegraph.com/github.com/pkg/sftp and clicking on Settings? (It should only take 15 seconds.)

Thank you!

Can't determine if dir already exists with Mkdir

I'd like to determine if after running a mkdir if a failure is due to the directory already existing. I've done:

if status, ok := err.(*sftp.StatusError); ok {
    log.Printf("Code: %v, %v", status.Code, status.Error())
    if status.Code == ??? {
        XXX
    }
}

but unfortunately there doesn't seem to be enough information in StatusError or the Code to determine this. Can it be added?

Thanks!

Slow downloads compared to FileZilla

I noticed some serious difference when comparing the download speed via this package with the same download via FileZilla.

When downloading with Filezilla I can download at 30Mpbs+ with 10 concurrent files (over a VPN), but when downloading with the GO program directly on a high speed internet connection I can't get above the 20Mpbs with 16 concurrent downloads (or about 10 with 10 concurrent).

The readme already lists that currently all traffic with the server is serialized and I can imagine that this is influencing the download performance. But maybe also other factors as packet size and or compression can play a role here.

The code that is facilitating the download:

log.Printf("[%s] Starting to download", f)

// check if file already exists
if _, err := os.Stat(file); !os.IsNotExist(err) {
    log.Printf("[%s] Already downloaded", f)
    return
}

// open local file to save download
out, err := os.Create(file)
if err != nil {
    log.Printf("[%s] Error (%s)", f, err.Error())
    return
}
defer out.Close()

// open file to download
dl, err := s.Open(f)
if err != nil {
    log.Fatal(err)
}
defer dl.Close()

// copy the bytes
nb, err := io.Copy(out, dl)
if err != nil {
    log.Println("error downloading file", err)

    // delete
    err = os.Remove(file)
    if err != nil {
        log.Println("error deleting file after download error", err)
    }
    return
}

log.Printf("[%s] Download completed (%d bytes)", f, nb)

Execute SUDO prior to transfer

Is it possible to invoke an sudo prior to executing commands such as is possible in winscp?

http://winscp.net/eng/docs/faq_su

Integration test hangs when run with -testserver flag

Running on osx 10.11.2 with

$ go test -tags debug github.com/pkg/sftp -integration -testserver -v -sftp /usr/libexec/sftp-server -run TestClientWrite

the test hangs.

It looks like all 8 of the server side workers are blocked responding to write requests:

#1:
goroutine 23 [semacquire]:
sync.runtime_Semacquire(0xc82007d0f4)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:43 +0x26
sync.(*Mutex).Lock(0xc82007d0f0)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/mutex.go:82 +0x1c4
github.com/pkg/sftp.(*Server).sendPacket(0xc8200c7b90, 0x7402c8, 0xc8200da1e0, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:135 +0x35
github.com/pkg/sftp.sshFxpWritePacket.respond(0x58, 0xc820122200, 0x1, 0x277fff, 0x8000, 0xc82011a000, 0x8000, 0x8000, 0xc8200c7b90, 0x0, ...)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:445 +0x3e3
github.com/pkg/sftp.(*sshFxpWritePacket).respond(0xc820078c40, 0xc8200c7b90, 0x0, 0x0)
    <autogenerated>:57 +0xac
github.com/pkg/sftp.(*Server).sftpServerWorker(0xc8200c7b90, 0xc8200d3b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:138 +0x2ad
created by github.com/pkg/sftp.(*Server).Serve
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:148 +0xe0

#2:
goroutine 24 [semacquire]:
sync.runtime_Semacquire(0xc82007d0f4)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:43 +0x26
sync.(*Mutex).Lock(0xc82007d0f0)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/mutex.go:82 +0x1c4
github.com/pkg/sftp.(*Server).sendPacket(0xc8200c7b90, 0x7402c8, 0xc8200d4000, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:135 +0x35
github.com/pkg/sftp.sshFxpWritePacket.respond(0x5b, 0xc8200aa030, 0x1, 0x28ffff, 0x8000, 0xc8205fe000, 0x8000, 0x8000, 0xc8200c7b90, 0x0, ...)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:445 +0x3e3
github.com/pkg/sftp.(*sshFxpWritePacket).respond(0xc8200d0040, 0xc8200c7b90, 0x0, 0x0)
    <autogenerated>:57 +0xac
github.com/pkg/sftp.(*Server).sftpServerWorker(0xc8200c7b90, 0xc8200d3b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:138 +0x2ad
created by github.com/pkg/sftp.(*Server).Serve
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:148 +0xe0

#3:
goroutine 25 [semacquire]:
sync.runtime_Semacquire(0xc82007d0f4)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:43 +0x26
sync.(*Mutex).Lock(0xc82007d0f0)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/mutex.go:82 +0x1c4
github.com/pkg/sftp.(*Server).sendPacket(0xc8200c7b90, 0x7402c8, 0xc8200101b0, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:135 +0x35
github.com/pkg/sftp.sshFxpWritePacket.respond(0x55, 0xc8200e0170, 0x1, 0x25ffff, 0x8000, 0xc820296000, 0x8000, 0x8000, 0xc8200c7b90, 0x0, ...)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:445 +0x3e3
github.com/pkg/sftp.(*sshFxpWritePacket).respond(0xc82012e280, 0xc8200c7b90, 0x0, 0x0)
    <autogenerated>:57 +0xac
github.com/pkg/sftp.(*Server).sftpServerWorker(0xc8200c7b90, 0xc8200d3b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:138 +0x2ad
created by github.com/pkg/sftp.(*Server).Serve
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:148 +0xe0

#4:
goroutine 26 [semacquire]:
sync.runtime_Semacquire(0xc82007d0f4)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:43 +0x26
sync.(*Mutex).Lock(0xc82007d0f0)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/mutex.go:82 +0x1c4
github.com/pkg/sftp.(*Server).sendPacket(0xc8200c7b90, 0x7402c8, 0xc820104000, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:135 +0x35
github.com/pkg/sftp.sshFxpWritePacket.respond(0x59, 0xc82007c440, 0x1, 0x27ffff, 0x8000, 0xc8205a4000, 0x8000, 0x8000, 0xc8200c7b90, 0x0, ...)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:445 +0x3e3
github.com/pkg/sftp.(*sshFxpWritePacket).respond(0xc820014400, 0xc8200c7b90, 0x0, 0x0)
    <autogenerated>:57 +0xac
github.com/pkg/sftp.(*Server).sftpServerWorker(0xc8200c7b90, 0xc8200d3b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:138 +0x2ad
created by github.com/pkg/sftp.(*Server).Serve
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:148 +0xe0

#5:
goroutine 27 [semacquire]:
sync.runtime_Semacquire(0xc82007d0f4)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:43 +0x26
sync.(*Mutex).Lock(0xc82007d0f0)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/mutex.go:82 +0x1c4
github.com/pkg/sftp.(*Server).sendPacket(0xc8200c7b90, 0x7402c8, 0xc820010150, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:135 +0x35
github.com/pkg/sftp.sshFxpWritePacket.respond(0x5d, 0xc8200c4090, 0x1, 0x29ffff, 0x8000, 0xc82086a000, 0x8000, 0x8000, 0xc8200c7b90, 0x0, ...)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:445 +0x3e3
github.com/pkg/sftp.(*sshFxpWritePacket).respond(0xc8200c2140, 0xc8200c7b90, 0x0, 0x0)
    <autogenerated>:57 +0xac
github.com/pkg/sftp.(*Server).sftpServerWorker(0xc8200c7b90, 0xc8200d3b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:138 +0x2ad
created by github.com/pkg/sftp.(*Server).Serve
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:148 +0xe0

#6:
goroutine 28 [semacquire]:
sync.runtime_Semacquire(0xc82007d0f4)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:43 +0x26
sync.(*Mutex).Lock(0xc82007d0f0)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/mutex.go:82 +0x1c4
github.com/pkg/sftp.(*Server).sendPacket(0xc8200c7b90, 0x7402c8, 0xc8200d6000, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:135 +0x35
github.com/pkg/sftp.sshFxpWritePacket.respond(0x5e, 0xc8200a0070, 0x1, 0x2a7fff, 0x8000, 0xc820886000, 0x8000, 0x8000, 0xc8200c7b90, 0x0, ...)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:445 +0x3e3
github.com/pkg/sftp.(*sshFxpWritePacket).respond(0xc8201021c0, 0xc8200c7b90, 0x0, 0x0)
    <autogenerated>:57 +0xac
github.com/pkg/sftp.(*Server).sftpServerWorker(0xc8200c7b90, 0xc8200d3b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:138 +0x2ad
created by github.com/pkg/sftp.(*Server).Serve
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:148 +0xe0

#7:
goroutine 29 [semacquire]:
sync.runtime_Syncsemacquire(0xc8200f0138)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:237 +0x201
sync.(*Cond).Wait(0xc8200f0128)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/cond.go:62 +0x9b
io.(*pipe).write(0xc8200f00c0, 0xc8200c4230, 0x4, 0x4, 0x0, 0x0, 0x0)
    /usr/local/Cellar/go/1.5.2/libexec/src/io/pipe.go:94 +0x23a
io.(*PipeWriter).Write(0xc82008c048, 0xc8200c4230, 0x4, 0x4, 0x0, 0x0, 0x0)
    /usr/local/Cellar/go/1.5.2/libexec/src/io/pipe.go:161 +0x50
github.com/pkg/sftp.sendPacket(0x784ab8, 0xc82008c048, 0x7402c8, 0xc820010210, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:125 +0x222
github.com/pkg/sftp.(*Server).sendPacket(0xc8200c7b90, 0x7402c8, 0xc820010210, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:137 +0xc4
github.com/pkg/sftp.sshFxpWritePacket.respond(0x64, 0xc8200c41f0, 0x1, 0x2d7fff, 0x8000, 0xc820574000, 0x8000, 0x8000, 0xc8200c7b90, 0x0, ...)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:445 +0x3e3
github.com/pkg/sftp.(*sshFxpWritePacket).respond(0xc8200c2340, 0xc8200c7b90, 0x0, 0x0)
    <autogenerated>:57 +0xac
github.com/pkg/sftp.(*Server).sftpServerWorker(0xc8200c7b90, 0xc8200d3b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:138 +0x2ad
created by github.com/pkg/sftp.(*Server).Serve
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:148 +0xe0

#8:
goroutine 30 [semacquire]:
sync.runtime_Semacquire(0xc82007d0f4)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:43 +0x26
sync.(*Mutex).Lock(0xc82007d0f0)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/mutex.go:82 +0x1c4
github.com/pkg/sftp.(*Server).sendPacket(0xc8200c7b90, 0x7402c8, 0xc820100030, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:135 +0x35
github.com/pkg/sftp.sshFxpWritePacket.respond(0x5a, 0xc820012110, 0x1, 0x287fff, 0x8000, 0xc8205c0000, 0x8000, 0x8000, 0xc8200c7b90, 0x0, ...)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:445 +0x3e3
github.com/pkg/sftp.(*sshFxpWritePacket).respond(0xc8200de080, 0xc8200c7b90, 0x0, 0x0)
    <autogenerated>:57 +0xac
github.com/pkg/sftp.(*Server).sftpServerWorker(0xc8200c7b90, 0xc8200d3b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:138 +0x2ad
created by github.com/pkg/sftp.(*Server).Serve
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/server.go:148 +0xe0

While that's happening, the client is holding its mutex while trying to send a packet to the server (which has no workers to receive):

goroutine 20 [semacquire]:
sync.runtime_Syncsemacquire(0xc8200f0078)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:237 +0x201
sync.(*Cond).Wait(0xc8200f0068)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/cond.go:62 +0x9b
io.(*pipe).write(0xc8200f0000, 0xc8200aa050, 0x4, 0x4, 0x0, 0x0, 0x0)
    /usr/local/Cellar/go/1.5.2/libexec/src/io/pipe.go:94 +0x23a
io.(*PipeWriter).Write(0xc82008c038, 0xc8200aa050, 0x4, 0x4, 0x0, 0x0, 0x0)
    /usr/local/Cellar/go/1.5.2/libexec/src/io/pipe.go:161 +0x50
github.com/pkg/sftp.sendPacket(0x784ab8, 0xc82008c038, 0x7847d8, 0xc8200d0100, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/packet.go:125 +0x222
github.com/pkg/sftp.(*Client).dispatchRequest(0xc8200d92c0, 0xc82007a1e0, 0x740268, 0xc8200d0100)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/client.go:668 +0x13d
github.com/pkg/sftp.(*File).Write(0xc820010330, 0xc820774000, 0xd8000, 0xd8000, 0x200000, 0x0, 0x0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/client.go:980 +0x296
github.com/pkg/sftp.TestClientWrite(0xc8200c7b00)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/client_integration_test.go:1026 +0x547
testing.tRunner(0xc8200c7b00, 0x5141a8)
    /usr/local/Cellar/go/1.5.2/libexec/src/testing/testing.go:456 +0x98
created by testing.RunTests
    /usr/local/Cellar/go/1.5.2/libexec/src/testing/testing.go:561 +0x86d

And the server responses are not being processed by the client because the client is blocked trying to acquire the mutex that is currently held by the blocked send:

goroutine 3 [semacquire]:
sync.runtime_Semacquire(0xc8200d92f0)
    /usr/local/Cellar/go/1.5.2/libexec/src/runtime/sema.go:43 +0x26
sync.(*Mutex).Lock(0xc8200d92ec)
    /usr/local/Cellar/go/1.5.2/libexec/src/sync/mutex.go:82 +0x1c4
github.com/pkg/sftp.(*Client).recv(0xc8200d92c0)
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/client.go:166 +0x1dc
created by github.com/pkg/sftp.NewClientPipe
    /Users/sykesm/workspace/diego-release/src/github.com/pkg/sftp/client.go:76 +0x2d6

Provide some FS API for Server

In my project I would like to use github.com/pkg/sftp as an embedded SFTP server.
Would be nice to make Server more extendable in terms of working with FS or having the following features:

1. Restrict user to use only specific directory (e.g. chroot)
2. Provide some kind of notifications when file upload is completed or user session is closed

In my pull request #94 I've moved all file operations into FileStorageBackend and made it replaceable. Maybe this solution is little bit dirty so let's workshop here the better solution

Some SFTP appliances return file does not exist for directories during a Remove

Similar to #62, non-compliant servers return unexpected results when you try to call Remove on a directory. Maybe it is best to expose RemoveDirectory, as I bet there's endless servers out there doing all kinds of weird things. The specific server I'm dealing with winds up with error:

"file does not exist"

I did not have time to track down which specific server this is, or what packet that error correlates with, but I do know with the standard openssh client an rmdir works fine, but an rm fails with the same response.

FileInfo.IsDir() returns wrong, on windows

I have a AIX ssh server, and the client is windows.

infos, err := client.ReadDir(dirname)
for _, info := range infos {
println(info.IsDir())
}

here, info.IsDir() always return false on windows. However if I use a linux or mac client, IsDir() returns correctly.

blowfish requested by sftp server.

I am having issues connecting with this type of SFTP server:
http://docs.ipswitch.com/WS_FTP_Server71/ReleaseNotes/index.htm?k_id=ipswitch_com_ftp_documents_worldwide_ws_ftpserverv71releasenotes

I can connect with filezilla fine with settings:
Protocol: "SFTP - SSH File Transfer Protocol"
Logon Type: "Normal"
Username: ***
Password: ***

SO i am using this method call:

sshcfg := &ssh.ClientConfig{
        User: user,
        Auth: []ssh.AuthMethod{
            ssh.Password(password),
        },
    }

When i hit: func (c *connection) clientAuthenticate(config *ClientConfig) error {

I get back err:

ssh: handshake failed: ssh: no common algorithm for client to server cipher; client offered: [aes128-ctr aes192-ctr aes256-ctr [email protected] arcfour256 arcfour128], server offered: [3des-cbc blowfish-cbc aes256-cbc aes128-cbc cast128-cbc]

SO looking at : https://github.com/golang/crypto/blob/master/ssh/cipher.go#L95
It looks like its not implemented yet ?

Is there a way to enable compression?

I've been looking into ways to use compression (zlib) with sftp package. But as far as I understand Go ssh package does not support that. Am I missing anything?

sftp.File read error, cannot read file large than 16777216 (2^24)

first, create a file larger than 2^24 byte

dd if=/dev/zero of=/tmp/zero.img bs=1M count=32

the golang code below demonstrates the bug:

// c is a sftp.Client

    rName := "/tmp/zero.img"

    fi, err := c.Lstat(rName)
    if err != nil {
        log.Fatal(err)
    }

    fp, err := c.Open(rName)
    if err != nil {
        log.Fatal(err)
    }
    defer fp.Close()

    n, err := io.Copy(ioutil.Discard, fp)
    if err != nil {
        log.Fatal(err)
    }
    if n != fi.Size() {
        log.Fatalf("copy %d != %d remote size", n, fi.Size())
    }

This means I cannot use the sftp package as a sftp client to get large files from the server.

Way to detect underlying SSH connection has closed

It would be great if there was something similar to ssh.ClientConn.Wait() for this package. Even exposing the underlying ClientConn's Wait() would be fine.

The use case here is to detect ssh failures and re-initialize the connection. Since a new session is created when using sftp.NewClient, I can't just update the *ssh.ClientConn in the background and have everything work. I'd need to also re-start a new sftp subsystem session.

Licensing and reuse

I am working on implementing a simple SFTP server and I would like to reuse the types and marshaling logic in the code. But since they are not exported, I have no choice but to copy them in my own package. How does the licensing clause apply to this condition? Do I copy the LICENSE file in my repo as well?

Latest commit broke the windows build

build on mac works nicely, but cross compile for windows and it blows up. I also tried it on a windows box, and received similar error. Roll back the latest PR, and all is well.

sftp $ go install
sftp $ env GOOS="windows" GOARCH="386" go install
# github.com/pkg/sftp
./attrs.go:134: undefined: syscall.Stat_t
./server_stubs.go:5: undefined: os in os.FileInfo
./server_stubs.go:6: undefined: path in path.Join

Manually marshal packets for moar speed

Thank you for this package, it's really useful !

In my last toy project the profiler showed that most of the time was spent in marshaling packets, which is unsurprising given that this package uses reflection. It turns out that uploading was ~40% slower than good ol' sftp on the command line.

I did some experiments and marshaled packets by hand for some of them, and here are the results:

SSH_FX_INIT packet:

BenchmarkMarshalInitReflect      1000000              1219 ns/op             292 B/op          7 allocs/op
BenchmarkMarshalInitManual       5000000               577 ns/op             231 B/op          4 allocs/op

SSH_FXP_OPEN packet:

BenchmarkMarshalOpenReflect      1000000              1193 ns/op             339 B/op          6 allocs/op
BenchmarkMarshalOpenManual       5000000               525 ns/op             230 B/op          3 allocs/op

SSH_FXP_WRITE packet with 32k of data (worst case):

BenchmarkMarshalWriteWorstCaseReflect       1000           1867246 ns/op          195184 B/op         24 allocs/op
BenchmarkMarshalWriteWorstCaseManual       50000             35216 ns/op           82100 B/op          4 allocs/op

SSH_FXP_WRITE packet with 1k of data:

BenchmarkMarshalWrite1kReflect     50000             58526 ns/op            4656 B/op         12 allocs/op
BenchmarkMarshalWrite1kManual    1000000              2330 ns/op            2612 B/op          4 allocs/op

As you can see, there could be some interest in evaluating this; I guess at least 50% win in "fixed size" packets, much much more for "variable size" packets. I figured since packet types are all known and won't change anytime soon, there is value in having some logic "hardcoded".

Here's what it looks like for a SSH_FXP_INIT packet:

type ssh_fx_init_packet struct {
    Type       byte
    Version    uint32
    Extensions []struct {
        Name, Data string
    }
}

func (s ssh_fx_init_packet) MarshalBinary() ([]byte, error) {
    l := 1 + 4
    for _, e := range s.Extensions {
        l += 4 + len(e.Name) + 4 + len(e.Data)
    }

    b := make([]byte, 0, l)
    b = append(b, s.Type)
    b = marshalUint32(b, s.Version)
    for _, e := range s.Extensions {
        b = marshalString(b, e.Name)
        b = marshalString(b, e.Data)
    }
    return b, nil
}

It's kind of ugly, but there are 2 things of note here:

• We know the structure, so we manually marshal (as said before)
• We know the total length, so we can specify the cap at the beginning and avoid further reallocs

I tried to be "generic" in using the encoding.BinaryMarshaler interface, although I personally have no idea what this specific interface is used for in other projects...

Is this kind of change worth a PR ?

Blocking channel on i/o error blocks client (endless loop)

File: client.go
Method: ReadFrom()

You create ch as a blocking channel (ch := make(chan result)) and pass it over to dispatchRequest(ch, ...). In case that an i/o error occures at sendPacket(), which is called by dispatchRequest, your error handling puts the error result into blocking channel ch. Parsing the blocked channel by select{} is done after the dispatchRequest() call so your client will block/freeze at this point.

Cheers and thanks for the greate package!
Nekron

Details:

func (f *File) ReadFrom(r io.Reader) (int64, error) {
    inFlight := 0
    desiredInFlight := 1
    offset := f.offset
    ch := make(chan result)  // you create a blocking channel here
    var firstErr error
    read := int64(0)
    b := make([]byte, f.c.maxPacket)
    for inFlight > 0 || firstErr == nil {
        for inFlight < desiredInFlight && firstErr == nil {
            n, err := r.Read(b)
            if err != nil {
                firstErr = err
            }
            f.c.dispatchRequest(ch, sshFxpWritePacket{  // you call dispatchRequest with the blocking channel as argument
                Id:     f.c.nextId(),
                Handle: f.handle,
                Offset: offset,
                Length: uint32(n),
                Data:   b[:n],
            })
            inFlight++
            offset += uint64(n)
            read += int64(n)
        }

        if inFlight == 0 {
            break
        }
        select {   // after dispatchRequest returns you evaluate blocking channels result which in case of an i/o error would never be reached
        case res := <-ch:

Quick workaround is to launch the ch <- result{err: err} as a goroutine to give your select{} block a chance to evaluate the result.

func (c *Client) dispatchRequest(ch chan<- result, p idmarshaler) {
    c.mu.Lock()
    c.inflight[p.id()] = ch
    if err := sendPacket(c.w, p); err != nil {
        delete(c.inflight, p.id())
        c.mu.Unlock()
        go func() { ch <- result{err: err} }() // if not launched as a goroutine ch will block your client forever!
        return
    }
    c.mu.Unlock()
}

SSH_FX_FAILURE for removing (some) directories

Like #35, I get for some directories I would like to remove, an SSH_FX_FAILURE error. I updated your package's Go code yesterday. Therefore, I should have the fix of #35. What can I do in order to analyse this issue?

At the moment, all directories with this error uses UTF8 characters, e.g. umlauts like ö, ü, ä, etc.

As I am writing this, I got an idea: I switched from Windows 10 to Mac OS X, thus, my Go program runs now under Mac. I recognised yesterday, that the same Go code handles such file names with umlauts differently (Windows vs Mac). Or, at least, both operating systems deliver different strings to Go and therefore to this library. Because: All remote directories, which I cannot delete right now (SSH_FX_FAILURE) are created from the Go program under Windows 10 and cannot deleted from within Mac OS.

Perhaps, this post helps some other users/developers. I will investigate this issue further and can close this issue later, if I had a solution. Thanks.

Slow speed

Hi
We were trying to upload a 10MB file to a file on AWS EC2 machines and it takes about 9mins to complete,however, it takes less than 1min with command line on the same machine. Any suggestions that can help to improve the speed? Thanks!!

km

As a user of sftp.Client, how do I determine the current working directory of relative paths?

The working directory is important when using relative paths with the sftp client. Standard sftp clients provide a pwd command to enable users to discover what the current working directory is and they implement this by requesting the server to execute the realpath operation on the "." path. I can find no existing mechanism in the current client to derive this information. Please see #58 as possible resolution.

Also, #58 addresses a bug where sshFxpRealpathPacket#MarshalBinary uses the wrong operation number (READLINK instead of REALPATH).

Return meaningful error when subsystem request for sftp failed

Hi,

I'm writing a backup program (restic) which uses the sftp library. Recently, a user reported an issue with using his NAS with restic over sftp.

It turned out that sftp was deactivated for the NAS, but restic only printed a generic error message:

subsystem request failed on channel 0

I'd like to be able to catch this error and replace it with a more meaningful message for my users.

Unfortunately, the original error (from ssh/session) is created like this:

err = errors.New("ssh: subsystem request failed")

and the sftp library just passes it up the stack (here:

if err := s.RequestSubsystem("sftp"); err != nil {
    return nil, err
}

So in order to distinguish this error from others, the only way is to match against the string. That's ugly.

Would you accept a PR which returns a custom wrapper error type when RequestSubsystem returns an error? Something along the lines:

type subsystemRequestFailed string

func (err subsystemRequestFailed) Error() string {
   return string(err)
}

func (err subsystemRequestFailed) SubsystemRequestFailed() bool {
   return true
}

[...]
// NewClient creates a new SFTP client on conn, using zero or more option
// functions.
func NewClient(conn *ssh.Client, opts ...func(*Client) error) (*Client, error) {
    s, err := conn.NewSession()
    if err != nil {
        return nil, err
    }
    if err := s.RequestSubsystem("sftp"); err != nil {
        return nil, subsystemRequestFailed(err.Error())
    }
    pw, err := s.StdinPipe()
    if err != nil {
        return nil, err
    }
    pr, err := s.StdoutPipe()
    if err != nil {
        return nil, err
    }

    return NewClientPipe(pr, pw, opts...)
}

[...]

Then the caller can test the behavior of the error by checking if it implements the method SubsystemRequestFailed().

Does that sound good? Any better idea for the method name?

file.Close() panics when internal variable is nil

https://github.com/pkg/sftp/blob/master/client.go#L627

If a call to SFTPServer.NewFile produces an error, a non-nil WriterCloser is still returned...and subsequent calls to Close() (like in a defer statement) causes a panic due to the internal variable c being nil.

Wanted: Docs on using NewClientPipe over TCP/TLS

In the documentation for NewClientPipe it says:

NewClientPipe creates a new SFTP client given a Reader and a WriteCloser. This can be used for connecting to an SFTP server over TCP/TLS or by using the system's ssh client program (e.g. via exec.Command).

AFAIS, the example below only covers the case when using the system's ssh client program. It would be awesome with a simple example or some pointers to how to connect over TCP/TLS directly too! :)

Provide a net/http-like API for the Server type

This will be an issue to track progress for providing a net/http-like API for the Server type.

TODO list:

• create sftp.Handler interface
  • https://golang.org/pkg/net/http/#Handler
• create sftp.Request type with request information
  • https://golang.org/pkg/net/http/#Request
• create sftp.Responder interface to respond to clients
  • https://golang.org/pkg/net/http/#ResponseWriter
  • name is tentative, suggestions appreciated
• create sftp.FileServer type which can serve files from the local filesystem, as is done now.
  • enables generic interface backend for fronting many different "filesystems" with SFTP
  • could also implement chroot-like functionality this way
• figure out how to cleanly integrate underlying SSH server configuration
  • retrieve information like SSH username

Ideas:

• add sftp.ServeMux type for using different handlers for different requests, e.g. create, open, read, write: https://golang.org/pkg/net/http/#ServeMux

Here's a couple sketches of what this might look like from previous samples:

type Handler struct {}

func (h *Handler) ServeSFTP(w sftp.Responder, r *sftp.Request) {
    log.Println("request:", r.Type)
    _, _ = sftp.PermissionDenied(w)
}

func main() {
    s := &sftp.Server{
        Handler: &Handler{},
    }

    _ = s.Serve()
}

func ServeSFTP(w sftp.Responder, r *sftp.Request) {
    if !authenticated(r) {
        sftp.FileServer(sftp.Dir("/srv/sftpchroot/"))(w, r)
        return
    }

    sftp.FileServer(sftp.Dir("/"))(w, r)
}

cc: @davecheney @marksheahan @xor-gate @punkeel

(*Client).Stat and (*Client).Open don't return os.ErrNotExist for nonexistent paths

Running the following test:

go test --integration

func TestNonexistentPath(t *testing.T) {
        sftp, cmd := testClient(t, READONLY, NO_DELAY)
        defer cmd.Wait()
        defer sftp.Close()

        if _, err := sftp.Stat("/doesnt/exist/"); !os.IsNotExist(err) {
                t.Errorf("os.IsNotExist(%v) = false, want true", err)
        }

        if _, err := sftp.Open("/doesnt/exist/"); !os.IsNotExist(err) {
                t.Errorf("os.IsNotExist(%v) = false, want true", err)
        }
}

Results in

--- FAIL: TestNonexistentPath (0.01s)
        client_integration_test.go:171: os.IsNotExist(sftp: "No such file" (SSH_FX_NO_SUCH_FILE)) = false, want true
        client_integration_test.go:175: os.IsNotExist(sftp: "No such file" (SSH_FX_NO_SUCH_FILE)) = false, want true
process_write: write failed
FAIL
exit status 1
FAIL        github.com/pkg/sftp     2.149s

Rename is broken?

Nevermind...

Remote host path separators would be nicer

Walking a directory of a Linux server on a Windows client results in strangeness like:

\home\user\.user
\home\user\.bashrc
\home\user\hello.txt

etc ...

This seems to be due to:
path := filepath.Join(w.cur.path, list[i].Name())
in the Step() method of Walker.

It's not immediately obvious to me how to fix this since the Separator is a constant in os.filepath.
Perhaps just augment the sftp.item type with a RemoteSeparator and implement:
Join(elem ...string) string
method on that type?

This is a really nice library BTW.