Browse Source

fix quota for uploads outside home dir if rename fails

Signed-off-by: Nicola Murino <[email protected]>
Nicola Murino 3 years ago
parent
commit
7c8bb5b18a
2 changed files with 65 additions and 11 deletions
  1. 52 0
      common/protocol_test.go
  2. 13 11
      common/transfer.go

+ 52 - 0
common/protocol_test.go

@@ -2409,6 +2409,58 @@ func TestFsPermissionErrors(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NoError(t, err)
 }
 }
 
 
+func TestRenameErrorOutsideHomeDir(t *testing.T) {
+	if runtime.GOOS == osWindows {
+		t.Skip("this test is not available on Windows")
+	}
+	oldUploadMode := common.Config.UploadMode
+	oldTempPath := common.Config.TempPath
+
+	common.Config.UploadMode = common.UploadModeAtomicWithResume
+	common.Config.TempPath = filepath.Clean(os.TempDir())
+	vfs.SetTempPath(common.Config.TempPath)
+
+	u := getTestUser()
+	u.QuotaFiles = 1000
+	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+
+	conn, client, err := getSftpClient(user)
+	if assert.NoError(t, err) {
+		defer conn.Close()
+		defer client.Close()
+
+		err = os.Chmod(user.GetHomeDir(), 0555)
+		assert.NoError(t, err)
+
+		err = checkBasicSFTP(client)
+		assert.NoError(t, err)
+		f, err := client.Create(testFileName)
+		assert.NoError(t, err)
+		_, err = f.Write(testFileContent)
+		assert.NoError(t, err)
+		err = f.Close()
+		assert.ErrorIs(t, err, os.ErrPermission)
+
+		user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK)
+		assert.NoError(t, err)
+		assert.Equal(t, 0, user.UsedQuotaFiles)
+		assert.Equal(t, int64(0), user.UsedQuotaSize)
+
+		err = os.Chmod(user.GetHomeDir(), os.ModeDir)
+		assert.NoError(t, err)
+	}
+
+	_, err = httpdtest.RemoveUser(user, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(user.GetHomeDir())
+	assert.NoError(t, err)
+
+	common.Config.UploadMode = oldUploadMode
+	common.Config.TempPath = oldTempPath
+	vfs.SetTempPath(oldTempPath)
+}
+
 func TestResolvePathError(t *testing.T) {
 func TestResolvePathError(t *testing.T) {
 	u := getTestUser()
 	u := getTestUser()
 	u.HomeDir = "relative_path"
 	u.HomeDir = "relative_path"

+ 13 - 11
common/transfer.go

@@ -306,19 +306,21 @@ func (t *BaseTransfer) getUploadFileSize() (int64, error) {
 	return fileSize, err
 	return fileSize, err
 }
 }
 
 
-// return 1 if the file is deleted
+// return 1 if the file is outside the user home dir
 func (t *BaseTransfer) checkUploadOutsideHomeDir(err error) int {
 func (t *BaseTransfer) checkUploadOutsideHomeDir(err error) int {
-	if Config.TempPath != "" && err != nil {
-		errRm := t.Fs.Remove(t.effectiveFsPath, false)
-		t.Connection.Log(logger.LevelWarn, "atomic upload in temp path cannot be renamed, delete temporary file: %#v, deletion error: %v",
-			t.effectiveFsPath, errRm)
-		if errRm == nil {
-			atomic.StoreInt64(&t.BytesReceived, 0)
-			t.MinWriteOffset = 0
-			return 1
-		}
+	if err == nil {
+		return 0
+	}
+	if Config.TempPath == "" {
+		return 0
 	}
 	}
-	return 0
+	err = t.Fs.Remove(t.effectiveFsPath, false)
+	t.Connection.Log(logger.LevelWarn, "upload in temp path cannot be renamed, delete temporary file: %#v, deletion error: %v",
+		t.effectiveFsPath, err)
+	// the file is outside the home dir so don't update the quota
+	atomic.StoreInt64(&t.BytesReceived, 0)
+	t.MinWriteOffset = 0
+	return 1
 }
 }
 
 
 // Close it is called when the transfer is completed.
 // Close it is called when the transfer is completed.