Skip to content

Commit c71cbb1

Browse files
eikenbeikenb
committed
'open' directory on opendir packet
Just like with files, the call to the hander to create the Lister is done when the OPENDIR packet is received. This eliminates the need for the lock in the filelist() method and opens the path for eliminating more locks.
1 parent 0a96e0d commit c71cbb1

File tree

5 files changed

+75
-62
lines changed

5 files changed

+75
-62
lines changed

‎request-example.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"path/filepath"
1313
"sort"
1414
"sync"
15+
"syscall"
1516
"time"
1617
)
1718

@@ -136,8 +137,16 @@ func (fs *root) Filelist(r *Request) (ListerAt, error) {
136137
fs.filesLock.Lock()
137138
defer fs.filesLock.Unlock()
138139

140+
file, err := fs.fetch(r.Filepath)
141+
if err != nil {
142+
return nil, err
143+
}
144+
139145
switch r.Method {
140146
case "List":
147+
if !file.IsDir() {
148+
return nil, syscall.ENOTDIR
149+
}
141150
ordered_names := []string{}
142151
for fn, _ := range fs.files {
143152
if filepath.Dir(fn) == r.Filepath {
@@ -151,16 +160,8 @@ func (fs *root) Filelist(r *Request) (ListerAt, error) {
151160
}
152161
return listerat(list), nil
153162
case "Stat":
154-
file, err := fs.fetch(r.Filepath)
155-
if err != nil {
156-
return nil, err
157-
}
158163
return listerat([]os.FileInfo{file}), nil
159164
case "Readlink":
160-
file, err := fs.fetch(r.Filepath)
161-
if err != nil {
162-
return nil, err
163-
}
164165
if file.symlink != "" {
165166
file, err = fs.fetch(file.symlink)
166167
if err != nil {

‎request-server.go

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package sftp
33
import (
44
"context"
55
"io"
6-
"os"
76
"path"
87
"path/filepath"
98
"strconv"
@@ -168,33 +167,18 @@ func (rs *RequestServer) packetWorker(
168167
case *sshFxInitPacket:
169168
rpkt = sshFxVersionPacket{Version: sftpProtocolVersion}
170169
case *sshFxpClosePacket:
171-
//fmt.Println("close")
172170
handle := pkt.getHandle()
173171
rpkt = statusFromError(pkt, rs.closeRequest(handle))
174172
case *sshFxpRealpathPacket:
175173
rpkt = cleanPacketPath(pkt)
176174
case *sshFxpOpendirPacket:
177175
request := requestFromPacket(ctx, pkt)
178-
rpkt = request.call(rs.Handlers, pkt)
179-
if stat, ok := rpkt.(*sshFxpStatResponse); ok {
180-
if stat.info.IsDir() {
181-
handle := rs.nextRequest(request)
182-
rpkt = sshFxpHandlePacket{ID: pkt.id(), Handle: handle}
183-
} else {
184-
rpkt = statusFromError(pkt, &os.PathError{
185-
Path: request.Filepath, Err: syscall.ENOTDIR})
186-
}
187-
}
176+
rs.nextRequest(request)
177+
rpkt = request.opendir(rs.Handlers, pkt)
188178
case *sshFxpOpenPacket:
189-
//fmt.Println("openpkt")
190-
// request w/ reader/writer set
191-
// method should be set to read/write
192-
// return handle packet (or err)
193-
// save handle for future calls
194-
195179
request := requestFromPacket(ctx, pkt)
196180
rs.nextRequest(request)
197-
rpkt = request.call(rs.Handlers, pkt)
181+
rpkt = request.open(rs.Handlers, pkt)
198182
case hasHandle:
199183
handle := pkt.getHandle()
200184
request, ok := rs.getRequest(handle, requestMethod(pkt))

‎request-server_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,9 @@ func TestRequestReaddir(t *testing.T) {
351351
for i := 0; i < 100; i++ {
352352
fname := fmt.Sprintf("/foo_%02d", i)
353353
_, err := putTestFile(p.cli, fname, fname)
354-
assert.Nil(t, err)
354+
if !assert.NoError(t, err) {
355+
t.Fatal(err)
356+
}
355357
}
356358
_, err := p.cli.ReadDir("/foo_01")
357359
assert.Equal(t, &StatusError{Code: ssh_FX_FAILURE,

‎request.go

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,6 @@ func (r *Request) close() error {
154154
// called from worker to handle packet/request
155155
func (r *Request) call(handlers Handlers, pkt requestPacket) responsePacket {
156156
switch r.Method {
157-
case "Open":
158-
return fileopen(handlers, r, pkt)
159157
case "Get":
160158
return fileget(handlers.FileGet, r, pkt)
161159
case "Put":
@@ -172,26 +170,37 @@ func (r *Request) call(handlers Handlers, pkt requestPacket) responsePacket {
172170
}
173171
}
174172

175-
func fileopen(h Handlers, r *Request, pkt requestPacket) responsePacket {
176-
//fmt.Println("fileopen", r)
173+
// Additional initialization for Open packets
174+
func (r *Request) open(h Handlers, pkt requestPacket) responsePacket {
177175
flags := r.Pflags()
178176
var err error
179177
switch {
180178
case flags.Write, flags.Append, flags.Creat, flags.Trunc:
181-
r.state.Lock()
182-
defer r.state.Unlock()
179+
r.Method = "Put"
183180
r.state.writerAt, err = h.FilePut.Filewrite(r)
184181
case flags.Read:
185-
r.state.Lock()
186-
defer r.state.Unlock()
182+
r.Method = "Get"
187183
r.state.readerAt, err = h.FileGet.Fileread(r)
184+
default:
185+
return statusFromError(pkt, errors.New("bad file flags"))
188186
}
189-
// XXX needs to return error OR sshFxpHandlePacket
190187
if err != nil {
191188
return statusFromError(pkt, err)
192189
}
193-
194-
return sshFxpHandlePacket{ID: pkt.id(), Handle: r.handle}
190+
return &sshFxpHandlePacket{ID: pkt.id(), Handle: r.handle}
191+
}
192+
func (r *Request) opendir(h Handlers, pkt requestPacket) responsePacket {
193+
var err error
194+
r.Method = "List"
195+
r.state.listerAt, err = h.FileList.Filelist(r)
196+
if err != nil {
197+
switch err.(type) {
198+
case syscall.Errno:
199+
err = &os.PathError{Path: r.Filepath, Err: err}
200+
}
201+
return statusFromError(pkt, err)
202+
}
203+
return &sshFxpHandlePacket{ID: pkt.id(), Handle: r.handle}
195204
}
196205

197206
// wrap FileReader handler
@@ -266,11 +275,7 @@ func filelist(h FileLister, r *Request, pkt requestPacket) responsePacket {
266275
var err error
267276
lister := r.getLister()
268277
if lister == nil {
269-
lister, err = h.Filelist(r)
270-
if err != nil {
271-
return statusFromError(pkt, err)
272-
}
273-
r.setListerState(lister)
278+
return statusFromError(pkt, errors.New("unexpected dir packet"))
274279
}
275280

276281
offset := r.lsNext()
@@ -355,16 +360,10 @@ func filestat(h FileLister, r *Request, pkt requestPacket) responsePacket {
355360
// init attributes of request object from packet data
356361
func requestMethod(p requestPacket) (method string) {
357362
switch p.(type) {
358-
case *sshFxpReadPacket:
359-
method = "Get"
360-
case *sshFxpWritePacket:
361-
method = "Put"
362-
case *sshFxpReaddirPacket:
363-
method = "List"
364-
case *sshFxpOpenPacket:
365-
method = "Open"
366-
case *sshFxpOpendirPacket:
367-
method = "Stat"
363+
case *sshFxpReadPacket, *sshFxpWritePacket, *sshFxpOpenPacket:
364+
// set in open() above
365+
case *sshFxpOpendirPacket, *sshFxpReaddirPacket:
366+
// set in opendir() above
368367
case *sshFxpSetstatPacket, *sshFxpFsetstatPacket:
369368
method = "Setstat"
370369
case *sshFxpRenamePacket:

‎request_test.go

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,18 @@ var filecontents = []byte("file-data.")
6262

6363
// XXX need new for creating test requests that supports Open-ing
6464
func testRequest(method string) *Request {
65+
var flags uint32
66+
switch method {
67+
case "Get":
68+
flags = flags | ssh_FXF_READ
69+
case "Put":
70+
flags = flags | ssh_FXF_WRITE
71+
}
6572
request := &Request{
6673
Filepath: "./request_test.go",
6774
Method: method,
6875
Attrs: []byte("foo"),
76+
Flags: flags,
6977
Target: "foo",
7078
state: state{RWMutex: new(sync.RWMutex)},
7179
}
@@ -138,7 +146,8 @@ func (fakePacket) UnmarshalBinary(d []byte) error { return nil }
138146
func TestRequestGet(t *testing.T) {
139147
handlers := newTestHandlers()
140148
request := testRequest("Get")
141-
request.state.readerAt, _ = handlers.FileGet.Fileread(request)
149+
pkt := fakePacket{myid: 1}
150+
request.open(handlers, pkt)
142151
// req.length is 5, so we test reads in 5 byte chunks
143152
for i, txt := range []string{"file-", "data."} {
144153
pkt := &sshFxpReadPacket{ID: uint32(i), Handle: "a",
@@ -198,28 +207,46 @@ func TestRequestInfoStat(t *testing.T) {
198207
assert.Equal(t, spkt.info.Name(), "request_test.go")
199208
}
200209

201-
func TestRequestInfoList(t *testing.T) { testInfoMethod(t, "List") }
202-
func TestRequestInfoReadlink(t *testing.T) { testInfoMethod(t, "Readlink") }
203-
func testInfoMethod(t *testing.T, method string) {
210+
func TestRequestInfoList(t *testing.T) {
211+
handlers := newTestHandlers()
212+
request := testRequest("List")
213+
request.handle = "1"
214+
pkt := fakePacket{myid: 1}
215+
rpkt := request.opendir(handlers, pkt)
216+
hpkt, ok := rpkt.(*sshFxpHandlePacket)
217+
if assert.True(t, ok) {
218+
assert.Equal(t, hpkt.Handle, "1")
219+
}
220+
pkt = fakePacket{myid: 2}
221+
request.call(handlers, pkt)
222+
}
223+
func TestRequestInfoReadlink(t *testing.T) {
204224
handlers := newTestHandlers()
205-
request := testRequest(method)
225+
request := testRequest("Readlink")
206226
pkt := fakePacket{myid: 1}
207227
rpkt := request.call(handlers, pkt)
208228
npkt, ok := rpkt.(*sshFxpNamePacket)
209-
assert.True(t, ok)
210-
assert.IsType(t, sshFxpNameAttr{}, npkt.NameAttrs[0])
211-
assert.Equal(t, npkt.NameAttrs[0].Name, "request_test.go")
229+
if assert.True(t, ok) {
230+
assert.IsType(t, sshFxpNameAttr{}, npkt.NameAttrs[0])
231+
assert.Equal(t, npkt.NameAttrs[0].Name, "request_test.go")
232+
}
212233
}
213234

214235
func TestOpendirHandleReuse(t *testing.T) {
215236
handlers := newTestHandlers()
216237
request := testRequest("Stat")
238+
request.handle = "1"
217239
pkt := fakePacket{myid: 1}
218240
rpkt := request.call(handlers, pkt)
219241
assert.IsType(t, &sshFxpStatResponse{}, rpkt)
220242

221243
request.Method = "List"
222244
pkt = fakePacket{myid: 2}
245+
rpkt = request.opendir(handlers, pkt)
246+
if assert.IsType(t, &sshFxpHandlePacket{}, rpkt) {
247+
hpkt := rpkt.(*sshFxpHandlePacket)
248+
assert.Equal(t, hpkt.Handle, "1")
249+
}
223250
rpkt = request.call(handlers, pkt)
224251
assert.IsType(t, &sshFxpNamePacket{}, rpkt)
225252
}

0 commit comments

Comments
 (0)