Skip to content

Commit c1f47ba

Browse files
peterverraedtpeterverraedt
committed
Add WithMaxTxPacket server option
Add the WithMaxTxPacket and WithRSMaxTxPacket server options to increase the maximum tx packet size to a value above 32K. This allows to send bigger chunks of data to the client as response to a read request. As the client specifies the wanted length, it should be safe to increase the server maximum value. This in particular allows the implemented Client with the MaxPacketUnchecked option to retrieve data in larger chunks. Signed-off-by: Peter Verraedt <[email protected]>
1 parent 06342e8 commit c1f47ba

File tree

5 files changed

+78
-40
lines changed

5 files changed

+78
-40
lines changed

‎packet.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -823,7 +823,7 @@ func (p *sshFxpReadPacket) UnmarshalBinary(b []byte) error {
823823
// So, we need: uint32(length) + byte(type) + uint32(id) + uint32(data_length)
824824
const dataHeaderLen = 4 + 1 + 4 + 4
825825

826-
func (p *sshFxpReadPacket) getDataSlice(alloc *allocator, orderID uint32) []byte {
826+
func (p *sshFxpReadPacket) getDataSlice(alloc *allocator, orderID uint32, maxTxPacket uint32) []byte {
827827
dataLen := p.Len
828828
if dataLen > maxTxPacket {
829829
dataLen = maxTxPacket

‎request-server.go‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"sync"
1111
)
1212

13-
var maxTxPacket uint32 = 1 << 15
13+
const defaultMaxTxPacket uint32 = 1 << 15
1414

1515
// Handlers contains the 4 SFTP server request handlers.
1616
type Handlers struct {
@@ -28,6 +28,7 @@ type RequestServer struct {
2828
pktMgr *packetManager
2929

3030
startDirectory string
31+
maxTxPacket uint32
3132

3233
mu sync.RWMutex
3334
handleCount int
@@ -57,6 +58,22 @@ func WithStartDirectory(startDirectory string) RequestServerOption {
5758
}
5859
}
5960

61+
// WithRSMaxTxPacket sets the maximum size of the payload returned to the client,
62+
// measured in bytes. The default value is 32768 bytes, and this option
63+
// can only be used to increase it. Setting this option to a larger value
64+
// should be safe, because the client decides the size of the requested payload.
65+
//
66+
// The default maximum packet size is 32768 bytes.
67+
func WithRSMaxTxPacket(size uint32) RequestServerOption {
68+
return func(rs *RequestServer) {
69+
if size < defaultMaxTxPacket {
70+
return
71+
}
72+
73+
rs.maxTxPacket = size
74+
}
75+
}
76+
6077
// NewRequestServer creates/allocates/returns new RequestServer.
6178
// Normally there will be one server per user-session.
6279
func NewRequestServer(rwc io.ReadWriteCloser, h Handlers, options ...RequestServerOption) *RequestServer {
@@ -73,6 +90,7 @@ func NewRequestServer(rwc io.ReadWriteCloser, h Handlers, options ...RequestServ
7390
pktMgr: newPktMgr(svrConn),
7491

7592
startDirectory: "/",
93+
maxTxPacket: defaultMaxTxPacket,
7694

7795
openRequests: make(map[string]*Request),
7896
}
@@ -260,7 +278,7 @@ func (rs *RequestServer) packetWorker(ctx context.Context, pktChan chan orderedR
260278
Method: "Stat",
261279
Filepath: cleanPathWithBase(rs.startDirectory, request.Filepath),
262280
}
263-
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID)
281+
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID, rs.maxTxPacket)
264282
}
265283
case *sshFxpFsetstatPacket:
266284
handle := pkt.getHandle()
@@ -272,32 +290,32 @@ func (rs *RequestServer) packetWorker(ctx context.Context, pktChan chan orderedR
272290
Method: "Setstat",
273291
Filepath: cleanPathWithBase(rs.startDirectory, request.Filepath),
274292
}
275-
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID)
293+
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID, rs.maxTxPacket)
276294
}
277295
case *sshFxpExtendedPacketPosixRename:
278296
request := &Request{
279297
Method: "PosixRename",
280298
Filepath: cleanPathWithBase(rs.startDirectory, pkt.Oldpath),
281299
Target: cleanPathWithBase(rs.startDirectory, pkt.Newpath),
282300
}
283-
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID)
301+
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID, rs.maxTxPacket)
284302
case *sshFxpExtendedPacketStatVFS:
285303
request := &Request{
286304
Method: "StatVFS",
287305
Filepath: cleanPathWithBase(rs.startDirectory, pkt.Path),
288306
}
289-
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID)
307+
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID, rs.maxTxPacket)
290308
case hasHandle:
291309
handle := pkt.getHandle()
292310
request, ok := rs.getRequest(handle)
293311
if !ok {
294312
rpkt = statusFromError(pkt.id(), EBADF)
295313
} else {
296-
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID)
314+
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID, rs.maxTxPacket)
297315
}
298316
case hasPath:
299317
request := requestFromPacket(ctx, pkt, rs.startDirectory)
300-
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID)
318+
rpkt = request.call(rs.Handlers, pkt, rs.pktMgr.alloc, orderID, rs.maxTxPacket)
301319
request.close()
302320
default:
303321
rpkt = statusFromError(pkt.id(), ErrSSHFxOpUnsupported)

‎request.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -300,14 +300,14 @@ func (r *Request) transferError(err error) {
300300
}
301301

302302
// called from worker to handle packet/request
303-
func (r *Request) call(handlers Handlers, pkt requestPacket, alloc *allocator, orderID uint32) responsePacket {
303+
func (r *Request) call(handlers Handlers, pkt requestPacket, alloc *allocator, orderID uint32, maxTxPacket uint32) responsePacket {
304304
switch r.Method {
305305
case "Get":
306-
return fileget(handlers.FileGet, r, pkt, alloc, orderID)
306+
return fileget(handlers.FileGet, r, pkt, alloc, orderID, maxTxPacket)
307307
case "Put":
308-
return fileput(handlers.FilePut, r, pkt, alloc, orderID)
308+
return fileput(handlers.FilePut, r, pkt, alloc, orderID, maxTxPacket)
309309
case "Open":
310-
return fileputget(handlers.FilePut, r, pkt, alloc, orderID)
310+
return fileputget(handlers.FilePut, r, pkt, alloc, orderID, maxTxPacket)
311311
case "Setstat", "Rename", "Rmdir", "Mkdir", "Link", "Symlink", "Remove", "PosixRename", "StatVFS":
312312
return filecmd(handlers.FileCmd, r, pkt)
313313
case "List":
@@ -392,13 +392,13 @@ func (r *Request) opendir(h Handlers, pkt requestPacket) responsePacket {
392392
}
393393

394394
// wrap FileReader handler
395-
func fileget(h FileReader, r *Request, pkt requestPacket, alloc *allocator, orderID uint32) responsePacket {
395+
func fileget(h FileReader, r *Request, pkt requestPacket, alloc *allocator, orderID uint32, maxTxPacket uint32) responsePacket {
396396
rd := r.getReaderAt()
397397
if rd == nil {
398398
return statusFromError(pkt.id(), errors.New("unexpected read packet"))
399399
}
400400

401-
data, offset, _ := packetData(pkt, alloc, orderID)
401+
data, offset, _ := packetData(pkt, alloc, orderID, maxTxPacket)
402402

403403
n, err := rd.ReadAt(data, offset)
404404
// only return EOF error if no data left to read
@@ -414,28 +414,28 @@ func fileget(h FileReader, r *Request, pkt requestPacket, alloc *allocator, orde
414414
}
415415

416416
// wrap FileWriter handler
417-
func fileput(h FileWriter, r *Request, pkt requestPacket, alloc *allocator, orderID uint32) responsePacket {
417+
func fileput(h FileWriter, r *Request, pkt requestPacket, alloc *allocator, orderID uint32, maxTxPacket uint32) responsePacket {
418418
wr := r.getWriterAt()
419419
if wr == nil {
420420
return statusFromError(pkt.id(), errors.New("unexpected write packet"))
421421
}
422422

423-
data, offset, _ := packetData(pkt, alloc, orderID)
423+
data, offset, _ := packetData(pkt, alloc, orderID, maxTxPacket)
424424

425425
_, err := wr.WriteAt(data, offset)
426426
return statusFromError(pkt.id(), err)
427427
}
428428

429429
// wrap OpenFileWriter handler
430-
func fileputget(h FileWriter, r *Request, pkt requestPacket, alloc *allocator, orderID uint32) responsePacket {
430+
func fileputget(h FileWriter, r *Request, pkt requestPacket, alloc *allocator, orderID uint32, maxTxPacket uint32) responsePacket {
431431
rw := r.getWriterAtReaderAt()
432432
if rw == nil {
433433
return statusFromError(pkt.id(), errors.New("unexpected write and read packet"))
434434
}
435435

436436
switch p := pkt.(type) {
437437
case *sshFxpReadPacket:
438-
data, offset := p.getDataSlice(alloc, orderID), int64(p.Offset)
438+
data, offset := p.getDataSlice(alloc, orderID, maxTxPacket), int64(p.Offset)
439439

440440
n, err := rw.ReadAt(data, offset)
441441
// only return EOF error if no data left to read
@@ -461,10 +461,10 @@ func fileputget(h FileWriter, r *Request, pkt requestPacket, alloc *allocator, o
461461
}
462462

463463
// file data for additional read/write packets
464-
func packetData(p requestPacket, alloc *allocator, orderID uint32) (data []byte, offset int64, length uint32) {
464+
func packetData(p requestPacket, alloc *allocator, orderID uint32, maxTxPacket uint32) (data []byte, offset int64, length uint32) {
465465
switch p := p.(type) {
466466
case *sshFxpReadPacket:
467-
return p.getDataSlice(alloc, orderID), int64(p.Offset), p.Len
467+
return p.getDataSlice(alloc, orderID, maxTxPacket), int64(p.Offset), p.Len
468468
case *sshFxpWritePacket:
469469
return p.Data, int64(p.Offset), p.Length
470470
}

‎request_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func TestRequestGet(t *testing.T) {
149149
for i, txt := range []string{"file-", "data."} {
150150
pkt := &sshFxpReadPacket{ID: uint32(i), Handle: "a",
151151
Offset: uint64(i * 5), Len: 5}
152-
rpkt := request.call(handlers, pkt, nil, 0)
152+
rpkt := request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
153153
dpkt := rpkt.(*sshFxpDataPacket)
154154
assert.Equal(t, dpkt.id(), uint32(i))
155155
assert.Equal(t, string(dpkt.Data), txt)
@@ -162,7 +162,7 @@ func TestRequestCustomError(t *testing.T) {
162162
pkt := fakePacket{myid: 1}
163163
cmdErr := errors.New("stat not supported")
164164
handlers.returnError(cmdErr)
165-
rpkt := request.call(handlers, pkt, nil, 0)
165+
rpkt := request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
166166
assert.Equal(t, rpkt, statusFromError(pkt.myid, cmdErr))
167167
}
168168

@@ -173,11 +173,11 @@ func TestRequestPut(t *testing.T) {
173173
request.state.writerAt, _ = handlers.FilePut.Filewrite(request)
174174
pkt := &sshFxpWritePacket{ID: 0, Handle: "a", Offset: 0, Length: 5,
175175
Data: []byte("file-")}
176-
rpkt := request.call(handlers, pkt, nil, 0)
176+
rpkt := request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
177177
checkOkStatus(t, rpkt)
178178
pkt = &sshFxpWritePacket{ID: 1, Handle: "a", Offset: 5, Length: 5,
179179
Data: []byte("data.")}
180-
rpkt = request.call(handlers, pkt, nil, 0)
180+
rpkt = request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
181181
checkOkStatus(t, rpkt)
182182
assert.Equal(t, "file-data.", handlers.getOutString())
183183
}
@@ -186,19 +186,19 @@ func TestRequestCmdr(t *testing.T) {
186186
handlers := newTestHandlers()
187187
request := testRequest("Mkdir")
188188
pkt := fakePacket{myid: 1}
189-
rpkt := request.call(handlers, pkt, nil, 0)
189+
rpkt := request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
190190
checkOkStatus(t, rpkt)
191191

192192
handlers.returnError(errTest)
193-
rpkt = request.call(handlers, pkt, nil, 0)
193+
rpkt = request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
194194
assert.Equal(t, rpkt, statusFromError(pkt.myid, errTest))
195195
}
196196

197197
func TestRequestInfoStat(t *testing.T) {
198198
handlers := newTestHandlers()
199199
request := testRequest("Stat")
200200
pkt := fakePacket{myid: 1}
201-
rpkt := request.call(handlers, pkt, nil, 0)
201+
rpkt := request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
202202
spkt, ok := rpkt.(*sshFxpStatResponse)
203203
assert.True(t, ok)
204204
assert.Equal(t, spkt.info.Name(), "request_test.go")
@@ -215,13 +215,13 @@ func TestRequestInfoList(t *testing.T) {
215215
assert.Equal(t, hpkt.Handle, "1")
216216
}
217217
pkt = fakePacket{myid: 2}
218-
request.call(handlers, pkt, nil, 0)
218+
request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
219219
}
220220
func TestRequestInfoReadlink(t *testing.T) {
221221
handlers := newTestHandlers()
222222
request := testRequest("Readlink")
223223
pkt := fakePacket{myid: 1}
224-
rpkt := request.call(handlers, pkt, nil, 0)
224+
rpkt := request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
225225
npkt, ok := rpkt.(*sshFxpNamePacket)
226226
if assert.True(t, ok) {
227227
assert.IsType(t, &sshFxpNameAttr{}, npkt.NameAttrs[0])
@@ -234,7 +234,7 @@ func TestOpendirHandleReuse(t *testing.T) {
234234
request := testRequest("Stat")
235235
request.handle = "1"
236236
pkt := fakePacket{myid: 1}
237-
rpkt := request.call(handlers, pkt, nil, 0)
237+
rpkt := request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
238238
assert.IsType(t, &sshFxpStatResponse{}, rpkt)
239239

240240
request.Method = "List"
@@ -244,6 +244,6 @@ func TestOpendirHandleReuse(t *testing.T) {
244244
hpkt := rpkt.(*sshFxpHandlePacket)
245245
assert.Equal(t, hpkt.Handle, "1")
246246
}
247-
rpkt = request.call(handlers, pkt, nil, 0)
247+
rpkt = request.call(handlers, pkt, nil, 0, defaultMaxTxPacket)
248248
assert.IsType(t, &sshFxpNamePacket{}, rpkt)
249249
}

‎server.go‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type Server struct {
3434
openFilesLock sync.RWMutex
3535
handleCount int
3636
workDir string
37+
maxTxPacket uint32
3738
}
3839

3940
func (svr *Server) nextHandle(f *os.File) string {
@@ -86,6 +87,7 @@ func NewServer(rwc io.ReadWriteCloser, options ...ServerOption) (*Server, error)
8687
debugStream: ioutil.Discard,
8788
pktMgr: newPktMgr(svrConn),
8889
openFiles: make(map[string]*os.File),
90+
maxTxPacket: defaultMaxTxPacket,
8991
}
9092

9193
for _, o := range options {
@@ -139,6 +141,24 @@ func WithServerWorkingDirectory(workDir string) ServerOption {
139141
}
140142
}
141143

144+
// WithMaxTxPacket sets the maximum size of the payload returned to the client,
145+
// measured in bytes. The default value is 32768 bytes, and this option
146+
// can only be used to increase it. Setting this option to a larger value
147+
// should be safe, because the client decides the size of the requested payload.
148+
//
149+
// The default maximum packet size is 32768 bytes.
150+
func WithMaxTxPacket(size uint32) ServerOption {
151+
return func(s *Server) error {
152+
if size < defaultMaxTxPacket {
153+
return errors.New("size must be greater than or equal to 32768")
154+
}
155+
156+
s.maxTxPacket = size
157+
158+
return nil
159+
}
160+
}
161+
142162
type rxPacket struct {
143163
pktType fxp
144164
pktBytes []byte
@@ -287,7 +307,7 @@ func handlePacket(s *Server, p orderedRequest) error {
287307
f, ok := s.getHandle(p.Handle)
288308
if ok {
289309
err = nil
290-
data := p.getDataSlice(s.pktMgr.alloc, orderID)
310+
data := p.getDataSlice(s.pktMgr.alloc, orderID, s.maxTxPacket)
291311
n, _err := f.ReadAt(data, int64(p.Offset))
292312
if _err != nil && (_err != io.EOF || n == 0) {
293313
err = _err
@@ -513,16 +533,16 @@ func (p *sshFxpSetstatPacket) respond(svr *Server) responsePacket {
513533

514534
fs, err := p.unmarshalFileStat(p.Flags)
515535

516-
if err == nil && (p.Flags & sshFileXferAttrSize) != 0 {
536+
if err == nil && (p.Flags&sshFileXferAttrSize) != 0 {
517537
err = os.Truncate(path, int64(fs.Size))
518538
}
519-
if err == nil && (p.Flags & sshFileXferAttrPermissions) != 0 {
539+
if err == nil && (p.Flags&sshFileXferAttrPermissions) != 0 {
520540
err = os.Chmod(path, fs.FileMode())
521541
}
522-
if err == nil && (p.Flags & sshFileXferAttrUIDGID) != 0 {
542+
if err == nil && (p.Flags&sshFileXferAttrUIDGID) != 0 {
523543
err = os.Chown(path, int(fs.UID), int(fs.GID))
524544
}
525-
if err == nil && (p.Flags & sshFileXferAttrACmodTime) != 0 {
545+
if err == nil && (p.Flags&sshFileXferAttrACmodTime) != 0 {
526546
err = os.Chtimes(path, fs.AccessTime(), fs.ModTime())
527547
}
528548

@@ -541,16 +561,16 @@ func (p *sshFxpFsetstatPacket) respond(svr *Server) responsePacket {
541561

542562
fs, err := p.unmarshalFileStat(p.Flags)
543563

544-
if err == nil && (p.Flags & sshFileXferAttrSize) != 0 {
564+
if err == nil && (p.Flags&sshFileXferAttrSize) != 0 {
545565
err = f.Truncate(int64(fs.Size))
546566
}
547-
if err == nil && (p.Flags & sshFileXferAttrPermissions) != 0 {
567+
if err == nil && (p.Flags&sshFileXferAttrPermissions) != 0 {
548568
err = f.Chmod(fs.FileMode())
549569
}
550-
if err == nil && (p.Flags & sshFileXferAttrUIDGID) != 0 {
570+
if err == nil && (p.Flags&sshFileXferAttrUIDGID) != 0 {
551571
err = f.Chown(int(fs.UID), int(fs.GID))
552572
}
553-
if err == nil && (p.Flags & sshFileXferAttrACmodTime) != 0 {
573+
if err == nil && (p.Flags&sshFileXferAttrACmodTime) != 0 {
554574
type chtimer interface {
555575
Chtimes(atime, mtime time.Time) error
556576
}

0 commit comments

Comments
 (0)