From bfb6dd60b3abceb809e2f372c17776d925b4ad9a Mon Sep 17 00:00:00 2001
From: Nicola Murino <nicola.murino@gmail.com>
Date: Sun, 2 Jun 2024 20:57:14 +0200
Subject: [PATCH] fix quoted, space separated, HASH parameters (#459) by
 @drakkan  as suggested by @syncplify

---
 handle_files.go      | 55 ++++++++++++++++++++++++++++----------------
 handle_files_test.go | 37 +++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/handle_files.go b/handle_files.go
index bbf895ff..a7a403a1 100644
--- a/handle_files.go
+++ b/handle_files.go
@@ -606,9 +606,14 @@ func (c *clientHandler) handleGenericHash(param string, algo HASHAlgo, isCustomM
 		return nil
 	}
 
-	args := strings.SplitN(param, " ", 3)
-	info, err := c.driver.Stat(args[0])
+	args, err := unquoteSpaceSeparatedParams(param)
+	if err != nil || len(args) == 0 {
+		c.writeMessage(StatusSyntaxErrorParameters, fmt.Sprintf("invalid HASH parameters: %v", param))
+
+		return nil //nolint:nilerr
+	}
 
+	info, err := c.driver.Stat(args[0])
 	if err != nil {
 		c.writeMessage(StatusActionNotTaken, fmt.Sprintf("%v: %v", param, err))
 
@@ -627,25 +632,11 @@ func (c *clientHandler) handleGenericHash(param string, algo HASHAlgo, isCustomM
 	// to support partial hash also for the HASH command, we should implement RANG,
 	// but it applies also to uploads/downloads and so it complicates their handling,
 	// we'll add this support in future improvements
-	if isCustomMode { //nolint:nestif // too much effort to change for now
-		// for custom command the range can be specified in this way:
-		// XSHA1 <file> <start> <end>
-		if len(args) > 1 {
-			start, err = strconv.ParseInt(args[1], 10, 64)
-			if err != nil {
-				c.writeMessage(StatusSyntaxErrorParameters, fmt.Sprintf("invalid start offset %v: %v", args[1], err))
-
-				return nil
-			}
-		}
-
-		if len(args) > 2 {
-			end, err = strconv.ParseInt(args[2], 10, 64)
-			if err != nil {
-				c.writeMessage(StatusSyntaxErrorParameters, fmt.Sprintf("invalid end offset %v: %v", args[2], err))
+	if isCustomMode {
+		if err = getPartialHASHRange(args, &start, &end); err != nil {
+			c.writeMessage(StatusSyntaxErrorParameters, err.Error())
 
-				return nil
-			}
+			return nil
 		}
 	}
 
@@ -747,6 +738,30 @@ func (c *clientHandler) closeUnchecked(file io.Closer) {
 	}
 }
 
+func getPartialHASHRange(args []string, start *int64, end *int64) error {
+	// for custom HASH commands the range can be specified in this way:
+	// XSHA1 <file> <start> <end>
+	if len(args) > 1 {
+		val, err := strconv.ParseInt(args[1], 10, 64)
+		if err != nil {
+			return fmt.Errorf("invalid start offset %v: %w", args[1], err)
+		}
+
+		*start = val
+	}
+
+	if len(args) > 2 {
+		val, err := strconv.ParseInt(args[2], 10, 64)
+		if err != nil {
+			return fmt.Errorf("invalid end offset %v: %w", args[1], err)
+		}
+
+		*end = val
+	}
+
+	return nil
+}
+
 // This method split params by spaces, except when the space is inside quotes.
 // It was introduced to support COMB command. Supported COMB examples:
 //
diff --git a/handle_files_test.go b/handle_files_test.go
index ad57f686..a41c4df9 100644
--- a/handle_files_test.go
+++ b/handle_files_test.go
@@ -546,6 +546,7 @@ func TestHASHCommand(t *testing.T) {
 	sha256Hash := "ceee704dd96e2b8c2ceca59c4c697bc01123fb9e66a1a3ac34dbdd2d6da9659b"
 
 	ftpUpload(t, client, tempFile, "file.txt")
+	ftpUpload(t, client, tempFile, "file with space.txt")
 
 	raw, err := client.OpenRawConn()
 	require.NoError(t, err, "Couldn't open raw connection")
@@ -562,6 +563,11 @@ func TestHASHCommand(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, StatusFileStatus, returnCode)
 	require.True(t, strings.HasSuffix(message, fmt.Sprintf("SHA-256 0-36 %v file.txt", sha256Hash)))
+	// test the same quoting the file name
+	returnCode, message, err = raw.SendCommand(`HASH "file with space.txt"`)
+	require.NoError(t, err)
+	require.Equal(t, StatusFileStatus, returnCode)
+	require.True(t, strings.HasSuffix(message, fmt.Sprintf("SHA-256 0-36 %v file with space.txt", sha256Hash)))
 
 	// change algo and request the hash again
 	returnCode, message, err = raw.SendCommand("OPTS HASH CRC32")
@@ -575,6 +581,37 @@ func TestHASHCommand(t *testing.T) {
 	require.True(t, strings.HasSuffix(message, fmt.Sprintf("CRC32 0-36 %v file.txt", crc32Sum)))
 }
 
+func TestHashWithoutParams(t *testing.T) {
+	server := NewTestServerWithTestDriver(
+		t,
+		&TestServerDriver{
+			Debug: false,
+			Settings: &Settings{
+				EnableHASH: true,
+			},
+		},
+	)
+	conf := goftp.Config{
+		User:     authUser,
+		Password: authPass,
+	}
+
+	client, err := goftp.DialConfig(conf, server.Addr())
+	require.NoError(t, err, "Couldn't connect")
+
+	defer func() { panicOnError(client.Close()) }()
+
+	raw, err := client.OpenRawConn()
+	require.NoError(t, err, "Couldn't open raw connection")
+
+	defer func() { require.NoError(t, raw.Close()) }()
+
+	returnCode, message, err := raw.SendCommand("HASH")
+	require.NoError(t, err)
+	require.Equal(t, StatusSyntaxErrorParameters, returnCode)
+	require.Contains(t, message, "invalid HASH parameters")
+}
+
 func TestCustomHASHCommands(t *testing.T) {
 	server := NewTestServer(t, false)
 	server.settings.EnableHASH = true