Skip to content

Commit

Permalink
progress
Browse files Browse the repository at this point in the history
  • Loading branch information
fclairamb committed Mar 10, 2024
1 parent 58e4978 commit ffcde43
Show file tree
Hide file tree
Showing 13 changed files with 29 additions and 48 deletions.
6 changes: 5 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ linters:
- godox
- goerr113
- gofmt
# - gofumpt -> conflicts with wsl
- goimports
- gosimple
# - golint --> revive
Expand Down Expand Up @@ -237,6 +238,7 @@ linters:
- prealloc
- nestif
- nilerr
- nilnil
- nolintlint
- nlreturn
- rowserrcheck
Expand Down Expand Up @@ -274,10 +276,12 @@ linters:
- unused
- usestdlibvars
- varnamelen
- wastedassign
# - varcheck -> unused
- whitespace
# - wrapcheck
# - wrapcheck -> too much effort for now
- wsl
# - zerologlint -> Will most probably never use it
fast: false

issues:
Expand Down
1 change: 0 additions & 1 deletion client_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ func TestConcurrency(t *testing.T) {
}

client, err := goftp.DialConfig(conf, server.Addr())

if err != nil {
panic(fmt.Sprintf("Couldn't connect: %v", err))
}
Expand Down
1 change: 0 additions & 1 deletion control_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ func Control(_, _ string, c syscall.RawConn) error {
return
}
})

if err != nil {
return fmt.Errorf("unable to set control options: %w", err)

Check warning on line 29 in control_unix.go

View check run for this annotation

Codecov / codecov/patch

control_unix.go#L29

Added line #L29 was not covered by tests
}
Expand Down
4 changes: 0 additions & 4 deletions driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,12 @@ type ClientDriver interface {

// ClientDriverExtensionAllocate is an extension to support the "ALLO" - file allocation - command
type ClientDriverExtensionAllocate interface {

// AllocateSpace reserves the space necessary to upload files
AllocateSpace(size int) error
}

// ClientDriverExtensionSymlink is an extension to support the "SITE SYMLINK" - symbolic link creation - command
type ClientDriverExtensionSymlink interface {

// Symlink creates a symlink
Symlink(oldname, newname string) error

Expand All @@ -96,15 +94,13 @@ type ClientDriverExtensionSymlink interface {
// ClientDriverExtensionFileList is a convenience extension to allow to return file listing
// without requiring to implement the methods Open/Readdir for your custom afero.File
type ClientDriverExtensionFileList interface {

// ReadDir reads the directory named by name and return a list of directory entries.
ReadDir(name string) ([]os.FileInfo, error)
}

// ClientDriverExtentionFileTransfer is a convenience extension to allow to transfer files
// without requiring to implement the methods Create/Open/OpenFile for your custom afero.File.
type ClientDriverExtentionFileTransfer interface {

// GetHandle return an handle to upload or download a file based on flags:
// os.O_RDONLY indicates a download
// os.O_WRONLY indicates an upload and can be combined with os.O_APPEND (resume) or
Expand Down
17 changes: 10 additions & 7 deletions driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (driver *TestServerDriver) Init() {

{
dir, _ := os.MkdirTemp("", "example")
if err := os.MkdirAll(dir, 0750); err != nil {
if err := os.MkdirAll(dir, 0o750); err != nil {
panic(err)
}

Expand Down Expand Up @@ -260,7 +260,7 @@ func (driver *TestServerDriver) AuthUser(_ ClientContext, user, pass string) (Cl
return clientdriver, nil
} else if user == "nil" && pass == "nil" {
// Definitely a bad behavior (but can be done on the driver side)
return nil, nil
return nil, nil //nolint:nilnil
}

return nil, errBadUserNameOrPassword
Expand Down Expand Up @@ -370,7 +370,8 @@ func (driver *TestServerDriver) PreAuthUser(cc ClientContext, _ string) error {
}

func (driver *TestServerDriver) VerifyConnection(_ ClientContext, _ string,
_ *tls.Conn) (ClientDriver, error) {
_ *tls.Conn,
) (ClientDriver, error) {
switch driver.TLSVerificationReply {
case tlsVerificationFailed:
return nil, errInvalidTLSCertificate
Expand All @@ -379,10 +380,10 @@ func (driver *TestServerDriver) VerifyConnection(_ ClientContext, _ string,

return clientdriver, nil
case tlsVerificationOK:
return nil, nil
return nil, nil //nolint:nilnil
}

return nil, nil
return nil, nil //nolint:nilnil
}

func (driver *TestServerDriver) WrapPassiveListener(listener net.Listener) (net.Listener, error) {
Expand Down Expand Up @@ -458,8 +459,10 @@ func (driver *TestClientDriver) GetAvailableSpace(dirName string) (int64, error)
return int64(123), nil
}

var errInvalidChownUser = errors.New("invalid chown on user")
var errInvalidChownGroup = errors.New("invalid chown on group")
var (
errInvalidChownUser = errors.New("invalid chown on user")
errInvalidChownGroup = errors.New("invalid chown on group")
)

func (driver *TestClientDriver) Chown(name string, uid int, gid int) error {
if uid != 0 && uid != authUserID {
Expand Down
2 changes: 0 additions & 2 deletions handle_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
func (c *clientHandler) handleUSER(user string) error {
if verifier, ok := c.server.driver.(MainDriverExtensionUserVerifier); ok {
err := verifier.PreAuthUser(c, user)

if err != nil {
c.writeMessage(StatusNotLoggedIn, fmt.Sprintf("User rejected: %v", err))
c.disconnect()
Expand Down Expand Up @@ -51,7 +50,6 @@ func (c *clientHandler) handleUserTLS(user string) bool {
}

driver, err := verifier.VerifyConnection(c, user, tlsConn)

if err != nil {
c.writeMessage(StatusNotLoggedIn, fmt.Sprintf("TLS verification failed: %v", err))
c.disconnect()
Expand Down
9 changes: 3 additions & 6 deletions handle_dirs.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (c *clientHandler) handleCWD(param string) error {

func (c *clientHandler) handleMKD(param string) error {
pathAbsolute := c.absPath(param)
if err := c.driver.Mkdir(pathAbsolute, 0755); err == nil {
if err := c.driver.Mkdir(pathAbsolute, 0o755); err == nil {
// handleMKD confirms to "quote-doubling"
// https://tools.ietf.org/html/rfc959 , page 63
c.writeMessage(StatusPathCreated, fmt.Sprintf(`Created dir "%s"`, quoteDoubling(pathAbsolute)))
Expand All @@ -97,7 +97,7 @@ func (c *clientHandler) handleMKDIR(params string) {

p := c.absPath(params)

if err := c.driver.MkdirAll(p, 0755); err == nil {
if err := c.driver.MkdirAll(p, 0o755); err == nil {
c.writeMessage(StatusFileOK, "Created dir "+p)
} else {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("Couldn't create dir %s: %v", p, err))
Expand Down Expand Up @@ -226,7 +226,6 @@ func (c *clientHandler) handleNLST(param string) error {
func (c *clientHandler) dirTransferNLST(writer io.Writer, files []os.FileInfo, parentDir string) error {
if len(files) == 0 {
_, err := writer.Write([]byte(""))

if err != nil {
err = newNetworkError("couldn't send NLST data", err)

Check warning on line 230 in handle_dirs.go

View check run for this annotation

Codecov / codecov/patch

handle_dirs.go#L230

Added line #L230 was not covered by tests
}
Expand Down Expand Up @@ -306,7 +305,6 @@ func (c *clientHandler) fileStat(file os.FileInfo) string {
func (c *clientHandler) dirTransferLIST(writer io.Writer, files []os.FileInfo) error {
if len(files) == 0 {
_, err := writer.Write([]byte(""))

if err != nil {
err = newNetworkError("error writing LIST entry", err)

Check warning on line 309 in handle_dirs.go

View check run for this annotation

Codecov / codecov/patch

handle_dirs.go#L309

Added line #L309 was not covered by tests
}
Expand All @@ -327,7 +325,6 @@ func (c *clientHandler) dirTransferLIST(writer io.Writer, files []os.FileInfo) e
func (c *clientHandler) dirTransferMLSD(writer io.Writer, files []os.FileInfo) error {
if len(files) == 0 {
_, err := writer.Write([]byte(""))

if err != nil {
err = newNetworkError("error writing MLSD entry", err)

Check warning on line 329 in handle_dirs.go

View check run for this annotation

Codecov / codecov/patch

handle_dirs.go#L329

Added line #L329 was not covered by tests
}
Expand All @@ -343,6 +340,7 @@ func (c *clientHandler) dirTransferMLSD(writer io.Writer, files []os.FileInfo) e

return nil
}

func (c *clientHandler) writeMLSxEntry(writer io.Writer, file os.FileInfo) error {
var listType string
if file.IsDir() {
Expand All @@ -359,7 +357,6 @@ func (c *clientHandler) writeMLSxEntry(writer io.Writer, file os.FileInfo) error
file.ModTime().UTC().Format(dateFormatMLSD),
file.Name(),
)

if err != nil {
err = fmt.Errorf("error writing MLSD entry: %w", err)

Check warning on line 361 in handle_dirs.go

View check run for this annotation

Codecov / codecov/patch

handle_dirs.go#L361

Added line #L361 was not covered by tests
}
Expand Down
4 changes: 2 additions & 2 deletions handle_dirs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestGetRelativePaths(t *testing.T) {
type relativePathTest struct {
workingDir, path, result string
}
var tests = []relativePathTest{
tests := []relativePathTest{
{"/", "/p", "p"},
{"/", "/", ""},
{"/p", "/p", ""},
Expand Down Expand Up @@ -590,7 +590,7 @@ func TestMLSDAndNLSTFilePathError(t *testing.T) {
defer func() { panicOnError(client.Close()) }()

// MLSD shouldn't work for filePaths
var fileName = "testfile.ext"
fileName := "testfile.ext"

_, err = client.ReadDir(fileName)
require.Error(t, err, "MLSD for not existing filePath must fail")
Expand Down
5 changes: 0 additions & 5 deletions handle_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ func (c *clientHandler) transferFile(write bool, appendFile bool, param, info st
}

file, err = c.getFileHandle(path, fileFlag, c.ctxRest)

// If this fail, can stop right here and reset the seek position
if err != nil {
if !c.isCommandAborted() {
Expand Down Expand Up @@ -420,7 +419,6 @@ func (c *clientHandler) handleSTATFile(param string) error {
path := c.absPath(param)

info, err := c.driver.Stat(path)

if err != nil {
c.writeMessage(StatusFileActionNotTaken, fmt.Sprintf("Could not STAT: %v", err))

Expand Down Expand Up @@ -496,7 +494,6 @@ func (c *clientHandler) handleMLST(param string) error {
func (c *clientHandler) handleALLO(param string) error {
// We should probably add a method in the driver
size, err := strconv.Atoi(param)

if err != nil {
c.writeMessage(StatusSyntaxErrorParameters, fmt.Sprintf("Couldn't parse size: %v", err))

Expand Down Expand Up @@ -701,7 +698,6 @@ func (c *clientHandler) computeHashForFile(filePath string, algo HASHAlgo, start
}

file, err = c.getFileHandle(filePath, os.O_RDONLY, start)

if err != nil {
return "", err
}
Expand Down Expand Up @@ -735,7 +731,6 @@ func (c *clientHandler) getFileHandle(name string, flags int, offset int64) (Fil
}

file, err := c.driver.OpenFile(name, flags, os.ModePerm)

if err != nil {
err = newDriverError("calling OpenFile", err)
}
Expand Down
14 changes: 3 additions & 11 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ import (
lognoop "github.com/fclairamb/go-log/noop"
)

var (
// ErrNotListening is returned when we are performing an action that is only valid while listening
ErrNotListening = errors.New("we aren't listening")
)
// ErrNotListening is returned when we are performing an action that is only valid while listening
var ErrNotListening = errors.New("we aren't listening")

// CommandDescription defines which function should be used and if it should be open to anyone or only logged in users
type CommandDescription struct {
Expand Down Expand Up @@ -95,9 +93,7 @@ var commandsMap = map[string]*CommandDescription{ //nolint:gochecknoglobals
"EPRT": {Fn: (*clientHandler).handlePORT},
}

var (
specialAttentionCommands = []string{"ABOR", "STAT", "QUIT"} //nolint:gochecknoglobals
)
var specialAttentionCommands = []string{"ABOR", "STAT", "QUIT"} //nolint:gochecknoglobals

// FtpServer is where everything is stored
// We want to keep it as simple as possible
Expand Down Expand Up @@ -163,7 +159,6 @@ func parseIPv4(publicHost string) (string, error) {
// It's not a blocking call
func (server *FtpServer) Listen() error {
err := server.loadSettings()

if err != nil {
return fmt.Errorf("could not load settings: %w", err)
}
Expand All @@ -174,7 +169,6 @@ func (server *FtpServer) Listen() error {
} else {
// Otherwise, it's what we currently use
server.listener, err = server.createListener()

if err != nil {
return fmt.Errorf("could not create listener: %w", err)
}
Expand All @@ -187,7 +181,6 @@ func (server *FtpServer) Listen() error {

func (server *FtpServer) createListener() (net.Listener, error) {
listener, err := net.Listen("tcp", server.settings.ListenAddr)

if err != nil {
server.Logger.Error("cannot listen on main port", "err", err, "listenAddr", server.settings.ListenAddr)

Expand Down Expand Up @@ -227,7 +220,6 @@ func (server *FtpServer) Serve() error {

for {
connection, err := server.listener.Accept()

if err != nil {
if ok, finalErr := server.handleAcceptError(err, &tempDelay); ok {
return finalErr
Expand Down
4 changes: 0 additions & 4 deletions transfer_active.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ func (a *activeTransferHandler) Open() (net.Conn, error) {
}

conn, err := dialer.Dial("tcp", a.raddr.String())

if err != nil {
return nil, newNetworkError("could not establish active connection", err)

Check warning on line 103 in transfer_active.go

View check run for this annotation

Codecov / codecov/patch

transfer_active.go#L103

Added line #L103 was not covered by tests
}
Expand Down Expand Up @@ -151,15 +150,13 @@ func parsePORTAddr(param string) (*net.TCPAddr, error) {
}

portByte2, err := strconv.Atoi(params[5])

if err != nil {
return nil, ErrRemoteAddrFormat

Check warning on line 154 in transfer_active.go

View check run for this annotation

Codecov / codecov/patch

transfer_active.go#L154

Added line #L154 was not covered by tests
}

port := portByte1<<8 + portByte2

addr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("%s:%d", ipParts, port))

if err != nil {
err = newNetworkError("could not resolve "+param, err)

Check warning on line 161 in transfer_active.go

View check run for this annotation

Codecov / codecov/patch

transfer_active.go#L161

Added line #L161 was not covered by tests
}
Expand Down Expand Up @@ -204,7 +201,6 @@ func parseEPRTAddr(param string) (*net.TCPAddr, error) {
}

addr, err = net.ResolveTCPAddr("tcp", net.JoinHostPort(ipAddress.String(), strconv.Itoa(portI)))

if err != nil {
err = newNetworkError(fmt.Sprintf("could not resolve addr %v:%v", ipAddress, portI), err)

Check warning on line 205 in transfer_active.go

View check run for this annotation

Codecov / codecov/patch

transfer_active.go#L205

Added line #L205 was not covered by tests
}
Expand Down
1 change: 0 additions & 1 deletion transfer_pasv.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ func (p *passiveTransferHandler) ConnectionWait(wait time.Duration) (net.Conn, e
}

p.connection, err = p.listener.Accept()

if err != nil {
return nil, fmt.Errorf("failed to accept passive transfer connection: %w", err)

Check warning on line 229 in transfer_pasv.go

View check run for this annotation

Codecov / codecov/patch

transfer_pasv.go#L229

Added line #L229 was not covered by tests
}
Expand Down
9 changes: 6 additions & 3 deletions transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ func TestTransfer(t *testing.T) {
Settings: &Settings{
ActiveTransferPortNon20: true,
TLSRequired: ImplicitEncryption,
}})
},
})

testTransferOnConnection(t, server, false, true, true)
testTransferOnConnection(t, server, true, true, true)
Expand Down Expand Up @@ -747,8 +748,10 @@ func TestABORWithoutOpenTransfer(t *testing.T) {
require.NoError(t, err)
require.Equal(t, StatusFileActionPending, returnCode, response)

for _, cmd := range []string{"RETR delay-io-fail-to-seek.bin", "LIST delay-io-fail-to-readdir",
"NLST delay-io-fail-to-readdir", "MLSD delay-io-fail-to-readdir"} {
for _, cmd := range []string{
"RETR delay-io-fail-to-seek.bin", "LIST delay-io-fail-to-readdir",
"NLST delay-io-fail-to-readdir", "MLSD delay-io-fail-to-readdir",
} {
_, err = raw.PrepareDataConn()
require.NoError(t, err)

Expand Down

0 comments on commit ffcde43

Please sign in to comment.