Skip to content

Commit beb0adc

Browse files
author
Chris Townsend
committed
sftp_client: Fix tests and general cleanup
1 parent 68ef049 commit beb0adc

File tree

5 files changed

+64
-48
lines changed

5 files changed

+64
-48
lines changed

‎src/ssh/sftp_client.cpp‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const std::string stream_file_name{"stream_output.dat"};
4141

4242
using SFTPSessionUPtr = std::unique_ptr<sftp_session_struct, void (*)(sftp_session)>;
4343
using SFTPFileUPtr = std::unique_ptr<sftp_file_struct, int (*)(sftp_file)>;
44+
using SFTPAttrUPtr = std::unique_ptr<sftp_attributes_struct, void (*)(sftp_attributes)>;
4445

4546
SFTPSessionUPtr make_sftp_session(ssh_session session)
4647
{
@@ -86,14 +87,12 @@ void mp::SFTPClient::push_file(const std::string& source_path, const std::string
8687
QFile source(QString::fromStdString(source_path));
8788
const auto size{source.size()};
8889

89-
auto raw_ptr = sftp_open(sftp.get(), full_destination_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC, file_mode);
90-
SFTPFileUPtr file_handle{raw_ptr, sftp_close};
91-
90+
SFTPFileUPtr file_handle{
91+
sftp_open(sftp.get(), full_destination_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC, file_mode), sftp_close};
9292
SSH::throw_on_error(sftp, *ssh_session, "[sftp push] open failed", sftp_get_error);
9393

9494
int total{0};
9595
std::array<char, 65536u> data;
96-
9796
if (!source.open(QIODevice::ReadOnly))
9897
throw std::runtime_error(
9998
fmt::format("[sftp push] error opening file for reading: {}", source.errorString().toStdString()));
@@ -120,7 +119,7 @@ void mp::SFTPClient::pull_file(const std::string& source_path, const std::string
120119
SFTPSessionUPtr sftp{make_sftp_session(*ssh_session)};
121120
SSH::throw_on_error(sftp, *ssh_session, "[sftp pull] init failed", sftp_init);
122121

123-
const auto file_attributes = sftp_stat(sftp.get(), source_path.c_str());
122+
SFTPAttrUPtr file_attributes{sftp_stat(sftp.get(), source_path.c_str()), sftp_attributes_free};
124123
SSH::throw_on_error(sftp, *ssh_session, "[sftp pull] getting stat failed", sftp_get_error);
125124

126125
const auto size{file_attributes->size};
@@ -135,9 +134,7 @@ void mp::SFTPClient::pull_file(const std::string& source_path, const std::string
135134
throw std::runtime_error(
136135
fmt::format("[sftp pull] error opening file for writing: {}", destination.errorString().toStdString()));
137136

138-
auto raw_ptr = sftp_open(sftp.get(), destination_path.c_str(), O_RDONLY, file_mode);
139-
SFTPFileUPtr file_handle{raw_ptr, sftp_close};
140-
137+
SFTPFileUPtr file_handle{sftp_open(sftp.get(), source_path.c_str(), O_RDONLY, file_mode), sftp_close};
141138
SSH::throw_on_error(sftp, *ssh_session, "[sftp pull] open failed", sftp_get_error);
142139

143140
int r;
@@ -163,9 +160,8 @@ void mp::SFTPClient::stream_file(const std::string& destination_path)
163160
SFTPSessionUPtr sftp{make_sftp_session(*ssh_session)};
164161
SSH::throw_on_error(sftp, *ssh_session, "[sftp stream] init session failed", sftp_init);
165162

166-
auto raw_ptr = sftp_open(sftp.get(), full_destination_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC, file_mode);
167-
SFTPFileUPtr file_handle{raw_ptr, sftp_close};
168-
163+
SFTPFileUPtr file_handle{
164+
sftp_open(sftp.get(), full_destination_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC, file_mode), sftp_close};
169165
SSH::throw_on_error(sftp, *ssh_session, "[sftp stream] open failed", sftp_get_error);
170166

171167
QTextStream in_stream(stdin);

‎tests/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ target_include_directories(multipass_tests
120120
PRIVATE ${MULTIPASS_GMOCK_DIR}/include
121121
)
122122

123+
add_definitions(-DWITH_SERVER)
123124
target_compile_definitions(ssh_test PRIVATE
124125
${c_mock_defines})
125126
target_compile_definitions(sshfs_mount_test PRIVATE

‎tests/c_mock_defines.cmake‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright © 2018 Canonical Ltd.
1+
# Copyright © 2018-2019 Canonical Ltd.
22
#
33
# This program is free software: you can redistribute it and/or modify
44
# it under the terms of the GNU General Public License version 3 as

‎tests/mock_sftpserver.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2018 Canonical, Ltd.
2+
* Copyright (C) 2018-2019 Canonical, Ltd.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License as published by
@@ -20,7 +20,6 @@
2020

2121
#include <premock.hpp>
2222

23-
#define WITH_SERVER
2423
#include <libssh/sftp.h>
2524

2625
DECL_MOCK(sftp_server_new);

‎tests/test_sftp_client.cpp‎

Lines changed: 54 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,29 @@
2727

2828
#include <gmock/gmock.h>
2929

30+
#include <fmt/format.h>
3031
namespace mp = multipass;
3132
namespace mpt = multipass::test;
3233

3334
namespace
3435
{
36+
sftp_file get_dummy_sftp_file()
37+
{
38+
return static_cast<sftp_file_struct*>(calloc(1, sizeof(struct sftp_file_struct)));
39+
}
40+
41+
sftp_attributes get_dummy_sftp_attributes()
42+
{
43+
return static_cast<sftp_attributes_struct*>(calloc(1, sizeof(struct sftp_attributes_struct)));
44+
}
45+
3546
struct SFTPClient : public testing::Test
3647
{
3748
SFTPClient()
3849
: sftp_new{mock_sftp_new,
3950
[](ssh_session session) -> sftp_session {
40-
sftp_session sftp;
41-
sftp = (sftp_session)std::calloc(1, sizeof(struct sftp_session_struct));
51+
sftp_session sftp =
52+
static_cast<sftp_session_struct*>(std::calloc(1, sizeof(struct sftp_session_struct)));
4253
return sftp;
4354
}},
4455
free_sftp{mock_sftp_free, [](sftp_session sftp) { std::free(sftp); }}
@@ -80,7 +91,6 @@ TEST_F(SFTPClient, push_throws_when_failed_to_init)
8091
{
8192
auto sftp = make_sftp_client();
8293

83-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
8494
REPLACE(sftp_init, [](auto...) { return SSH_ERROR; });
8595

8696
EXPECT_THROW(sftp.push_file("foo", "bar"), std::runtime_error);
@@ -90,21 +100,21 @@ TEST_F(SFTPClient, push_throws_on_sftp_open_failed)
90100
{
91101
auto sftp = make_sftp_client();
92102

93-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
94103
REPLACE(sftp_init, [](auto...) { return SSH_OK; });
95-
REPLACE(sftp_open, [](auto...) { return nullptr; });
104+
REPLACE(sftp_open, [](sftp_session session, auto...) {
105+
session->errnum = SSH_ERROR;
106+
return nullptr;
107+
});
96108

97109
EXPECT_THROW(sftp.push_file("foo", "bar"), std::runtime_error);
98110
}
99111

100-
TEST_F(SFTPClient, push_throws_on_sftp_read_failed)
112+
TEST_F(SFTPClient, push_throws_on_invalid_source)
101113
{
102114
auto sftp = make_sftp_client();
103115

104-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
105116
REPLACE(sftp_init, [](auto...) { return SSH_OK; });
106-
REPLACE(sftp_open, [](auto...) { return nullptr; });
107-
REPLACE(sftp_get_error, [](auto...) { return SSH_OK; });
117+
REPLACE(sftp_open, [](auto...) { return get_dummy_sftp_file(); });
108118

109119
EXPECT_THROW(sftp.push_file("foo", "bar"), std::runtime_error);
110120
}
@@ -117,13 +127,15 @@ TEST_F(SFTPClient, push_throws_on_sftp_write_error)
117127

118128
auto sftp = make_sftp_client();
119129

120-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
121130
REPLACE(sftp_init, [](auto...) { return SSH_OK; });
122-
REPLACE(sftp_open, [](auto...) { return nullptr; });
123-
REPLACE(sftp_write, [](auto...) { return 0; });
124-
REPLACE(sftp_get_error, [](auto...) {
125-
static int errorCount = 0;
126-
return (errorCount-- < 0) ? SSH_ERROR : SSH_OK;
131+
REPLACE(sftp_open, [](sftp_session session, auto...) {
132+
sftp_file file = get_dummy_sftp_file();
133+
file->sftp = session;
134+
return file;
135+
});
136+
REPLACE(sftp_write, [](sftp_file file, auto...) {
137+
file->sftp->errnum = SSH_ERROR;
138+
return -1;
127139
});
128140

129141
EXPECT_THROW(sftp.push_file(file_name.toStdString(), "bar"), std::runtime_error);
@@ -135,7 +147,6 @@ TEST_F(SFTPClient, pull_throws_when_failed_to_init)
135147
{
136148
auto sftp = make_sftp_client();
137149

138-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
139150
REPLACE(sftp_init, [](auto...) { return SSH_ERROR; });
140151

141152
EXPECT_THROW(sftp.pull_file("foo", "bar"), std::runtime_error);
@@ -145,24 +156,29 @@ TEST_F(SFTPClient, pull_throws_on_sftp_get_stat_failed)
145156
{
146157
auto sftp = make_sftp_client();
147158

148-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
149159
REPLACE(sftp_init, [](auto...) { return SSH_OK; });
150-
REPLACE(sftp_stat, [](auto...) { return nullptr; });
160+
REPLACE(sftp_stat, [](sftp_session session, auto...) {
161+
session->errnum = SSH_ERROR;
162+
return nullptr;
163+
});
151164

152165
EXPECT_THROW(sftp.pull_file("foo", "bar"), std::runtime_error);
153166
}
154167

155168
TEST_F(SFTPClient, pull_throws_on_sftp_open_failed)
156169
{
157170
auto sftp = make_sftp_client();
171+
const std::string source_path{"foo"};
158172

159-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
160173
REPLACE(sftp_init, [](auto...) { return SSH_OK; });
161-
REPLACE(sftp_stat, [](auto...) { return new sftp_attributes_struct; });
162-
REPLACE(sftp_open, [](auto...) { return nullptr; });
163-
REPLACE(sftp_get_error, [](auto...) {
164-
static int errorCount = 0;
165-
return (errorCount-- < 0) ? SSH_ERROR : SSH_OK;
174+
REPLACE(sftp_stat, [&source_path](auto...) {
175+
auto attributes = get_dummy_sftp_attributes();
176+
attributes->name = strdup(source_path.c_str());
177+
return attributes;
178+
});
179+
REPLACE(sftp_open, [](sftp_session session, auto...) {
180+
session->errnum = SSH_ERROR;
181+
return nullptr;
166182
});
167183

168184
EXPECT_THROW(sftp.pull_file("foo", "bar"), std::runtime_error);
@@ -171,15 +187,22 @@ TEST_F(SFTPClient, pull_throws_on_sftp_open_failed)
171187
TEST_F(SFTPClient, pull_throws_on_sftp_read_failed)
172188
{
173189
auto sftp = make_sftp_client();
190+
const std::string source_path{"foo"};
174191

175-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
176192
REPLACE(sftp_init, [](auto...) { return SSH_OK; });
177-
REPLACE(sftp_stat, [](auto...) { return new sftp_attributes_struct; });
178-
REPLACE(sftp_open, [](auto...) { return new sftp_file_struct; });
179-
REPLACE(sftp_read, [](auto...) { return 0; });
180-
REPLACE(sftp_get_error, [](auto...) {
181-
static int errorCount = 1;
182-
return (errorCount-- < 0) ? SSH_ERROR : SSH_OK;
193+
REPLACE(sftp_stat, [&source_path](auto...) {
194+
auto attributes = get_dummy_sftp_attributes();
195+
attributes->name = strdup(source_path.c_str());
196+
return attributes;
197+
});
198+
REPLACE(sftp_open, [](sftp_session session, auto...) {
199+
auto file = get_dummy_sftp_file();
200+
file->sftp = session;
201+
return file;
202+
});
203+
REPLACE(sftp_read, [](sftp_file file, auto...) {
204+
file->sftp->errnum = SSH_ERROR;
205+
return -1;
183206
});
184207

185208
EXPECT_THROW(sftp.pull_file("foo", "bar"), std::runtime_error);
@@ -191,7 +214,6 @@ TEST_F(SFTPClient, stream_throws_when_failed_to_init)
191214
{
192215
auto sftp = make_sftp_client();
193216

194-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
195217
REPLACE(sftp_init, [](auto...) { return SSH_ERROR; });
196218

197219
EXPECT_THROW(sftp.stream_file("bar"), std::runtime_error);
@@ -201,7 +223,6 @@ TEST_F(SFTPClient, steam_throws_on_sftp_open_failed)
201223
{
202224
auto sftp = make_sftp_client();
203225

204-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
205226
REPLACE(sftp_init, [](auto...) { return SSH_OK; });
206227
REPLACE(sftp_open, [](auto...) { return nullptr; });
207228
REPLACE(sftp_get_error, [](auto...) { return SSH_ERROR; });
@@ -213,7 +234,6 @@ TEST_F(SFTPClient, stream_throws_on_write_failed)
213234
{
214235
auto sftp = make_sftp_client();
215236

216-
REPLACE(sftp_new, [](auto...) { return new sftp_session_struct; });
217237
REPLACE(sftp_init, [](auto...) { return SSH_OK; });
218238
REPLACE(sftp_open, [](auto...) { return nullptr; });
219239
REPLACE(sftp_write, [](auto...) { return 0; });

0 commit comments

Comments
 (0)