Commit Graph

74 Commits

Author SHA1 Message Date
John Eikenberry 7f7e75b40d ensure packet responses in same order as requests
Previous code used the request ids to do ordering. This worked until a
client came along that used un-ordered request ids. This reworks the
ordering to use an internal counter (per session) to order all packets
ensuring that responses are sent in the same order as the requests were
received.

Fixes #260
2018-08-01 13:18:57 -07:00
John Eikenberry 1afc1d9a78 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).
2018-07-25 15:01:43 -07:00
John Eikenberry b50b1f9eaf fix sendError usage to match packet type signature
sendError takes a requestPacket but was simplifying it to an ider
interface. Future work needed it to be requestPacket but I wanted to fix
didn't up type usage in its own commit.
2018-07-23 16:53:51 -07:00
John Eikenberry 523bded012 replace interface{} with more specific type 2018-07-23 16:38:21 -07:00
John Eikenberry 048358fb96 use correct param type instead of asserting
Instead of accepting a more general type and then asserting it to the
proper type, just take the proper type as the argument.
Also clean up some of the use of it where it checked old direct sending
code's return error (error is now always nil).
2018-07-23 15:53:41 -07:00
John Eikenberry 4cad28be43 eliminate bare values in composite literals
Using key-value pairs is much more future proof.
2018-07-23 14:49:12 -07:00
John Eikenberry d0a1c8098b remove unnecessary type
Added to shorten code text, but not used enough to be worth the extra type.
2018-07-23 12:18:00 -07:00
John Eikenberry 218c0d4148 Opendir return an error status when not found
The initial Opendir packet is supposed to repond with an error status if
the directory wasn't found. It was just returning a handle without
checking, now it does a Stat on the path and only returns the handle if
the Stat is successful and it indicates it is a directory, otherwise it
returns an error.
2018-05-26 13:59:55 -07:00
Allan Feid 820ccceeef Send unsupported error on extended packets.
Following the rules outlined here:

https://tools.ietf.org/html/draft-ietf-secsh-filexfer-extensions-00

Return an SSH_FXP_STATUS with appropriate status error for extended
packets that we do not support.
2018-03-19 10:32:22 -04:00
John Eikenberry 9649a986f0 remove unused variables 2018-02-15 11:00:22 -08:00
John Eikenberry 738e088bbd support handlers returning explicit error codes
Add errors for all the SSH_FXP_STATUS codes to give the developer
implementing request server handlers greater control over the returned
codes. Most helpful in cases where nothing currently would work (eg.
unsupported).

Fixes #223
2018-01-26 17:26:44 -08:00
John Eikenberry 4ab81b0271 improve statusFromError readability 2018-01-25 17:29:36 -08:00
John Eikenberry bc6b56aae0 packageManager to use pointer receivers everywhere
Had a problem with getting a copy because a value receiver called a
pointer receiver.
2017-08-20 15:23:55 -07:00
John Eikenberry 26ceac758e add os.ErrNotExist handling into statusFromError
And remove errorAdapter() as this makes it unnecessary.
2017-08-20 14:54:45 -07:00
andreas 4d7bb970c4 Resolved conflict with SftpServerWorkerCount
Splitted cleanPath into cleanPacketPath and cleanPath for better handling of slashes in file paths
Added test for cleanPath func
Removed code duplication => filepath.ToSlash(filepath.Clean(...)) => cleanPath(...)
Fixed tests for runLs to match year or time
Renamed constants to fit hound rules
2017-08-13 14:00:08 +02:00
andreas ec08c0d53b Moved permission detection for runLs to server.go
Make runLs return the recommended format in stubs, even if it does not contain all information
Removed unused parameter
2017-08-10 07:34:48 +02:00
John Eikenberry e97b9a47e1 avoid data race in worker creation
There is a data race with the waitgroup (wg) object used to synchronize
the workers with the server exit. The workers called wg.Add()
asynchronously and it was possible for the Wait() to get hit before any
of the Add() calls were made in certain conditions. I only ever saw this
sporatically in the travis tests.

This fixes it by making the wg.Add() calls synchronous.
2017-07-03 17:53:55 -07:00
John Eikenberry 07d4ed1b5c don't copy lock
govet pointed out that I was copying a lock when creating/assigning the
connection object. It didn't cause any issues, but was a bad practice.
2017-07-03 17:45:58 -07:00
John Eikenberry 5024cb048c Move packer ordering code into packet-manager
The worker/packet mangement code needs to be in the packet manager so
the request-server can utilize it as well. This also improves the
encapsulation of the method as it relied on internal data that should be
better isolated inside the file/struct.
2017-04-23 13:47:05 -07:00
John Eikenberry d1bd7b3f9c ensure packets are processed in order
File operations that happen after the open packet has been received,
like reading/writing, can be done with the pool as the order they are
run in doesn't matter (the packets contain the file offsets).

Command operations, on the other hand, need to be serialized.

This flips between a pool of workers for file operations and a single
worker for everything else. It flips on Open and Close packets.
2017-04-17 17:24:15 -07:00
John Eikenberry 5fd073bcc3 decouple packet channel from server struct
looking to create 2 pools, so I want to pass the channel in
2017-04-17 17:24:15 -07:00
Pavel Borzenkov 9aa225fc15 Don't copy Server struct in sendPacket/sendError
Server struct contains Mutex which might be copied in inconsistent
state. Avoid this by declaring methods on pointer receiver.

This calms down go race detector.

Signed-off-by: Pavel Borzenkov <pavel.borzenkov@gmail.com>
2017-04-05 14:15:57 -07:00
John Eikenberry 5152c21cf9 integrate packageManager into servers 2017-03-23 16:15:04 -07:00
John Eikenberry 51cb116815 better name: packet -> requestPacket 2017-03-23 16:15:04 -07:00
John Eikenberry 89c474ca1d new name for id (now ider) interface
more in line with go naming scheme for interfaces
better greppability
2017-03-23 16:15:04 -07:00
John Eikenberry 0b42971846 packet unmarshalling code into main server loop
This means we are now passing full packets to the workers. This makes
the request id's available which is required for fixing packet ordering.
Benchmark tests weren't affected at all.
2017-03-23 16:15:03 -07:00
John Eikenberry 4f1fe4fe3b Use common packet typing code
Remove duplication due to request code being separate.
2017-03-23 16:15:03 -07:00
John Eikenberry 890b9f1a4a change readonly checks to interface type checks 2017-03-23 16:15:03 -07:00
George Xie 725c6ac61d make path more Unix like on windows servers 2017-03-01 20:04:44 -08:00
Dave Cheney d4c18e7ffd Integrate readPacket and writePacket into handlePacket (#120) 2016-06-15 21:17:20 +10:00
Dave Cheney 3a7eae5fbb Introduce serverConn (#119) 2016-06-15 21:08:29 +10:00
Dave Cheney 7f77924bb0 Centralise packet handling for common packets … (#111)
* Centralise packet handling for common packets

Updates #84, #95, #96

* use sendError
2016-06-15 20:47:20 +10:00
Dave Cheney ea1a7bcdfc conn: seperate io.Reader and io.WriteCloser (#118)
Reading and Writing are less coupled in the sftp client and server than
they may be over a traditional ReadWriter. This mirrors the full duplex
/ half close nature of a TCP connection.

In essence conn.Reader cannot be closed locally, it can only close in
response to the remote side using conn.WriteCloser.Close(), or a network
error, which is effectively the same thing.

Make this behaviour super clear by no longer smooshing the type into a
ReadWriteCloser.
2016-06-15 20:19:51 +10:00
Dave Cheney 9d52a809ce Always close client conn when clientConn.recv exits (#117)
If recv has exited then nobody will retrive the response, so
make sure we cannot send anything by shutting down the Write side of the
connection.
2016-06-15 20:04:26 +10:00
Dave Cheney 7d4fc94878 Introduce sftp.conn type to handle common send/recv behaviour (#113)
Introduce sftp.conn type to bind packet send/recv to a
io.ReadWriteCloser.
2016-06-15 11:50:02 +10:00
Dave Cheney c5050bbe83 removed unused code 2016-06-15 11:33:39 +10:00
Dave Cheney 4c7551ad7e Refactor all the calls to sendPacket(statusFromError(...)) to a sendError helper (#114)
* statusFromError now takes a type with an id() uint32 method

* Introduce sendError

Refactor all the calls to sendPacket(statusFromError(...)) to a
sendError helper.
2016-06-14 21:11:15 +10:00
Mark Sheahan 9ff4de5c31 add serverside StatVFS function, implemented for darwin and linux (#89) 2016-06-13 14:45:13 +10:00
Dave Cheney 8bf4044e7f remove svr.in svr.out, replace with svr.rwc (#107) 2016-05-29 18:22:40 +10:00
Dave Cheney 4f0e386862 Cleanup server code (#106)
Cleanup server code, especially around the shutdown logic, now there are
no more errors about leaking files during the tests.
2016-05-29 17:59:23 +10:00
Dave Cheney f3fc26f1c3 General server cleanups (#103)
- no need to initalise mutexes, their zero value is valid
- make NewServer simpler, 90% of use cases already had an
  io.ReadWriteCloser, in the single case that it it is easy to provide a
  simple wrapper.
2016-05-29 16:32:05 +10:00
Dave Cheney dfb5f84e82 Use pkg/errors for error handling (#101) (#102) 2016-05-23 20:54:08 +10:00
Dave Cheney 5e9ad277fc Use pkg/errors for error handling (#101) 2016-05-19 15:16:48 +10:00
Dave Cheney ca0111c37f server: fix ignored error handling in server (#100) 2016-05-19 15:04:32 +10:00
Dave Cheney bd4f7a70da server: refactor packet decoding and read only handling 2016-05-19 14:49:23 +10:00
Matt Layher 8e47c759f5 server: remote rootDir parameter, as it does nothing 2016-01-16 12:41:01 -05:00
Matt Layher 7d53b6d4ab server: return ServerOption for godoc documentation 2016-01-11 12:21:15 -05:00
Matt Layher d396b1b4cb Merge pull request #80 from pkg/functional-options
server: use functional options for NewServer
2016-01-11 11:56:30 -05:00
Matt Layher 403657b31e server: use functional options for NewServer 2016-01-11 11:52:51 -05:00
Matt Layher 167a495725 packet, server: use a single read-only check 2016-01-11 11:50:10 -05:00