db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-11-06 08:49:29 +02:00
Code Issues Releases Activity

Fix recursive path remove in SFTP storage driver.

Browse Source 
storageSftpPathRemove() used LIBSSH2_FX_FAILURE to determine when it was attempting to unlink a directory, but it appears that LIBSSH2_FX_PERMISSION_DENIED is also valid for this case.

Update storageSftpPathRemove() to accept either error and adjust tests.
This commit is contained in:
Reid Thompson
2023-11-18 08:47:58 -05:00
committed by GitHub
parent e2b734eff9
commit c4dc4665f8
4 changed files with 80 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
doc/xml/release/2023/2.49.xml
View File

@@ -20,6 +20,19 @@
<p>Fix regression in retries.</p>
</release-item>
<release-item>
<github-issue id="2219"/>
<github-pull-request id="2226"/>
<release-item-contributor-list>
<release-item-ideator id="luc.languagetools"/>
<release-item-contributor id="reid.thompson"/>
<release-item-reviewer id="stephen.frost"/>
</release-item-contributor-list>
<p>Fix recursive path remove in <proper>SFTP</proper> storage driver.</p>
</release-item>
</release-bug-list>
<release-improvement-list>

5
doc/xml/release/contributor.xml
View File

@@ -630,6 +630,11 @@
<contributor-id type="github">fluca1978</contributor-id>
</contributor>
<contributor id="luc.languagetools">
<contributor-name-display>Luc</contributor-name-display>
<contributor-id type="github">luc-languagetools</contributor-id>
</contributor>
<contributor id="lukas.ertl">
<contributor-name-display>Lukas Ertl</contributor-name-display>
<contributor-id type="github">lukasertl</contributor-id>

24
src/storage/sftp/storage.c
View File

@@ -952,14 +952,22 @@ storageSftpPathRemove(THIS_VOID, const String *const path, const bool recurse, c
if (rc == LIBSSH2_ERROR_EAGAIN)
THROW_FMT(PathRemoveError, "timeout removing file '%s'", strZ(file));
// Attempting to unlink a directory appears to return LIBSSH2_FX_FAILURE
if (rc == LIBSSH2_ERROR_SFTP_PROTOCOL &&
libssh2_sftp_last_error(this->sftpSession) == LIBSSH2_FX_FAILURE)
// Attempting to unlink a directory appears to return LIBSSH2_FX_FAILURE or LIBSSH2_FX_PERMISSION_DENIED
if (rc == LIBSSH2_ERROR_SFTP_PROTOCOL)
{
storageInterfacePathRemoveP(this, file, true);
const uint64_t sftpErrno = libssh2_sftp_last_error(this->sftpSession);
if (sftpErrno == LIBSSH2_FX_FAILURE || sftpErrno == LIBSSH2_FX_PERMISSION_DENIED)
storageInterfacePathRemoveP(this, file, true);
else
{
THROW_FMT(
PathRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE " libssh sftp [%" PRIu64 "]", strZ(file),
sftpErrno);
}
}
else
THROW_FMT(PathRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE, strZ(file));
THROW_FMT(PathRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE " libssh ssh [%d]", strZ(file), rc);
}
// Reset the memory context occasionally so we don't use too much memory or slow down processing
@@ -986,8 +994,10 @@ storageSftpPathRemove(THIS_VOID, const String *const path, const bool recurse, c
if (rc == LIBSSH2_ERROR_SFTP_PROTOCOL)
{
if (libssh2_sftp_last_error(this->sftpSession) != LIBSSH2_FX_NO_SUCH_FILE)
THROW_FMT(PathRemoveError, STORAGE_ERROR_PATH_REMOVE, strZ(path));
const uint64_t sftpErrno = libssh2_sftp_last_error(this->sftpSession);
if (sftpErrno != LIBSSH2_FX_NO_SUCH_FILE)
THROW_FMT(PathRemoveError, STORAGE_ERROR_PATH_REMOVE " sftp error [%" PRIu64 "]", strZ(path), sftpErrno);
// Path does not exist
result = false;

49
test/src/module/storage/sftpTest.c
View File

@@ -4251,6 +4251,18 @@ testRun(void)
{.function = HRNLIBSSH2_SFTP_UNLINK_EX, .param = "[\"" TEST_PATH "/remove1/remove2/remove.txt\"]",
.resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SFTP_LAST_ERROR, .resultUInt = LIBSSH2_FX_PERMISSION_DENIED},
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .param = "[\"" TEST_PATH "/remove1/remove2/remove.txt\",0,0,1]",
.resultNull = true},
{.function = HRNLIBSSH2_SESSION_LAST_ERRNO, .resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SESSION_LAST_ERRNO, .resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SFTP_LAST_ERROR, .resultUInt = LIBSSH2_FX_NO_SUCH_FILE},
{.function = HRNLIBSSH2_SFTP_RMDIR_EX, .param = "[\"" TEST_PATH "/remove1/remove2/remove.txt\"]",
.resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SFTP_LAST_ERROR, .resultUInt = LIBSSH2_FX_NO_SUCH_FILE},
// !!! jrt need to verify that in case where remove.txt is permission denied that this is what happens
{.function = HRNLIBSSH2_SFTP_RMDIR_EX, .param = "[\"" TEST_PATH "/remove1/remove2\"]",
.resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SFTP_LAST_ERROR, .resultUInt = LIBSSH2_FX_PERMISSION_DENIED},
// Path remove - path with subpath and file removed
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .param = "[\"" TEST_PATH "/remove1\",0,0,1]"},
{.function = HRNLIBSSH2_SFTP_READDIR_EX, .param = "[\"\",4095,null,0]", .fileName = STRDEF("remove2"), .resultInt = 7,
@@ -4305,6 +4317,23 @@ testRun(void)
{.function = HRNLIBSSH2_SFTP_CLOSE_HANDLE, .resultInt = LIBSSH2_ERROR_NONE},
{.function = HRNLIBSSH2_SFTP_UNLINK_EX, .param = "[\"" TEST_PATH "/remove1/remove2\"]",
.resultInt = LIBSSH2_ERROR_SOCKET_SEND},
// unlink SFTP failure other than LIBSSH2_FX_FAILURE / LIBSSH2_FX_PERMISSION_DENIED
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .param = "[\"" TEST_PATH "/remove1\",0,0,1]"},
{.function = HRNLIBSSH2_SFTP_READDIR_EX, .param = "[\"\",4095,null,0]", .fileName = STRDEF("remove2"), .resultInt = 7,
.attrPerms = LIBSSH2_SFTP_S_IFDIR | LIBSSH2_SFTP_S_IRWXU | LIBSSH2_SFTP_S_IRWXG| LIBSSH2_SFTP_S_IRWXO,
.flags = LIBSSH2_SFTP_ATTR_PERMISSIONS | LIBSSH2_SFTP_ATTR_ACMODTIME | LIBSSH2_SFTP_ATTR_UIDGID,
.mtime = 1656434296, .uid = TEST_USER_ID, .gid = TEST_GROUP_ID},
{.function = HRNLIBSSH2_SFTP_READDIR_EX, .param = "[\"remove2\",4095,null,0]", .fileName = STRDEF(""),
.resultInt = LIBSSH2_ERROR_NONE,
.attrPerms = LIBSSH2_SFTP_S_IFREG | LIBSSH2_SFTP_S_IRUSR | LIBSSH2_SFTP_S_IWUSR | LIBSSH2_SFTP_S_IRGRP |
LIBSSH2_SFTP_S_IROTH,
.flags = LIBSSH2_SFTP_ATTR_PERMISSIONS | LIBSSH2_SFTP_ATTR_ACMODTIME | LIBSSH2_SFTP_ATTR_UIDGID,
.mtime = 1656434296, .uid = 0, .gid = 0},
{.function = HRNLIBSSH2_SFTP_CLOSE_HANDLE, .resultInt = LIBSSH2_ERROR_NONE},
{.function = HRNLIBSSH2_SFTP_UNLINK_EX, .param = "[\"" TEST_PATH "/remove1/remove2\"]",
.resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SFTP_LAST_ERROR, .resultUInt = LIBSSH2_FX_CONNECTION_LOST},
HRNLIBSSH2_MACRO_SHUTDOWN()
});
@@ -4338,7 +4367,9 @@ testRun(void)
String *pathRemove2 = strNewFmt("%s/remove2", strZ(pathRemove1));
// Mimic creation of pathRemove2 mode 700
TEST_ERROR_FMT(storagePathRemoveP(storageTest, pathRemove2), PathRemoveError, STORAGE_ERROR_PATH_REMOVE, strZ(pathRemove2));
TEST_ERROR_FMT(
storagePathRemoveP(storageTest, pathRemove2), PathRemoveError, STORAGE_ERROR_PATH_REMOVE " sftp error [3]",
strZ(pathRemove2));
TEST_ERROR_FMT(
storagePathRemoveP(storageTest, pathRemove2, .recurse = true), PathOpenError,
STORAGE_ERROR_LIST_INFO ": libssh2 error [-31]: sftp error [3]", strZ(pathRemove2));
@@ -4358,8 +4389,9 @@ testRun(void)
// Mimic "sudo chmod 755 %s && sudo touch %s && sudo chmod 777 %s", strZ(pathRemove2), strZ(fileRemove), strZ(fileRemove));
TEST_ERROR_FMT(
storagePathRemoveP(storageTest, pathRemove1, .recurse = true), PathRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE,
strZ(fileRemove));
storagePathRemoveP(
storageTest, pathRemove1, .recurse = true), PathRemoveError, STORAGE_ERROR_PATH_REMOVE " sftp error [3]",
strZ(pathRemove2));
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("path remove - path with subpath and file removed");
@@ -4371,7 +4403,16 @@ testRun(void)
TEST_TITLE("path remove - path with subpath ssh fail on unlink");
TEST_ERROR_FMT(
storagePathRemoveP(storageTest, pathRemove1, .recurse = true), PathRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE,
storagePathRemoveP(
storageTest, pathRemove1, .recurse = true), PathRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE " libssh ssh [-7]",
strZ(pathRemove2));
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("path remove - other than LIBSSH2_FX_FAILURE/LIBSSH2_FX_PERMISSION_DENIED");
TEST_ERROR_FMT(
storagePathRemoveP(
storageTest, pathRemove1, .recurse = true), PathRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE " libssh sftp [7]",
strZ(pathRemove2));
memContextFree(objMemContext((StorageSftp *)storageDriver(storageTest)));