diff -Nru golang-github-pkg-sftp-1.7.0/attrs_unix.go golang-github-pkg-sftp-1.8.3/attrs_unix.go --- golang-github-pkg-sftp-1.7.0/attrs_unix.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/attrs_unix.go 2018-09-17 22:22:55.000000000 +0000 @@ -1,4 +1,4 @@ -// +build darwin dragonfly freebsd !android,linux netbsd openbsd solaris +// +build darwin dragonfly freebsd !android,linux netbsd openbsd solaris aix // +build cgo package sftp diff -Nru golang-github-pkg-sftp-1.7.0/client.go golang-github-pkg-sftp-1.8.3/client.go --- golang-github-pkg-sftp-1.7.0/client.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/client.go 2018-09-17 22:22:55.000000000 +0000 @@ -76,6 +76,19 @@ return MaxPacketChecked(size) } +// MaxConcurrentRequestsPerFile sets the maximum concurrent requests allowed for a single file. +// +// The default maximum concurrent requests is 64. +func MaxConcurrentRequestsPerFile(n int) ClientOption { + return func(c *Client) error { + if n < 1 { + return errors.Errorf("n must be greater or equal to 1") + } + c.maxConcurrentRequests = n + return nil + } +} + // NewClient creates a new SFTP client on conn, using zero or more option // functions. func NewClient(conn *ssh.Client, opts ...ClientOption) (*Client, error) { @@ -110,7 +123,8 @@ }, inflight: make(map[uint32]chan<- result), }, - maxPacket: 1 << 15, + maxPacket: 1 << 15, + maxConcurrentRequests: 64, } if err := sftp.applyOptions(opts...); err != nil { wr.Close() @@ -137,8 +151,9 @@ type Client struct { clientConn - maxPacket int // max packet size read or written. - nextid uint32 + maxPacket int // max packet size read or written. + nextid uint32 + maxConcurrentRequests int } // Create creates the named file mode 0666 (before umask), truncating it if it @@ -759,12 +774,15 @@ return f.path } -const maxConcurrentRequests = 64 - // Read reads up to len(b) bytes from the File. It returns the number of bytes // read and an error, if any. Read follows io.Reader semantics, so when Read // encounters an error or EOF condition after successfully reading n > 0 bytes, // it returns the number of bytes read. +// +// To maximise throughput for transferring the entire file (especially +// over high latency links) it is recommended to use WriteTo rather +// than calling Read multiple times. io.Copy will do this +// automatically. func (f *File) Read(b []byte) (int, error) { // Split the read into multiple maxPacket sized concurrent reads // bounded by maxConcurrentRequests. This allows reads with a suitably @@ -775,7 +793,7 @@ offset := f.offset // maxConcurrentRequests buffer to deal with broadcastErr() floods // also must have a buffer of max value of (desiredInFlight - inFlight) - ch := make(chan result, maxConcurrentRequests+1) + ch := make(chan result, f.c.maxConcurrentRequests+1) type inflightRead struct { b []byte offset uint64 @@ -840,7 +858,7 @@ if n < len(req.b) { sendReq(req.b[l:], req.offset+uint64(l)) } - if desiredInFlight < maxConcurrentRequests { + if desiredInFlight < f.c.maxConcurrentRequests { desiredInFlight++ } default: @@ -860,6 +878,10 @@ // WriteTo writes the file to w. The return value is the number of bytes // written. Any error encountered during the write is also returned. +// +// This method is preferred over calling Read multiple times to +// maximise throughput for transferring the entire file (especially +// over high latency links). func (f *File) WriteTo(w io.Writer) (int64, error) { fi, err := f.Stat() if err != nil { @@ -871,7 +893,7 @@ writeOffset := offset fileSize := uint64(fi.Size()) // see comment on same line in Read() above - ch := make(chan result, maxConcurrentRequests+1) + ch := make(chan result, f.c.maxConcurrentRequests+1) type inflightRead struct { b []byte offset uint64 @@ -951,7 +973,7 @@ switch { case offset > fileSize: desiredInFlight = 1 - case desiredInFlight < maxConcurrentRequests: + case desiredInFlight < f.c.maxConcurrentRequests: desiredInFlight++ } writeOffset += uint64(nbytes) @@ -1005,6 +1027,11 @@ // Write writes len(b) bytes to the File. It returns the number of bytes // written and an error, if any. Write returns a non-nil error when n != // len(b). +// +// To maximise throughput for transferring the entire file (especially +// over high latency links) it is recommended to use ReadFrom rather +// than calling Write multiple times. io.Copy will do this +// automatically. func (f *File) Write(b []byte) (int, error) { // Split the write into multiple maxPacket sized concurrent writes // bounded by maxConcurrentRequests. This allows writes with a suitably @@ -1014,7 +1041,7 @@ desiredInFlight := 1 offset := f.offset // see comment on same line in Read() above - ch := make(chan result, maxConcurrentRequests+1) + ch := make(chan result, f.c.maxConcurrentRequests+1) var firstErr error written := len(b) for len(b) > 0 || inFlight > 0 { @@ -1050,7 +1077,7 @@ firstErr = err break } - if desiredInFlight < maxConcurrentRequests { + if desiredInFlight < f.c.maxConcurrentRequests { desiredInFlight++ } default: @@ -1070,12 +1097,16 @@ // ReadFrom reads data from r until EOF and writes it to the file. The return // value is the number of bytes read. Any error except io.EOF encountered // during the read is also returned. +// +// This method is preferred over calling Write multiple times to +// maximise throughput for transferring the entire file (especially +// over high latency links). func (f *File) ReadFrom(r io.Reader) (int64, error) { inFlight := 0 desiredInFlight := 1 offset := f.offset // see comment on same line in Read() above - ch := make(chan result, maxConcurrentRequests+1) + ch := make(chan result, f.c.maxConcurrentRequests+1) var firstErr error read := int64(0) b := make([]byte, f.c.maxPacket) @@ -1114,7 +1145,7 @@ firstErr = err break } - if desiredInFlight < maxConcurrentRequests { + if desiredInFlight < f.c.maxConcurrentRequests { desiredInFlight++ } default: diff -Nru golang-github-pkg-sftp-1.7.0/debian/changelog golang-github-pkg-sftp-1.8.3/debian/changelog --- golang-github-pkg-sftp-1.7.0/debian/changelog 2018-05-14 07:25:25.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/debian/changelog 2019-01-01 07:31:00.000000000 +0000 @@ -1,3 +1,11 @@ +golang-github-pkg-sftp (1.8.3-1) unstable; urgency=medium + + * New upstream version 1.8.3 + * Update Maintainer email address to team+pkg-go@tracker.debian.org + * Bump Standards-Version to 4.3.0 (no change) + + -- Anthony Fok Tue, 01 Jan 2019 00:31:00 -0700 + golang-github-pkg-sftp (1.7.0-1) unstable; urgency=medium * New upstream version 1.7.0 diff -Nru golang-github-pkg-sftp-1.7.0/debian/control golang-github-pkg-sftp-1.8.3/debian/control --- golang-github-pkg-sftp-1.7.0/debian/control 2018-04-10 13:25:33.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/debian/control 2019-01-01 07:25:31.000000000 +0000 @@ -1,5 +1,5 @@ Source: golang-github-pkg-sftp -Maintainer: Debian Go Packaging Team +Maintainer: Debian Go Packaging Team Uploaders: Anthony Fok Section: devel Testsuite: autopkgtest-pkg-go @@ -11,7 +11,7 @@ golang-github-pkg-errors-dev, golang-github-stretchr-testify-dev, golang-golang-x-crypto-dev -Standards-Version: 4.1.4 +Standards-Version: 4.3.0 Vcs-Browser: https://salsa.debian.org/go-team/packages/golang-github-pkg-sftp Vcs-Git: https://salsa.debian.org/go-team/packages/golang-github-pkg-sftp.git Homepage: https://github.com/pkg/sftp diff -Nru golang-github-pkg-sftp-1.7.0/packet.go golang-github-pkg-sftp-1.8.3/packet.go --- golang-github-pkg-sftp-1.7.0/packet.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/packet.go 2018-09-17 22:22:55.000000000 +0000 @@ -125,15 +125,14 @@ } else if debugDumpTxPacket { debug("send packet: %s %d bytes", fxp(bb[0]), len(bb)) } - l := uint32(len(bb)) - hdr := []byte{byte(l >> 24), byte(l >> 16), byte(l >> 8), byte(l)} - _, err = w.Write(hdr) - if err != nil { - return errors.Errorf("failed to send packet header: %v", err) - } - _, err = w.Write(bb) + // Slide packet down 4 bytes to make room for length header. + packet := append(bb, make([]byte, 4)...) // optimistically assume bb has capacity + copy(packet[4:], bb) + binary.BigEndian.PutUint32(packet[:4], uint32(len(bb))) + + _, err = w.Write(packet) if err != nil { - return errors.Errorf("failed to send packet body: %v", err) + return errors.Errorf("failed to send packet: %v", err) } return nil } @@ -882,9 +881,9 @@ return p.SpecificPacket.readonly() } -func (p sshFxpExtendedPacket) respond(svr *Server) error { +func (p sshFxpExtendedPacket) respond(svr *Server) responsePacket { if p.SpecificPacket == nil { - return nil + return statusFromError(p, nil) } return p.SpecificPacket.respond(svr) } @@ -954,7 +953,7 @@ return nil } -func (p sshFxpExtendedPacketPosixRename) respond(s *Server) error { +func (p sshFxpExtendedPacketPosixRename) respond(s *Server) responsePacket { err := os.Rename(p.Oldpath, p.Newpath) - return s.sendError(p, err) + return statusFromError(p, err) } diff -Nru golang-github-pkg-sftp-1.7.0/packet-manager.go golang-github-pkg-sftp-1.8.3/packet-manager.go --- golang-github-pkg-sftp-1.7.0/packet-manager.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/packet-manager.go 2018-09-17 22:22:55.000000000 +0000 @@ -7,30 +7,30 @@ ) // The goal of the packetManager is to keep the outgoing packets in the same -// order as the incoming. This is due to some sftp clients requiring this -// behavior (eg. winscp). +// order as the incoming as is requires by section 7 of the RFC. -type packetSender interface { - sendPacket(encoding.BinaryMarshaler) error +type packetManager struct { + requests chan orderedPacket + responses chan orderedPacket + fini chan struct{} + incoming orderedPackets + outgoing orderedPackets + sender packetSender // connection object + working *sync.WaitGroup + packetCount uint32 } -type packetManager struct { - requests chan requestPacket - responses chan responsePacket - fini chan struct{} - incoming requestPacketIDs - outgoing responsePackets - sender packetSender // connection object - working *sync.WaitGroup +type packetSender interface { + sendPacket(encoding.BinaryMarshaler) error } func newPktMgr(sender packetSender) *packetManager { s := &packetManager{ - requests: make(chan requestPacket, SftpServerWorkerCount), - responses: make(chan responsePacket, SftpServerWorkerCount), + requests: make(chan orderedPacket, SftpServerWorkerCount), + responses: make(chan orderedPacket, SftpServerWorkerCount), fini: make(chan struct{}), - incoming: make([]uint32, 0, SftpServerWorkerCount), - outgoing: make([]responsePacket, 0, SftpServerWorkerCount), + incoming: make([]orderedPacket, 0, SftpServerWorkerCount), + outgoing: make([]orderedPacket, 0, SftpServerWorkerCount), sender: sender, working: &sync.WaitGroup{}, } @@ -38,31 +38,56 @@ return s } -type responsePackets []responsePacket +//// packet ordering +func (s *packetManager) newOrderId() uint32 { + s.packetCount++ + return s.packetCount +} -func (r responsePackets) Sort() { - sort.Slice(r, func(i, j int) bool { - return r[i].id() < r[j].id() - }) +type orderedRequest struct { + requestPacket + orderid uint32 } -type requestPacketIDs []uint32 +func (s *packetManager) newOrderedRequest(p requestPacket) orderedRequest { + return orderedRequest{requestPacket: p, orderid: s.newOrderId()} +} +func (p orderedRequest) orderId() uint32 { return p.orderid } +func (p orderedRequest) setOrderId(oid uint32) { p.orderid = oid } -func (r requestPacketIDs) Sort() { - sort.Slice(r, func(i, j int) bool { - return r[i] < r[j] +type orderedResponse struct { + responsePacket + orderid uint32 +} + +func (s *packetManager) newOrderedResponse(p responsePacket, id uint32, +) orderedResponse { + return orderedResponse{responsePacket: p, orderid: id} +} +func (p orderedResponse) orderId() uint32 { return p.orderid } +func (p orderedResponse) setOrderId(oid uint32) { p.orderid = oid } + +type orderedPacket interface { + id() uint32 + orderId() uint32 +} +type orderedPackets []orderedPacket + +func (o orderedPackets) Sort() { + sort.Slice(o, func(i, j int) bool { + return o[i].orderId() < o[j].orderId() }) } +//// packet registry // register incoming packets to be handled -// send id of 0 for packets without id -func (s *packetManager) incomingPacket(pkt requestPacket) { +func (s *packetManager) incomingPacket(pkt orderedRequest) { s.working.Add(1) - s.requests <- pkt // buffer == SftpServerWorkerCount + s.requests <- pkt } // register outgoing packets as being ready -func (s *packetManager) readyPacket(pkt responsePacket) { +func (s *packetManager) readyPacket(pkt orderedResponse) { s.responses <- pkt s.working.Done() } @@ -75,38 +100,37 @@ } // Passed a worker function, returns a channel for incoming packets. -// The goal is to process packets in the order they are received as is -// requires by section 7 of the RFC, while maximizing throughput of file -// transfers. -func (s *packetManager) workerChan(runWorker func(requestChan)) requestChan { +// Keep process packet responses in the order they are received while +// maximizing throughput of file transfers. +func (s *packetManager) workerChan(runWorker func(chan orderedRequest), +) chan orderedRequest { - rwChan := make(chan requestPacket, SftpServerWorkerCount) + // multiple workers for faster read/writes + rwChan := make(chan orderedRequest, SftpServerWorkerCount) for i := 0; i < SftpServerWorkerCount; i++ { runWorker(rwChan) } - cmdChan := make(chan requestPacket) + // single worker to enforce sequential processing of everything else + cmdChan := make(chan orderedRequest) runWorker(cmdChan) - pktChan := make(chan requestPacket, SftpServerWorkerCount) + pktChan := make(chan orderedRequest, SftpServerWorkerCount) go func() { - // start with cmdChan - curChan := cmdChan for pkt := range pktChan { - // on file open packet, switch to rwChan - switch pkt.(type) { - case *sshFxpOpenPacket: - curChan = rwChan - // on file close packet, switch back to cmdChan - // after waiting for any reads/writes to finish + switch pkt.requestPacket.(type) { + case *sshFxpReadPacket, *sshFxpWritePacket: + s.incomingPacket(pkt) + rwChan <- pkt + continue case *sshFxpClosePacket: - // wait for rwChan to finish + // wait for reads/writes to finish when file is closed + // incomingPacket() call must occur after this s.working.Wait() - // stop using rwChan - curChan = cmdChan } s.incomingPacket(pkt) - curChan <- pkt + // all non-RW use sequential cmdChan + cmdChan <- pkt } close(rwChan) close(cmdChan) @@ -121,17 +145,13 @@ for { select { case pkt := <-s.requests: - debug("incoming id: %v", pkt.id()) - s.incoming = append(s.incoming, pkt.id()) - if len(s.incoming) > 1 { - s.incoming.Sort() - } + debug("incoming id (oid): %v (%v)", pkt.id(), pkt.orderId()) + s.incoming = append(s.incoming, pkt) + s.incoming.Sort() case pkt := <-s.responses: - debug("outgoing pkt: %v", pkt.id()) + debug("outgoing id (oid): %v (%v)", pkt.id(), pkt.orderId()) s.outgoing = append(s.outgoing, pkt) - if len(s.outgoing) > 1 { - s.outgoing.Sort() - } + s.outgoing.Sort() case <-s.fini: return } @@ -149,10 +169,11 @@ } out := s.outgoing[0] in := s.incoming[0] - // debug("incoming: %v", s.incoming) - // debug("outgoing: %v", outfilter(s.outgoing)) - if in == out.id() { - s.sender.sendPacket(out) + // debug("incoming: %v", ids(s.incoming)) + // debug("outgoing: %v", ids(s.outgoing)) + if in.orderId() == out.orderId() { + debug("Sending packet: %v", out.id()) + s.sender.sendPacket(out.(encoding.BinaryMarshaler)) // pop off heads copy(s.incoming, s.incoming[1:]) // shift left s.incoming = s.incoming[:len(s.incoming)-1] // remove last @@ -164,10 +185,17 @@ } } -//func outfilter(o []responsePacket) []uint32 { -// res := make([]uint32, 0, len(o)) -// for _, v := range o { -// res = append(res, v.id()) -// } -// return res -//} +// func oids(o []orderedPacket) []uint32 { +// res := make([]uint32, 0, len(o)) +// for _, v := range o { +// res = append(res, v.orderId()) +// } +// return res +// } +// func ids(o []orderedPacket) []uint32 { +// res := make([]uint32, 0, len(o)) +// for _, v := range o { +// res = append(res, v.id()) +// } +// return res +// } diff -Nru golang-github-pkg-sftp-1.7.0/packet-manager_test.go golang-github-pkg-sftp-1.8.3/packet-manager_test.go --- golang-github-pkg-sftp-1.7.0/packet-manager_test.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/packet-manager_test.go 2018-09-17 22:22:55.000000000 +0000 @@ -21,7 +21,14 @@ return nil } -type fakepacket uint32 +type fakepacket struct { + reqid uint32 + oid uint32 +} + +func fake(rid, order uint32) fakepacket { + return fakepacket{reqid: rid, oid: order} +} func (fakepacket) MarshalBinary() ([]byte, error) { return []byte{}, nil @@ -32,40 +39,51 @@ } func (f fakepacket) id() uint32 { - return uint32(f) + return f.reqid } type pair struct { - in fakepacket - out fakepacket + in, out fakepacket +} + +type ordered_pair struct { + in orderedRequest + out orderedResponse } // basic test var ttable1 = []pair{ - pair{fakepacket(0), fakepacket(0)}, - pair{fakepacket(1), fakepacket(1)}, - pair{fakepacket(2), fakepacket(2)}, - pair{fakepacket(3), fakepacket(3)}, + pair{fake(0, 0), fake(0, 0)}, + pair{fake(1, 1), fake(1, 1)}, + pair{fake(2, 2), fake(2, 2)}, + pair{fake(3, 3), fake(3, 3)}, } // outgoing packets out of order var ttable2 = []pair{ - pair{fakepacket(0), fakepacket(0)}, - pair{fakepacket(1), fakepacket(4)}, - pair{fakepacket(2), fakepacket(1)}, - pair{fakepacket(3), fakepacket(3)}, - pair{fakepacket(4), fakepacket(2)}, + pair{fake(10, 0), fake(12, 2)}, + pair{fake(11, 1), fake(11, 1)}, + pair{fake(12, 2), fake(13, 3)}, + pair{fake(13, 3), fake(10, 0)}, } -// incoming packets out of order +// request ids are not incremental var ttable3 = []pair{ - pair{fakepacket(2), fakepacket(0)}, - pair{fakepacket(1), fakepacket(1)}, - pair{fakepacket(3), fakepacket(2)}, - pair{fakepacket(0), fakepacket(3)}, + pair{fake(7, 0), fake(7, 0)}, + pair{fake(1, 1), fake(1, 1)}, + pair{fake(9, 2), fake(3, 3)}, + pair{fake(3, 3), fake(9, 2)}, } -var tables = [][]pair{ttable1, ttable2, ttable3} +// request ids are all the same +var ttable4 = []pair{ + pair{fake(1, 0), fake(1, 0)}, + pair{fake(1, 1), fake(1, 1)}, + pair{fake(1, 2), fake(1, 3)}, + pair{fake(1, 3), fake(1, 2)}, +} + +var tables = [][]pair{ttable1, ttable2, ttable3, ttable4} func TestPacketManager(t *testing.T) { sender := newTestSender() @@ -73,30 +91,37 @@ for i := range tables { table := tables[i] + ordered_pairs := make([]ordered_pair, 0, len(table)) for _, p := range table { + ordered_pairs = append(ordered_pairs, ordered_pair{ + in: orderedRequest{p.in, p.in.oid}, + out: orderedResponse{p.out, p.out.oid}, + }) + } + for _, p := range ordered_pairs { s.incomingPacket(p.in) } - for _, p := range table { + for _, p := range ordered_pairs { s.readyPacket(p.out) } - for i := 0; i < len(table); i++ { + for _, p := range table { pkt := <-sender.sent - id := pkt.(fakepacket).id() - assert.Equal(t, id, uint32(i)) + id := pkt.(orderedResponse).id() + assert.Equal(t, id, p.in.id()) } } s.close() } func (p sshFxpRemovePacket) String() string { - return fmt.Sprintf("RmPct:%d", p.ID) + return fmt.Sprintf("RmPkt:%d", p.ID) } func (p sshFxpOpenPacket) String() string { - return fmt.Sprintf("OpPct:%d", p.ID) + return fmt.Sprintf("OpPkt:%d", p.ID) } func (p sshFxpWritePacket) String() string { - return fmt.Sprintf("WrPct:%d", p.ID) + return fmt.Sprintf("WrPkt:%d", p.ID) } func (p sshFxpClosePacket) String() string { - return fmt.Sprintf("ClPct:%d", p.ID) + return fmt.Sprintf("ClPkt:%d", p.ID) } diff -Nru golang-github-pkg-sftp-1.7.0/packet-typing.go golang-github-pkg-sftp-1.8.3/packet-typing.go --- golang-github-pkg-sftp-1.7.0/packet-typing.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/packet-typing.go 2018-09-17 22:22:55.000000000 +0000 @@ -12,8 +12,6 @@ id() uint32 } -type requestChan chan requestPacket - type responsePacket interface { encoding.BinaryMarshaler id() uint32 @@ -77,6 +75,7 @@ func (p sshFxpStatResponse) id() uint32 { return p.ID } func (p sshFxpNamePacket) id() uint32 { return p.ID } func (p sshFxpHandlePacket) id() uint32 { return p.ID } +func (p StatVFS) id() uint32 { return p.ID } func (p sshFxVersionPacket) id() uint32 { return 0 } // take raw incoming packet data and build packet objects diff -Nru golang-github-pkg-sftp-1.7.0/request-example.go golang-github-pkg-sftp-1.8.3/request-example.go --- golang-github-pkg-sftp-1.7.0/request-example.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/request-example.go 2018-09-17 22:22:55.000000000 +0000 @@ -83,6 +83,7 @@ return &os.LinkError{Op: "rename", Old: r.Filepath, New: r.Target, Err: fmt.Errorf("dest file exists")} } + file.name = r.Target fs.files[r.Target] = file delete(fs.files, r.Filepath) case "Rmdir", "Remove": diff -Nru golang-github-pkg-sftp-1.7.0/request.go golang-github-pkg-sftp-1.8.3/request.go --- golang-github-pkg-sftp-1.7.0/request.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/request.go 2018-09-17 22:22:55.000000000 +0000 @@ -116,34 +116,12 @@ } // manage file read/write state -func (r *Request) setWriterState(wa io.WriterAt) { - r.state.Lock() - defer r.state.Unlock() - r.state.writerAt = wa -} -func (r *Request) setReaderState(ra io.ReaderAt) { - r.state.Lock() - defer r.state.Unlock() - r.state.readerAt = ra -} func (r *Request) setListerState(la ListerAt) { r.state.Lock() defer r.state.Unlock() r.state.listerAt = la } -func (r *Request) getWriter() io.WriterAt { - r.state.RLock() - defer r.state.RUnlock() - return r.state.writerAt -} - -func (r *Request) getReader() io.ReaderAt { - r.state.RLock() - defer r.state.RUnlock() - return r.state.readerAt -} - func (r *Request) getLister() ListerAt { r.state.RLock() defer r.state.RUnlock() @@ -152,17 +130,23 @@ // Close reader/writer if possible func (r *Request) close() error { - rd := r.getReader() + defer func() { + if r.cancelCtx != nil { + r.cancelCtx() + } + }() + r.state.RLock() + rd := r.state.readerAt + r.state.RUnlock() if c, ok := rd.(io.Closer); ok { return c.Close() } - wt := r.getWriter() + r.state.RLock() + wt := r.state.writerAt + r.state.RUnlock() if c, ok := wt.(io.Closer); ok { return c.Close() } - if r.cancelCtx != nil { - r.cancelCtx() - } return nil } @@ -175,8 +159,10 @@ return fileput(handlers.FilePut, r, pkt) case "Setstat", "Rename", "Rmdir", "Mkdir", "Symlink", "Remove": return filecmd(handlers.FileCmd, r, pkt) - case "List", "Stat", "Readlink": + case "List": return filelist(handlers.FileList, r, pkt) + case "Stat", "Readlink": + return filestat(handlers.FileList, r, pkt) default: return statusFromError(pkt, errors.Errorf("unexpected method: %s", r.Method)) @@ -200,13 +186,20 @@ // wrap FileReader handler func fileget(h FileReader, r *Request, pkt requestPacket) responsePacket { var err error - reader := r.getReader() + r.state.RLock() + reader := r.state.readerAt + r.state.RUnlock() if reader == nil { - reader, err = h.Fileread(r) - if err != nil { - return statusFromError(pkt, err) + r.state.Lock() + if r.state.readerAt == nil { + r.state.readerAt, err = h.Fileread(r) + if err != nil { + r.state.Unlock() + return statusFromError(pkt, err) + } } - r.setReaderState(reader) + reader = r.state.readerAt + r.state.Unlock() } _, offset, length := packetData(pkt) @@ -226,13 +219,20 @@ // wrap FileWriter handler func fileput(h FileWriter, r *Request, pkt requestPacket) responsePacket { var err error - writer := r.getWriter() + r.state.RLock() + writer := r.state.writerAt + r.state.RUnlock() if writer == nil { - writer, err = h.Filewrite(r) - if err != nil { - return statusFromError(pkt, err) + r.state.Lock() + if r.state.writerAt == nil { + r.state.writerAt, err = h.Filewrite(r) + if err != nil { + r.state.Unlock() + return statusFromError(pkt, err) + } } - r.setWriterState(writer) + writer = r.state.writerAt + r.state.Unlock() } data, offset, _ := packetData(pkt) @@ -290,6 +290,22 @@ }) } return ret + default: + err = errors.Errorf("unexpected method: %s", r.Method) + return statusFromError(pkt, err) + } +} + +func filestat(h FileLister, r *Request, pkt requestPacket) responsePacket { + lister, err := h.Filelist(r) + if err != nil { + return statusFromError(pkt, err) + } + finfo := make([]os.FileInfo, 1) + n, err := lister.ListAt(finfo, 0) + finfo = finfo[:n] // avoid need for nil tests below + + switch r.Method { case "Stat": if err != nil && err != io.EOF { return statusFromError(pkt, err) @@ -336,8 +352,10 @@ method = "Put" case *sshFxpReaddirPacket: method = "List" - case *sshFxpOpenPacket, *sshFxpOpendirPacket: + case *sshFxpOpenPacket: method = "Open" + case *sshFxpOpendirPacket: + method = "Stat" case *sshFxpSetstatPacket, *sshFxpFsetstatPacket: method = "Setstat" case *sshFxpRenamePacket: diff -Nru golang-github-pkg-sftp-1.7.0/request-interfaces.go golang-github-pkg-sftp-1.8.3/request-interfaces.go --- golang-github-pkg-sftp-1.7.0/request-interfaces.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/request-interfaces.go 2018-09-17 22:22:55.000000000 +0000 @@ -8,8 +8,14 @@ // Interfaces are differentiated based on required returned values. // All input arguments are to be pulled from Request (the only arg). +// The Handler interfaces all take the Request object as its only argument. +// All the data you should need to handle the call are in the Request object. +// The request.Method attribute is initially the most important one as it +// determines which Handler gets called. + // FileReader should return an io.ReaderAt for the filepath // Note in cases of an error, the error text will be sent to the client. +// Called for Methods: Get type FileReader interface { Fileread(*Request) (io.ReaderAt, error) } @@ -19,18 +25,21 @@ // The request server code will call Close() on the returned io.WriterAt // ojbect if an io.Closer type assertion succeeds. // Note in cases of an error, the error text will be sent to the client. +// Called for Methods: Put, Open type FileWriter interface { Filewrite(*Request) (io.WriterAt, error) } // FileCmder should return an error // Note in cases of an error, the error text will be sent to the client. +// Called for Methods: Setstat, Rename, Rmdir, Mkdir, Symlink, Remove type FileCmder interface { Filecmd(*Request) error } // FileLister should return an object that fulfils the ListerAt interface // Note in cases of an error, the error text will be sent to the client. +// Called for Methods: List, Stat, Readlink type FileLister interface { Filelist(*Request) (ListerAt, error) } diff -Nru golang-github-pkg-sftp-1.7.0/request-server.go golang-github-pkg-sftp-1.8.3/request-server.go --- golang-github-pkg-sftp-1.7.0/request-server.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/request-server.go 2018-09-17 22:22:55.000000000 +0000 @@ -2,8 +2,8 @@ import ( "context" - "encoding" "io" + "os" "path" "path/filepath" "strconv" @@ -105,7 +105,7 @@ ctx, cancel := context.WithCancel(context.Background()) defer cancel() var wg sync.WaitGroup - runWorker := func(ch requestChan) { + runWorker := func(ch chan orderedRequest) { wg.Add(1) go func() { defer wg.Done() @@ -142,7 +142,7 @@ } } - pktChan <- pkt + pktChan <- rs.pktMgr.newOrderedRequest(pkt) } close(pktChan) // shuts down sftpServerWorkers @@ -159,13 +159,13 @@ } func (rs *RequestServer) packetWorker( - ctx context.Context, pktChan chan requestPacket, + ctx context.Context, pktChan chan orderedRequest, ) error { for pkt := range pktChan { var rpkt responsePacket - switch pkt := pkt.(type) { + switch pkt := pkt.requestPacket.(type) { case *sshFxInitPacket: - rpkt = sshFxVersionPacket{sftpProtocolVersion, nil} + rpkt = sshFxVersionPacket{Version: sftpProtocolVersion} case *sshFxpClosePacket: handle := pkt.getHandle() rpkt = statusFromError(pkt, rs.closeRequest(handle)) @@ -173,12 +173,20 @@ rpkt = cleanPacketPath(pkt) case *sshFxpOpendirPacket: request := requestFromPacket(ctx, pkt) - handle := rs.nextRequest(request) - rpkt = sshFxpHandlePacket{pkt.id(), handle} + rpkt = request.call(rs.Handlers, pkt) + if stat, ok := rpkt.(*sshFxpStatResponse); ok { + if stat.info.IsDir() { + handle := rs.nextRequest(request) + rpkt = sshFxpHandlePacket{ID: pkt.id(), Handle: handle} + } else { + rpkt = statusFromError(pkt, &os.PathError{ + Path: request.Filepath, Err: syscall.ENOTDIR}) + } + } case *sshFxpOpenPacket: request := requestFromPacket(ctx, pkt) handle := rs.nextRequest(request) - rpkt = sshFxpHandlePacket{pkt.id(), handle} + rpkt = sshFxpHandlePacket{ID: pkt.id(), Handle: handle} if pkt.hasPflags(ssh_FXF_CREAT) { if p := request.call(rs.Handlers, pkt); !statusOk(p) { rpkt = p // if error in write, return it @@ -200,10 +208,8 @@ return errors.Errorf("unexpected packet type %T", pkt) } - err := rs.sendPacket(rpkt) - if err != nil { - return err - } + rs.pktMgr.readyPacket( + rs.pktMgr.newOrderedResponse(rpkt, pkt.orderId())) } return nil } @@ -235,13 +241,3 @@ } return path.Clean(p) } - -// Wrap underlying connection methods to use packetManager -func (rs *RequestServer) sendPacket(m encoding.BinaryMarshaler) error { - if pkt, ok := m.(responsePacket); ok { - rs.pktMgr.readyPacket(pkt) - } else { - return errors.Errorf("unexpected packet type %T", m) - } - return nil -} diff -Nru golang-github-pkg-sftp-1.7.0/request-server_test.go golang-github-pkg-sftp-1.8.3/request-server_test.go --- golang-github-pkg-sftp-1.7.0/request-server_test.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/request-server_test.go 2018-09-17 22:22:55.000000000 +0000 @@ -242,10 +242,11 @@ assert.Nil(t, err) err = p.cli.Rename("/foo", "/bar") assert.Nil(t, err) - _, err = r.fetch("/bar") + f, err := r.fetch("/bar") + assert.Equal(t, "bar", f.Name()) assert.Nil(t, err) _, err = r.fetch("/foo") - assert.Equal(t, err, os.ErrNotExist) + assert.Equal(t, os.ErrNotExist, err) } func TestRequestRenameFail(t *testing.T) { @@ -355,6 +356,11 @@ _, err := putTestFile(p.cli, fname, fname) assert.Nil(t, err) } + _, err := p.cli.ReadDir("/foo_01") + assert.Equal(t, &StatusError{Code: ssh_FX_FAILURE, + msg: " /foo_01: not a directory"}, err) + _, err = p.cli.ReadDir("/does_not_exist") + assert.Equal(t, os.ErrNotExist, err) di, err := p.cli.ReadDir("/") assert.Nil(t, err) assert.Len(t, di, 100) diff -Nru golang-github-pkg-sftp-1.7.0/request_test.go golang-github-pkg-sftp-1.8.3/request_test.go --- golang-github-pkg-sftp-1.7.0/request_test.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/request_test.go 2018-09-17 22:22:55.000000000 +0000 @@ -134,7 +134,8 @@ request := testRequest("Get") // req.length is 5, so we test reads in 5 byte chunks for i, txt := range []string{"file-", "data."} { - pkt := &sshFxpReadPacket{uint32(i), "a", uint64(i * 5), 5} + pkt := &sshFxpReadPacket{ID: uint32(i), Handle: "a", + Offset: uint64(i * 5), Len: 5} rpkt := request.call(handlers, pkt) dpkt := rpkt.(*sshFxpDataPacket) assert.Equal(t, dpkt.id(), uint32(i)) @@ -155,10 +156,12 @@ func TestRequestPut(t *testing.T) { handlers := newTestHandlers() request := testRequest("Put") - pkt := &sshFxpWritePacket{0, "a", 0, 5, []byte("file-")} + pkt := &sshFxpWritePacket{ID: 0, Handle: "a", Offset: 0, Length: 5, + Data: []byte("file-")} rpkt := request.call(handlers, pkt) checkOkStatus(t, rpkt) - pkt = &sshFxpWritePacket{1, "a", 5, 5, []byte("data.")} + pkt = &sshFxpWritePacket{ID: 1, Handle: "a", Offset: 5, Length: 5, + Data: []byte("data.")} rpkt = request.call(handlers, pkt) checkOkStatus(t, rpkt) assert.Equal(t, "file-data.", handlers.getOutString()) @@ -176,8 +179,6 @@ assert.Equal(t, rpkt, statusFromError(rpkt, errTest)) } -func TestRequestInfoList(t *testing.T) { testInfoMethod(t, "List") } -func TestRequestInfoReadlink(t *testing.T) { testInfoMethod(t, "Readlink") } func TestRequestInfoStat(t *testing.T) { handlers := newTestHandlers() request := testRequest("Stat") @@ -188,6 +189,8 @@ assert.Equal(t, spkt.info.Name(), "request_test.go") } +func TestRequestInfoList(t *testing.T) { testInfoMethod(t, "List") } +func TestRequestInfoReadlink(t *testing.T) { testInfoMethod(t, "Readlink") } func testInfoMethod(t *testing.T, method string) { handlers := newTestHandlers() request := testRequest(method) @@ -198,3 +201,16 @@ assert.IsType(t, sshFxpNameAttr{}, npkt.NameAttrs[0]) assert.Equal(t, npkt.NameAttrs[0].Name, "request_test.go") } + +func TestOpendirHandleReuse(t *testing.T) { + handlers := newTestHandlers() + request := testRequest("Stat") + pkt := fakePacket{myid: 1} + rpkt := request.call(handlers, pkt) + assert.IsType(t, &sshFxpStatResponse{}, rpkt) + + request.Method = "List" + pkt = fakePacket{myid: 2} + rpkt = request.call(handlers, pkt) + assert.IsType(t, &sshFxpNamePacket{}, rpkt) +} diff -Nru golang-github-pkg-sftp-1.7.0/server.go golang-github-pkg-sftp-1.8.3/server.go --- golang-github-pkg-sftp-1.7.0/server.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/server.go 2018-09-17 22:22:55.000000000 +0000 @@ -66,7 +66,7 @@ type serverRespondablePacket interface { encoding.BinaryUnmarshaler id() uint32 - respond(svr *Server) error + respond(svr *Server) responsePacket } // NewServer creates a new Server instance around the provided streams, serving @@ -123,12 +123,11 @@ } // Up to N parallel servers -func (svr *Server) sftpServerWorker(pktChan chan requestPacket) error { +func (svr *Server) sftpServerWorker(pktChan chan orderedRequest) error { for pkt := range pktChan { - // readonly checks readonly := true - switch pkt := pkt.(type) { + switch pkt := pkt.requestPacket.(type) { case notReadOnly: readonly = false case *sshFxpOpenPacket: @@ -140,9 +139,9 @@ // If server is operating read-only and a write operation is requested, // return permission denied if !readonly && svr.readOnly { - if err := svr.sendError(pkt, syscall.EPERM); err != nil { - return errors.Wrap(err, "failed to send read only packet response") - } + svr.sendPacket(orderedResponse{ + responsePacket: statusFromError(pkt, syscall.EPERM), + orderid: pkt.orderId()}) continue } @@ -153,135 +152,145 @@ return nil } -func handlePacket(s *Server, p interface{}) error { - switch p := p.(type) { +func handlePacket(s *Server, p orderedRequest) error { + var rpkt responsePacket + switch p := p.requestPacket.(type) { case *sshFxInitPacket: - return s.sendPacket(sshFxVersionPacket{sftpProtocolVersion, nil}) + rpkt = sshFxVersionPacket{Version: sftpProtocolVersion} case *sshFxpStatPacket: // stat the requested file info, err := os.Stat(p.Path) - if err != nil { - return s.sendError(p, err) - } - return s.sendPacket(sshFxpStatResponse{ + rpkt = sshFxpStatResponse{ ID: p.ID, info: info, - }) + } + if err != nil { + rpkt = statusFromError(p, err) + } case *sshFxpLstatPacket: // stat the requested file info, err := os.Lstat(p.Path) - if err != nil { - return s.sendError(p, err) - } - return s.sendPacket(sshFxpStatResponse{ + rpkt = sshFxpStatResponse{ ID: p.ID, info: info, - }) + } + if err != nil { + rpkt = statusFromError(p, err) + } case *sshFxpFstatPacket: f, ok := s.getHandle(p.Handle) - if !ok { - return s.sendError(p, syscall.EBADF) + var err error = syscall.EBADF + var info os.FileInfo + if ok { + info, err = f.Stat() + rpkt = sshFxpStatResponse{ + ID: p.ID, + info: info, + } } - - info, err := f.Stat() if err != nil { - return s.sendError(p, err) + rpkt = statusFromError(p, err) } - - return s.sendPacket(sshFxpStatResponse{ - ID: p.ID, - info: info, - }) case *sshFxpMkdirPacket: // TODO FIXME: ignore flags field err := os.Mkdir(p.Path, 0755) - return s.sendError(p, err) + rpkt = statusFromError(p, err) case *sshFxpRmdirPacket: err := os.Remove(p.Path) - return s.sendError(p, err) + rpkt = statusFromError(p, err) case *sshFxpRemovePacket: err := os.Remove(p.Filename) - return s.sendError(p, err) + rpkt = statusFromError(p, err) case *sshFxpRenamePacket: err := os.Rename(p.Oldpath, p.Newpath) - return s.sendError(p, err) + rpkt = statusFromError(p, err) case *sshFxpSymlinkPacket: err := os.Symlink(p.Targetpath, p.Linkpath) - return s.sendError(p, err) + rpkt = statusFromError(p, err) case *sshFxpClosePacket: - return s.sendError(p, s.closeHandle(p.Handle)) + rpkt = statusFromError(p, s.closeHandle(p.Handle)) case *sshFxpReadlinkPacket: f, err := os.Readlink(p.Path) - if err != nil { - return s.sendError(p, err) - } - - return s.sendPacket(sshFxpNamePacket{ + rpkt = sshFxpNamePacket{ ID: p.ID, NameAttrs: []sshFxpNameAttr{{ Name: f, LongName: f, Attrs: emptyFileStat, }}, - }) - - case *sshFxpRealpathPacket: - f, err := filepath.Abs(p.Path) + } if err != nil { - return s.sendError(p, err) + rpkt = statusFromError(p, err) } + case *sshFxpRealpathPacket: + f, err := filepath.Abs(p.Path) f = cleanPath(f) - return s.sendPacket(sshFxpNamePacket{ + rpkt = sshFxpNamePacket{ ID: p.ID, NameAttrs: []sshFxpNameAttr{{ Name: f, LongName: f, Attrs: emptyFileStat, }}, - }) + } + if err != nil { + rpkt = statusFromError(p, err) + } case *sshFxpOpendirPacket: - return sshFxpOpenPacket{ - ID: p.ID, - Path: p.Path, - Pflags: ssh_FXF_READ, - }.respond(s) + if stat, err := os.Stat(p.Path); err != nil { + rpkt = statusFromError(p, err) + } else if !stat.IsDir() { + rpkt = statusFromError(p, &os.PathError{ + Path: p.Path, Err: syscall.ENOTDIR}) + } else { + rpkt = sshFxpOpenPacket{ + ID: p.ID, + Path: p.Path, + Pflags: ssh_FXF_READ, + }.respond(s) + } case *sshFxpReadPacket: + var err error = syscall.EBADF f, ok := s.getHandle(p.Handle) - if !ok { - return s.sendError(p, syscall.EBADF) + if ok { + err = nil + data := make([]byte, clamp(p.Len, s.maxTxPacket)) + n, _err := f.ReadAt(data, int64(p.Offset)) + if _err != nil && (_err != io.EOF || n == 0) { + err = _err + } + rpkt = sshFxpDataPacket{ + ID: p.ID, + Length: uint32(n), + Data: data[:n], + } + } + if err != nil { + rpkt = statusFromError(p, err) } - data := make([]byte, clamp(p.Len, s.maxTxPacket)) - n, err := f.ReadAt(data, int64(p.Offset)) - if err != nil && (err != io.EOF || n == 0) { - return s.sendError(p, err) - } - return s.sendPacket(sshFxpDataPacket{ - ID: p.ID, - Length: uint32(n), - Data: data[:n], - }) case *sshFxpWritePacket: f, ok := s.getHandle(p.Handle) - if !ok { - return s.sendError(p, syscall.EBADF) + var err error = syscall.EBADF + if ok { + _, err = f.WriteAt(p.Data, int64(p.Offset)) } - - _, err := f.WriteAt(p.Data, int64(p.Offset)) - return s.sendError(p, err) + rpkt = statusFromError(p, err) case serverRespondablePacket: - err := p.respond(s) - return errors.Wrap(err, "pkt.respond failed") + rpkt = p.respond(s) default: return errors.Errorf("unexpected packet type %T", p) } + + s.pktMgr.readyPacket(s.pktMgr.newOrderedResponse(rpkt, p.orderId())) + return nil } // Serve serves SFTP connections until the streams stop or the SFTP subsystem // is stopped. func (svr *Server) Serve() error { var wg sync.WaitGroup - runWorker := func(ch requestChan) { + runWorker := func(ch chan orderedRequest) { wg.Add(1) go func() { defer wg.Done() @@ -318,7 +327,7 @@ } } - pktChan <- pkt + pktChan <- svr.pktMgr.newOrderedRequest(pkt) } close(pktChan) // shuts down sftpServerWorkers @@ -332,20 +341,6 @@ return err // error from recvPacket } -// Wrap underlying connection methods to use packetManager -func (svr *Server) sendPacket(m encoding.BinaryMarshaler) error { - if pkt, ok := m.(responsePacket); ok { - svr.pktMgr.readyPacket(pkt) - } else { - return errors.Errorf("unexpected packet type %T", m) - } - return nil -} - -func (svr *Server) sendError(p ider, err error) error { - return svr.sendPacket(statusFromError(p, err)) -} - type ider interface { id() uint32 } @@ -380,7 +375,7 @@ return true } -func (p sshFxpOpenPacket) respond(svr *Server) error { +func (p sshFxpOpenPacket) respond(svr *Server) responsePacket { var osFlags int if p.hasPflags(ssh_FXF_READ, ssh_FXF_WRITE) { osFlags |= os.O_RDWR @@ -390,7 +385,7 @@ osFlags |= os.O_RDONLY } else { // how are they opening? - return svr.sendError(p, syscall.EINVAL) + return statusFromError(p, syscall.EINVAL) } if p.hasPflags(ssh_FXF_APPEND) { @@ -408,23 +403,23 @@ f, err := os.OpenFile(p.Path, osFlags, 0644) if err != nil { - return svr.sendError(p, err) + return statusFromError(p, err) } handle := svr.nextHandle(f) - return svr.sendPacket(sshFxpHandlePacket{p.ID, handle}) + return sshFxpHandlePacket{ID: p.id(), Handle: handle} } -func (p sshFxpReaddirPacket) respond(svr *Server) error { +func (p sshFxpReaddirPacket) respond(svr *Server) responsePacket { f, ok := svr.getHandle(p.Handle) if !ok { - return svr.sendError(p, syscall.EBADF) + return statusFromError(p, syscall.EBADF) } dirname := f.Name() dirents, err := f.Readdir(128) if err != nil { - return svr.sendError(p, err) + return statusFromError(p, err) } ret := sshFxpNamePacket{ID: p.ID} @@ -435,10 +430,10 @@ Attrs: []interface{}{dirent}, }) } - return svr.sendPacket(ret) + return ret } -func (p sshFxpSetstatPacket) respond(svr *Server) error { +func (p sshFxpSetstatPacket) respond(svr *Server) responsePacket { // additional unmarshalling is required for each possibility here b := p.Attrs.([]byte) var err error @@ -477,13 +472,13 @@ } } - return svr.sendError(p, err) + return statusFromError(p, err) } -func (p sshFxpFsetstatPacket) respond(svr *Server) error { +func (p sshFxpFsetstatPacket) respond(svr *Server) responsePacket { f, ok := svr.getHandle(p.Handle) if !ok { - return svr.sendError(p, syscall.EBADF) + return statusFromError(p, syscall.EBADF) } // additional unmarshalling is required for each possibility here @@ -524,7 +519,7 @@ } } - return svr.sendError(p, err) + return statusFromError(p, err) } // translateErrno translates a syscall error number to a SFTP error code. diff -Nru golang-github-pkg-sftp-1.7.0/server_statvfs_impl.go golang-github-pkg-sftp-1.8.3/server_statvfs_impl.go --- golang-github-pkg-sftp-1.7.0/server_statvfs_impl.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/server_statvfs_impl.go 2018-09-17 22:22:55.000000000 +0000 @@ -9,17 +9,17 @@ "syscall" ) -func (p sshFxpExtendedPacketStatVFS) respond(svr *Server) error { +func (p sshFxpExtendedPacketStatVFS) respond(svr *Server) responsePacket { stat := &syscall.Statfs_t{} if err := syscall.Statfs(p.Path, stat); err != nil { - return svr.sendPacket(statusFromError(p, err)) + return statusFromError(p, err) } retPkt, err := statvfsFromStatfst(stat) if err != nil { - return svr.sendPacket(statusFromError(p, err)) + return statusFromError(p, err) } retPkt.ID = p.ID - return svr.sendPacket(retPkt) + return retPkt } diff -Nru golang-github-pkg-sftp-1.7.0/server_statvfs_stubs.go golang-github-pkg-sftp-1.8.3/server_statvfs_stubs.go --- golang-github-pkg-sftp-1.7.0/server_statvfs_stubs.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/server_statvfs_stubs.go 2018-09-17 22:22:55.000000000 +0000 @@ -6,6 +6,6 @@ "syscall" ) -func (p sshFxpExtendedPacketStatVFS) respond(svr *Server) error { - return syscall.ENOTSUP +func (p sshFxpExtendedPacketStatVFS) respond(svr *Server) responsePacket { + return statusFromError(p, syscall.ENOTSUP) } diff -Nru golang-github-pkg-sftp-1.7.0/server_test.go golang-github-pkg-sftp-1.8.3/server_test.go --- golang-github-pkg-sftp-1.7.0/server_test.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/server_test.go 2018-09-17 22:22:55.000000000 +0000 @@ -3,6 +3,7 @@ import ( "io" "os" + "path" "regexp" "sync" "syscall" @@ -278,3 +279,51 @@ assert.Equal(t, tc.pkt, statusFromError(tc.pkt, tc.err)) } } + +// This was written to test a race b/w open immediately followed by a stat. +// Previous to this the Open would trigger the use of a worker pool, then the +// stat packet would come in an hit the pool and return faster than the open +// (returning a file-not-found error). +// The below by itself wouldn't trigger the race however, I needed to add a +// small sleep in the openpacket code to trigger the issue. I wanted to add a +// way to inject that in the code but right now there is no good place for it. +// I'm thinking after I convert the server into a request-server backend I +// might be able to do something with the runWorker method passed into the +// packet manager. But with the 2 implementations fo the server it just doesn't +// fit well right now. +func TestOpenStatRace(t *testing.T) { + client, server := clientServerPair(t) + defer client.Close() + defer server.Close() + + // openpacket finishes to fast to trigger race in tests + // need to add a small sleep on server to openpackets somehow + tmppath := path.Join(os.TempDir(), "stat_race") + pflags := flags(os.O_RDWR | os.O_CREATE | os.O_TRUNC) + ch := make(chan result, 3) + id1 := client.nextID() + client.dispatchRequest(ch, sshFxpOpenPacket{ + ID: id1, + Path: tmppath, + Pflags: pflags, + }) + id2 := client.nextID() + client.dispatchRequest(ch, sshFxpLstatPacket{ + ID: id2, + Path: tmppath, + }) + testreply := func(id uint32, ch chan result) { + r := <-ch + switch r.typ { + case ssh_FXP_ATTRS, ssh_FXP_HANDLE: // ignore + case ssh_FXP_STATUS: + err := normaliseError(unmarshalStatus(id, r.data)) + assert.NoError(t, err, "race hit, stat before open") + default: + assert.Fail(t, "Unexpected type") + } + } + testreply(id1, ch) + testreply(id2, ch) + os.Remove(tmppath) +} diff -Nru golang-github-pkg-sftp-1.7.0/server_unix.go golang-github-pkg-sftp-1.8.3/server_unix.go --- golang-github-pkg-sftp-1.7.0/server_unix.go 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/server_unix.go 2018-09-17 22:22:55.000000000 +0000 @@ -1,4 +1,4 @@ -// +build darwin dragonfly freebsd !android,linux netbsd openbsd solaris +// +build darwin dragonfly freebsd !android,linux netbsd openbsd solaris aix // +build cgo package sftp @@ -6,7 +6,6 @@ import ( "fmt" "os" - "os/user" "path" "syscall" "time" @@ -20,14 +19,12 @@ typeword := runLsTypeWord(dirent) numLinks := statt.Nlink - username := fmt.Sprintf("%d", statt.Uid) - if usr, err := user.LookupId(username); err == nil { - username = usr.Username - } - groupname := fmt.Sprintf("%d", statt.Gid) - if grp, err := user.LookupGroupId(groupname); err == nil { - groupname = grp.Name - } + uid := statt.Uid + gid := statt.Gid + username := fmt.Sprintf("%d", uid) + groupname := fmt.Sprintf("%d", gid) + // TODO FIXME: uid -> username, gid -> groupname lookup for ls -l format output + mtime := dirent.ModTime() monthStr := mtime.Month().String()[0:3] day := mtime.Day() @@ -40,9 +37,7 @@ yearOrTime = fmt.Sprintf("%d", year) } - return fmt.Sprintf("%s %4d %-8s %-8s %8d %s %2d %5s %s", typeword, - numLinks, username, groupname, dirent.Size(), monthStr, day, - yearOrTime, dirent.Name()) + return fmt.Sprintf("%s %4d %-8s %-8s %8d %s %2d %5s %s", typeword, numLinks, username, groupname, dirent.Size(), monthStr, day, yearOrTime, dirent.Name()) } // ls -l style output for a file, which is in the 'long output' section of a readdir response packet diff -Nru golang-github-pkg-sftp-1.7.0/.travis.yml golang-github-pkg-sftp-1.8.3/.travis.yml --- golang-github-pkg-sftp-1.7.0/.travis.yml 2018-05-11 21:26:11.000000000 +0000 +++ golang-github-pkg-sftp-1.8.3/.travis.yml 2018-09-17 22:22:55.000000000 +0000 @@ -4,8 +4,8 @@ # current and previous stable releases, plus tip # remember to exclude previous and tip for macs below go: - - 1.9.x - 1.10.x + - 1.11.x - tip os: @@ -15,7 +15,7 @@ matrix: exclude: - os: osx - go: 1.9.x + go: 1.10.x - os: osx go: tip