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 d60471d commit 1152f8f
Show file tree
Hide file tree
Showing 13 changed files with 129 additions and 96 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ ftpserver
.idea
.vscode
debug
__debug_bin
__debug_bin*
*.toml
20 changes: 20 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,31 @@ linters:
- noctx
- nonamedreturns
# - scopelint --> exportloopref
- nosprintfhostport
- exportloopref
- staticcheck
# - structcheck -> unused
- stylecheck
# - paralleltest -> buggy, doesn't work with subtests
- typecheck
- perfsprint
- prealloc
- predeclared
- reassign
- promlinter
- protogetter
- rowserrcheck
- sloglint
- spancheck
- sqlclosecheck
- stylecheck
- tagalign
- tagliatelle
- tenv
- testableexamples
- testifylint
# - testpackage -> too late for that
# - thelper
- unconvert
- unparam
- unused
Expand Down
7 changes: 4 additions & 3 deletions client_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ func (*testNetListener) Addr() net.Addr {
}

func TestDataConnectionRequirements(t *testing.T) {
r := require.New(t)
controlConnIP := net.ParseIP("192.168.1.1")

c := clientHandler{
Expand All @@ -420,7 +421,7 @@ func TestDataConnectionRequirements(t *testing.T) {
}

err := c.checkDataConnectionRequirement(controlConnIP, DataChannelPassive)
assert.NoError(t, err) // ip match
r.NoError(err) // ip match

err = c.checkDataConnectionRequirement(net.ParseIP("192.168.1.2"), DataChannelActive)
if assert.Error(t, err) {
Expand All @@ -432,12 +433,12 @@ func TestDataConnectionRequirements(t *testing.T) {
}

err = c.checkDataConnectionRequirement(controlConnIP, DataChannelPassive)
assert.Error(t, err)
r.Error(err)

// nil remote address
c.conn = &testNetConn{}
err = c.checkDataConnectionRequirement(controlConnIP, DataChannelActive)
assert.Error(t, err)
r.Error(err)

// invalid IP
c.conn = &testNetConn{
Expand Down
30 changes: 15 additions & 15 deletions errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,34 +31,34 @@ func TestTransferCloseStorageExceeded(t *testing.T) {
func TestErrorTypes(t *testing.T) {
// a := assert.New(t)
t.Run("DriverError", func(t *testing.T) {
a := assert.New(t)
r := require.New(t)
var err error = newDriverError("test", os.ErrPermission)
a.Equal("driver error: test: permission denied", err.Error())
a.ErrorIs(err, os.ErrPermission)
r.Equal("driver error: test: permission denied", err.Error())
r.ErrorIs(err, os.ErrPermission)

var specificError DriverError
a.ErrorAs(err, &specificError)
a.Equal("test", specificError.str)
r.ErrorAs(err, &specificError)
r.Equal("test", specificError.str)
})

t.Run("NetworkError", func(t *testing.T) {
a := assert.New(t)
r := require.New(t)
var err error = newNetworkError("test", os.ErrPermission)
a.Equal("network error: test: permission denied", err.Error())
a.ErrorIs(err, os.ErrPermission)
r.Equal("network error: test: permission denied", err.Error())
r.ErrorIs(err, os.ErrPermission)

var specificError NetworkError
a.ErrorAs(err, &specificError)
a.Equal("test", specificError.str)
r.ErrorAs(err, &specificError)
r.Equal("test", specificError.str)
})

t.Run("FileAccessError", func(t *testing.T) {
a := assert.New(t)
r := require.New(t)
var err error = newFileAccessError("test", os.ErrPermission)
a.Equal("file access error: test: permission denied", err.Error())
a.ErrorIs(err, os.ErrPermission)
r.Equal("file access error: test: permission denied", err.Error())
r.ErrorIs(err, os.ErrPermission)
var specificError FileAccessError
a.ErrorAs(err, &specificError)
a.Equal("test", specificError.str)
r.ErrorAs(err, &specificError)
r.Equal("test", specificError.str)
})
}
10 changes: 5 additions & 5 deletions handle_dirs.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (c *clientHandler) handleCWD(param string) error {
if stat, err := c.driver.Stat(p); err == nil {
if stat.IsDir() {
c.SetPath(p)
c.writeMessage(StatusFileOK, fmt.Sprintf("CD worked on %s", p))
c.writeMessage(StatusFileOK, "CD worked on "+p)
} else {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("Can't change directory to %s: Not a Directory", p))
}
Expand Down Expand Up @@ -98,7 +98,7 @@ func (c *clientHandler) handleMKDIR(params string) {
p := c.absPath(params)

if err := c.driver.MkdirAll(p, 0755); err == nil {
c.writeMessage(StatusFileOK, fmt.Sprintf("Created dir %s", p))
c.writeMessage(StatusFileOK, "Created dir "+p)
} else {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("Couldn't create dir %s: %v", p, err))
}
Expand All @@ -116,7 +116,7 @@ func (c *clientHandler) handleRMD(param string) error {
}

if err == nil {
c.writeMessage(StatusFileOK, fmt.Sprintf("Deleted dir %s", p))
c.writeMessage(StatusFileOK, "Deleted dir "+p)
} else {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("Could not delete dir %s: %v", p, err))
}
Expand All @@ -134,7 +134,7 @@ func (c *clientHandler) handleRMDIR(params string) {
p := c.absPath(params)

if err := c.driver.RemoveAll(p); err == nil {
c.writeMessage(StatusFileOK, fmt.Sprintf("Removed dir %s", p))
c.writeMessage(StatusFileOK, "Removed dir "+p)
} else {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("Couldn't remove dir %s: %v", p, err))
}
Expand All @@ -148,7 +148,7 @@ func (c *clientHandler) handleCDUP(_ string) error {

if _, err := c.driver.Stat(parent); err == nil {
c.SetPath(parent)
c.writeMessage(StatusFileOK, fmt.Sprintf("CDUP worked on %s", parent))
c.writeMessage(StatusFileOK, "CDUP worked on "+parent)
} else {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("CDUP issue: %v", err))
}
Expand Down
36 changes: 19 additions & 17 deletions handle_dirs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,10 @@ func TestDirListingWithSpace(t *testing.T) {

defer func() { require.NoError(t, raw.Close()) }()

rc, response, err := raw.SendCommand(fmt.Sprintf("CWD /%s", dirName))
rc, response, err := raw.SendCommand("CWD /" + dirName)
require.NoError(t, err)
require.Equal(t, StatusFileOK, rc)
require.Equal(t, fmt.Sprintf("CD worked on /%s", dirName), response)
require.Equal(t, "CD worked on /"+dirName, response)

dcGetter, err := raw.PrepareDataConn()
require.NoError(t, err)
Expand Down Expand Up @@ -354,7 +354,7 @@ func TestCleanPath(t *testing.T) {
"/./",
"/././.",
} {
rc, response, err := raw.SendCommand(fmt.Sprintf("CWD %s", dir))
rc, response, err := raw.SendCommand("CWD " + dir)
require.NoError(t, err)
require.Equal(t, StatusFileOK, rc)
require.Equal(t, "CD worked on /", response)
Expand Down Expand Up @@ -389,7 +389,7 @@ func TestTLSTransfer(t *testing.T) {

contents, err := c.ReadDir("/")
require.NoError(t, err)
require.Len(t, contents, 0)
require.Empty(t, contents)

raw, err := c.OpenRawConn()
require.NoError(t, err, "Couldn't open raw connection")
Expand Down Expand Up @@ -500,14 +500,16 @@ func TestListArgs(t *testing.T) {
}

func testListDirArgs(t *testing.T, s *FtpServer) {
r := require.New(t)

conf := goftp.Config{
User: authUser,
Password: authPass,
}
testDir := "testdir"

c, err := goftp.DialConfig(conf, s.Addr())
require.NoError(t, err, "Couldn't connect")
r.NoError(err, "Couldn't connect")

defer func() { panicOnError(c.Close()) }()

Expand All @@ -520,31 +522,31 @@ func testListDirArgs(t *testing.T, s *FtpServer) {
s.settings.DisableLISTArgs = false

contents, err := c.ReadDir(arg)
require.NoError(t, err)
require.Len(t, contents, 0)
r.NoError(err)
r.Empty(contents)

_, err = c.Mkdir(arg)
require.NoError(t, err)
r.NoError(err)

_, err = c.Mkdir(fmt.Sprintf("%v/%v", arg, testDir))
require.NoError(t, err)
r.NoError(err)

contents, err = c.ReadDir(arg)
require.NoError(t, err)
require.Len(t, contents, 1)
require.Equal(t, contents[0].Name(), testDir)
r.NoError(err)
r.Len(contents, 1)
r.Equal(contents[0].Name(), testDir)

contents, err = c.ReadDir(fmt.Sprintf("%v %v", arg, arg))
require.NoError(t, err)
require.Len(t, contents, 1)
require.Equal(t, contents[0].Name(), testDir)
r.NoError(err)
r.Len(contents, 1)
r.Equal(contents[0].Name(), testDir)

// cleanup
err = c.Rmdir(fmt.Sprintf("%v/%v", arg, testDir))
require.NoError(t, err)
r.NoError(err)

err = c.Rmdir(arg)
require.NoError(t, err)
r.NoError(err)
}
}

Expand Down
17 changes: 9 additions & 8 deletions handle_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (c *clientHandler) handleRETR(param string) error {

// File transfer, read or write, seek or not, is basically the same.
// To make sure we don't miss any step, we execute everything in order
func (c *clientHandler) transferFile(write bool, append bool, param, info string) {
func (c *clientHandler) transferFile(write bool, appendFile bool, param, info string) {
var file FileTransfer
var err error
var fileFlag int
Expand All @@ -53,7 +53,7 @@ func (c *clientHandler) transferFile(write bool, append bool, param, info string
// We try to open the file
if write { //nolint:nestif // too much effort to change for now
fileFlag = os.O_WRONLY
if append {
if appendFile {
fileFlag |= os.O_CREATE | os.O_APPEND
// ignore the seek position for append mode
c.ctxRest = 0
Expand Down Expand Up @@ -353,7 +353,7 @@ func (c *clientHandler) handleSYMLINK(params string) {
func (c *clientHandler) handleDELE(param string) error {
path := c.absPath(param)
if err := c.driver.Remove(path); err == nil {
c.writeMessage(StatusFileOK, fmt.Sprintf("Removed file %s", path))
c.writeMessage(StatusFileOK, "Removed file "+path)
} else {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("Couldn't delete %s: %v", path, err))
}
Expand Down Expand Up @@ -408,7 +408,7 @@ func (c *clientHandler) handleSIZE(param string) error {

path := c.absPath(param)
if info, err := c.driver.Stat(path); err == nil {
c.writeMessage(StatusFileStatus, fmt.Sprintf("%d", info.Size()))
c.writeMessage(StatusFileStatus, strconv.FormatInt(info.Size(), 10))
} else {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("Couldn't access %s: %v", path, err))
}
Expand All @@ -430,7 +430,7 @@ func (c *clientHandler) handleSTATFile(param string) error {
if !info.IsDir() {
defer c.multilineAnswer(StatusFileStatus, fmt.Sprintf("STAT %v", param))()

c.writeLine(fmt.Sprintf(" %s", c.fileStat(info)))
c.writeLine(" " + c.fileStat(info))

return nil
}
Expand Down Expand Up @@ -459,7 +459,7 @@ func (c *clientHandler) handleSTATFile(param string) error {
defer c.multilineAnswer(StatusDirectoryStatus, fmt.Sprintf("STAT %v", param))()

for _, f := range files {
c.writeLine(fmt.Sprintf(" %s", c.fileStat(f)))
c.writeLine(" %s" + c.fileStat(f))
}
} else {
c.writeMessage(StatusFileActionNotTaken, fmt.Sprintf("Could not list: %v", errList))

Check warning on line 465 in handle_files.go

View check run for this annotation

Codecov / codecov/patch

handle_files.go#L465

Added line #L465 was not covered by tests
Expand Down Expand Up @@ -548,8 +548,9 @@ func (c *clientHandler) handleMDTM(param string) error {
func (c *clientHandler) handleMFMT(param string) error {
params := strings.SplitN(param, " ", 2)
if len(params) != 2 {
c.writeMessage(StatusSyntaxErrorNotRecognised, fmt.Sprintf(
"Couldn't set mtime, not enough params, given: %s", param))
c.writeMessage(StatusSyntaxErrorNotRecognised,
"Couldn't set mtime, not enough params, given: "+param,
)

return nil
}
Expand Down
3 changes: 2 additions & 1 deletion handle_files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"os"
"regexp"
"strconv"
"strings"
"testing"

Expand Down Expand Up @@ -662,7 +663,7 @@ func TestCOMB(t *testing.T) {
createTemporaryFile(t, partSize), createTemporaryFile(t, partSize))

for idx, part := range parts {
ftpUpload(t, c, part, fmt.Sprintf("%d", idx))
ftpUpload(t, c, part, strconv.Itoa(idx))
_, err = part.Seek(0, io.SeekStart)
require.NoError(t, err)
_, err = io.Copy(hasher, part)
Expand Down
9 changes: 5 additions & 4 deletions handle_misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/tls"
"errors"
"fmt"
"strconv"
"strings"
"time"
)
Expand Down Expand Up @@ -89,7 +90,7 @@ func (c *clientHandler) handleSITE(param string) error {
case "RMDIR":
c.handleRMDIR(params)
default:
c.writeMessage(StatusSyntaxErrorNotRecognised, fmt.Sprintf("Unknown SITE subcommand: %s", cmd))
c.writeMessage(StatusSyntaxErrorNotRecognised, "Unknown SITE subcommand: "+cmd)
}

return nil
Expand Down Expand Up @@ -121,7 +122,7 @@ func (c *clientHandler) handleSTATServer() error {
))

if c.user != "" {
c.writeLine(fmt.Sprintf("Logged in as %s", c.user))
c.writeLine("Logged in as " + c.user)
} else {
c.writeLine("Not logged in yet")
}
Expand Down Expand Up @@ -344,7 +345,7 @@ func (c *clientHandler) handleAVBL(param string) error {
}

if !info.IsDir() {
c.writeMessage(StatusActionNotTaken, fmt.Sprintf("%s: is not a directory", path))
c.writeMessage(StatusActionNotTaken, path+": is not a directory")

return nil
}
Expand All @@ -356,7 +357,7 @@ func (c *clientHandler) handleAVBL(param string) error {
return nil
}

c.writeMessage(StatusFileStatus, fmt.Sprintf("%d", available))
c.writeMessage(StatusFileStatus, strconv.FormatInt(available, 10))
} else {
c.writeMessage(StatusNotImplemented, "This extension hasn't been implemented !")
}
Expand Down
Loading

0 comments on commit 1152f8f

Please sign in to comment.