Skip to content

Commit 1afc1d9

Browse files
eikenbeikenb
committed
refactor server response to allow for extending
Instead of sendPacket/sendError being sprayed all over the place, this change has all those places instead return a responsePacket (eventually) back to the main handling function which then calls sendPacket in one place. Behaviour of the code should remain exactly the same. This makes it much easier to work with the response packets (eg. for the packet ordering issue I'm working on).
1 parent b50b1f9 commit 1afc1d9

File tree

4 files changed

+95
-94
lines changed

4 files changed

+95
-94
lines changed

‎packet.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -882,9 +882,9 @@ func (p sshFxpExtendedPacket) readonly() bool {
882882
return p.SpecificPacket.readonly()
883883
}
884884

885-
func (p sshFxpExtendedPacket) respond(svr *Server) error {
885+
func (p sshFxpExtendedPacket) respond(svr *Server) responsePacket {
886886
if p.SpecificPacket == nil {
887-
return nil
887+
return statusFromError(p, nil)
888888
}
889889
return p.SpecificPacket.respond(svr)
890890
}
@@ -954,7 +954,7 @@ func (p *sshFxpExtendedPacketPosixRename) UnmarshalBinary(b []byte) error {
954954
return nil
955955
}
956956

957-
func (p sshFxpExtendedPacketPosixRename) respond(s *Server) error {
957+
func (p sshFxpExtendedPacketPosixRename) respond(s *Server) responsePacket {
958958
err := os.Rename(p.Oldpath, p.Newpath)
959-
return s.sendError(&p, err)
959+
return statusFromError(p, err)
960960
}

‎server.go

+85-84
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func (svr *Server) getHandle(handle string) (*os.File, bool) {
6666
type serverRespondablePacket interface {
6767
encoding.BinaryUnmarshaler
6868
id() uint32
69-
respond(svr *Server) error
69+
respond(svr *Server) responsePacket
7070
}
7171

7272
// NewServer creates a new Server instance around the provided streams, serving
@@ -140,7 +140,7 @@ func (svr *Server) sftpServerWorker(pktChan chan requestPacket) error {
140140
// If server is operating read-only and a write operation is requested,
141141
// return permission denied
142142
if !readonly && svr.readOnly {
143-
svr.sendError(pkt, syscall.EPERM)
143+
svr.sendPacket(statusFromError(pkt, syscall.EPERM))
144144
continue
145145
}
146146

@@ -152,133 +152,138 @@ func (svr *Server) sftpServerWorker(pktChan chan requestPacket) error {
152152
}
153153

154154
func handlePacket(s *Server, p requestPacket) error {
155+
var rpkt responsePacket
155156
switch p := p.(type) {
156157
case *sshFxInitPacket:
157-
return s.sendPacket(sshFxVersionPacket{Version: sftpProtocolVersion})
158+
rpkt = sshFxVersionPacket{Version: sftpProtocolVersion}
158159
case *sshFxpStatPacket:
159160
// stat the requested file
160161
info, err := os.Stat(p.Path)
161-
if err != nil {
162-
return s.sendError(p, err)
163-
}
164-
return s.sendPacket(sshFxpStatResponse{
162+
rpkt = sshFxpStatResponse{
165163
ID: p.ID,
166164
info: info,
167-
})
165+
}
166+
if err != nil {
167+
rpkt = statusFromError(p, err)
168+
}
168169
case *sshFxpLstatPacket:
169170
// stat the requested file
170171
info, err := os.Lstat(p.Path)
171-
if err != nil {
172-
return s.sendError(p, err)
173-
}
174-
return s.sendPacket(sshFxpStatResponse{
172+
rpkt = sshFxpStatResponse{
175173
ID: p.ID,
176174
info: info,
177-
})
175+
}
176+
if err != nil {
177+
rpkt = statusFromError(p, err)
178+
}
178179
case *sshFxpFstatPacket:
180+
fmt.Println("fstat")
179181
f, ok := s.getHandle(p.Handle)
180-
if !ok {
181-
return s.sendError(p, syscall.EBADF)
182+
var err error = syscall.EBADF
183+
var info os.FileInfo
184+
if ok {
185+
info, err = f.Stat()
186+
rpkt = sshFxpStatResponse{
187+
ID: p.ID,
188+
info: info,
189+
}
182190
}
183-
184-
info, err := f.Stat()
185191
if err != nil {
186-
return s.sendError(p, err)
192+
rpkt = statusFromError(p, err)
187193
}
188-
189-
return s.sendPacket(sshFxpStatResponse{
190-
ID: p.ID,
191-
info: info,
192-
})
193194
case *sshFxpMkdirPacket:
194195
// TODO FIXME: ignore flags field
195196
err := os.Mkdir(p.Path, 0755)
196-
return s.sendError(p, err)
197+
rpkt = statusFromError(p, err)
197198
case *sshFxpRmdirPacket:
198199
err := os.Remove(p.Path)
199-
return s.sendError(p, err)
200+
rpkt = statusFromError(p, err)
200201
case *sshFxpRemovePacket:
201202
err := os.Remove(p.Filename)
202-
return s.sendError(p, err)
203+
rpkt = statusFromError(p, err)
203204
case *sshFxpRenamePacket:
204205
err := os.Rename(p.Oldpath, p.Newpath)
205-
return s.sendError(p, err)
206+
rpkt = statusFromError(p, err)
206207
case *sshFxpSymlinkPacket:
207208
err := os.Symlink(p.Targetpath, p.Linkpath)
208-
return s.sendError(p, err)
209+
rpkt = statusFromError(p, err)
209210
case *sshFxpClosePacket:
210-
return s.sendError(p, s.closeHandle(p.Handle))
211+
rpkt = statusFromError(p, s.closeHandle(p.Handle))
211212
case *sshFxpReadlinkPacket:
212213
f, err := os.Readlink(p.Path)
213-
if err != nil {
214-
return s.sendError(p, err)
215-
}
216-
217-
return s.sendPacket(sshFxpNamePacket{
214+
rpkt = sshFxpNamePacket{
218215
ID: p.ID,
219216
NameAttrs: []sshFxpNameAttr{{
220217
Name: f,
221218
LongName: f,
222219
Attrs: emptyFileStat,
223220
}},
224-
})
225-
226-
case *sshFxpRealpathPacket:
227-
f, err := filepath.Abs(p.Path)
221+
}
228222
if err != nil {
229-
return s.sendError(p, err)
223+
rpkt = statusFromError(p, err)
230224
}
225+
case *sshFxpRealpathPacket:
226+
f, err := filepath.Abs(p.Path)
231227
f = cleanPath(f)
232-
return s.sendPacket(sshFxpNamePacket{
228+
rpkt = sshFxpNamePacket{
233229
ID: p.ID,
234230
NameAttrs: []sshFxpNameAttr{{
235231
Name: f,
236232
LongName: f,
237233
Attrs: emptyFileStat,
238234
}},
239-
})
235+
}
236+
if err != nil {
237+
rpkt = statusFromError(p, err)
238+
}
240239
case *sshFxpOpendirPacket:
241240
if stat, err := os.Stat(p.Path); err != nil {
242-
return s.sendError(p, err)
241+
rpkt = statusFromError(p, err)
243242
} else if !stat.IsDir() {
244-
return s.sendError(p, &os.PathError{
243+
rpkt = statusFromError(p, &os.PathError{
245244
Path: p.Path, Err: syscall.ENOTDIR})
245+
} else {
246+
rpkt = sshFxpOpenPacket{
247+
ID: p.ID,
248+
Path: p.Path,
249+
Pflags: ssh_FXF_READ,
250+
}.respond(s)
246251
}
247-
return sshFxpOpenPacket{
248-
ID: p.ID,
249-
Path: p.Path,
250-
Pflags: ssh_FXF_READ,
251-
}.respond(s)
252252
case *sshFxpReadPacket:
253+
var err error = syscall.EBADF
253254
f, ok := s.getHandle(p.Handle)
254-
if !ok {
255-
return s.sendError(p, syscall.EBADF)
255+
if ok {
256+
err = nil
257+
data := make([]byte, clamp(p.Len, s.maxTxPacket))
258+
n, _err := f.ReadAt(data, int64(p.Offset))
259+
if _err != nil && (_err != io.EOF || n == 0) {
260+
err = _err
261+
}
262+
rpkt = sshFxpDataPacket{
263+
ID: p.ID,
264+
Length: uint32(n),
265+
Data: data[:n],
266+
}
256267
}
257-
258-
data := make([]byte, clamp(p.Len, s.maxTxPacket))
259-
n, err := f.ReadAt(data, int64(p.Offset))
260-
if err != nil && (err != io.EOF || n == 0) {
261-
return s.sendError(p, err)
268+
if err != nil {
269+
rpkt = statusFromError(p, err)
262270
}
263-
return s.sendPacket(sshFxpDataPacket{
264-
ID: p.ID,
265-
Length: uint32(n),
266-
Data: data[:n],
267-
})
271+
268272
case *sshFxpWritePacket:
269273
f, ok := s.getHandle(p.Handle)
270-
if !ok {
271-
return s.sendError(p, syscall.EBADF)
274+
var err error = syscall.EBADF
275+
if ok {
276+
_, err = f.WriteAt(p.Data, int64(p.Offset))
272277
}
273-
274-
_, err := f.WriteAt(p.Data, int64(p.Offset))
275-
return s.sendError(p, err)
278+
rpkt = statusFromError(p, err)
276279
case serverRespondablePacket:
277-
err := p.respond(s)
278-
return errors.Wrap(err, "pkt.respond failed")
280+
rpkt = p.respond(s)
279281
default:
280282
return errors.Errorf("unexpected packet type %T", p)
281283
}
284+
285+
s.sendPacket(rpkt)
286+
return nil
282287
}
283288

284289
// Serve serves SFTP connections until the streams stop or the SFTP subsystem
@@ -342,10 +347,6 @@ func (svr *Server) sendPacket(pkt responsePacket) error {
342347
return nil
343348
}
344349

345-
func (svr *Server) sendError(p requestPacket, err error) error {
346-
return svr.sendPacket(statusFromError(p, err))
347-
}
348-
349350
type ider interface {
350351
id() uint32
351352
}
@@ -380,7 +381,7 @@ func (p sshFxpOpenPacket) hasPflags(flags ...uint32) bool {
380381
return true
381382
}
382383

383-
func (p sshFxpOpenPacket) respond(svr *Server) error {
384+
func (p sshFxpOpenPacket) respond(svr *Server) responsePacket {
384385
var osFlags int
385386
if p.hasPflags(ssh_FXF_READ, ssh_FXF_WRITE) {
386387
osFlags |= os.O_RDWR
@@ -390,7 +391,7 @@ func (p sshFxpOpenPacket) respond(svr *Server) error {
390391
osFlags |= os.O_RDONLY
391392
} else {
392393
// how are they opening?
393-
return svr.sendError(&p, syscall.EINVAL)
394+
return statusFromError(p, syscall.EINVAL)
394395
}
395396

396397
if p.hasPflags(ssh_FXF_APPEND) {
@@ -408,23 +409,23 @@ func (p sshFxpOpenPacket) respond(svr *Server) error {
408409

409410
f, err := os.OpenFile(p.Path, osFlags, 0644)
410411
if err != nil {
411-
return svr.sendError(&p, err)
412+
return statusFromError(p, err)
412413
}
413414

414415
handle := svr.nextHandle(f)
415-
return svr.sendPacket(sshFxpHandlePacket{ID: p.id(), Handle: handle})
416+
return sshFxpHandlePacket{ID: p.id(), Handle: handle}
416417
}
417418

418-
func (p sshFxpReaddirPacket) respond(svr *Server) error {
419+
func (p sshFxpReaddirPacket) respond(svr *Server) responsePacket {
419420
f, ok := svr.getHandle(p.Handle)
420421
if !ok {
421-
return svr.sendError(&p, syscall.EBADF)
422+
return statusFromError(p, syscall.EBADF)
422423
}
423424

424425
dirname := f.Name()
425426
dirents, err := f.Readdir(128)
426427
if err != nil {
427-
return svr.sendError(&p, err)
428+
return statusFromError(p, err)
428429
}
429430

430431
ret := sshFxpNamePacket{ID: p.ID}
@@ -435,10 +436,10 @@ func (p sshFxpReaddirPacket) respond(svr *Server) error {
435436
Attrs: []interface{}{dirent},
436437
})
437438
}
438-
return svr.sendPacket(ret)
439+
return ret
439440
}
440441

441-
func (p sshFxpSetstatPacket) respond(svr *Server) error {
442+
func (p sshFxpSetstatPacket) respond(svr *Server) responsePacket {
442443
// additional unmarshalling is required for each possibility here
443444
b := p.Attrs.([]byte)
444445
var err error
@@ -477,13 +478,13 @@ func (p sshFxpSetstatPacket) respond(svr *Server) error {
477478
}
478479
}
479480

480-
return svr.sendError(&p, err)
481+
return statusFromError(p, err)
481482
}
482483

483-
func (p sshFxpFsetstatPacket) respond(svr *Server) error {
484+
func (p sshFxpFsetstatPacket) respond(svr *Server) responsePacket {
484485
f, ok := svr.getHandle(p.Handle)
485486
if !ok {
486-
return svr.sendError(&p, syscall.EBADF)
487+
return statusFromError(p, syscall.EBADF)
487488
}
488489

489490
// additional unmarshalling is required for each possibility here
@@ -524,7 +525,7 @@ func (p sshFxpFsetstatPacket) respond(svr *Server) error {
524525
}
525526
}
526527

527-
return svr.sendError(&p, err)
528+
return statusFromError(p, err)
528529
}
529530

530531
// translateErrno translates a syscall error number to a SFTP error code.

‎server_statvfs_impl.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,17 @@ import (
99
"syscall"
1010
)
1111

12-
func (p sshFxpExtendedPacketStatVFS) respond(svr *Server) error {
12+
func (p sshFxpExtendedPacketStatVFS) respond(svr *Server) responsePacket {
1313
stat := &syscall.Statfs_t{}
1414
if err := syscall.Statfs(p.Path, stat); err != nil {
15-
return svr.sendPacket(statusFromError(p, err))
15+
return statusFromError(p, err)
1616
}
1717

1818
retPkt, err := statvfsFromStatfst(stat)
1919
if err != nil {
20-
return svr.sendPacket(statusFromError(p, err))
20+
return statusFromError(p, err)
2121
}
2222
retPkt.ID = p.ID
2323

24-
return svr.sendPacket(retPkt)
24+
return retPkt
2525
}

‎server_statvfs_stubs.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ import (
66
"syscall"
77
)
88

9-
func (p sshFxpExtendedPacketStatVFS) respond(svr *Server) error {
10-
return syscall.ENOTSUP
9+
func (p sshFxpExtendedPacketStatVFS) respond(svr *Server) responsePacket {
10+
return statusFromError(p, syscall.ENOTSUP)
1111
}

0 commit comments

Comments
 (0)