From 6030be0dfcac9b44dcc9410970f2057427e85ea9 Mon Sep 17 00:00:00 2001
From: Adrien Beraud <adrien.beraud@savoirfairelinux.com>
Date: Wed, 5 Jun 2024 15:54:39 -0400
Subject: [PATCH] data transfer: download to temporary file, rename on
completion
GitLab: #972
Change-Id: Ic3f03fdc8fc28b387add46d1a59bf0bc15418595
---
src/data_transfer.cpp | 44 +++++++++++++++---------
src/data_transfer.h | 7 ++--
src/jamidht/jamiaccount.cpp | 6 ++--
src/jamidht/transfer_channel_handler.cpp | 7 ++--
4 files changed, 38 insertions(+), 26 deletions(-)
diff --git a/src/data_transfer.cpp b/src/data_transfer.cpp
index 638d542cd1..e579b96fb9 100644
--- a/src/data_transfer.cpp
+++ b/src/data_transfer.cpp
@@ -154,8 +154,9 @@ IncomingFile::IncomingFile(const std::shared_ptr<dhtnet::ChannelSocket>& channel
const std::string& sha3Sum)
: FileInfo(channel, fileId, interactionId, info)
, sha3Sum_(sha3Sum)
+ , path_(info.path + ".tmp")
{
- stream_.open(std::filesystem::path(info_.path),
+ stream_.open(path_,
std::ios::binary | std::ios::out | std::ios::app);
if (!stream_)
return;
@@ -188,7 +189,7 @@ IncomingFile::cancel()
void
IncomingFile::process()
{
- channel_->setOnRecv([w = weak()](const uint8_t* buf, size_t len) {
+ channel_->setOnRecv([w = weak_from_this()](const uint8_t* buf, size_t len) {
if (auto shared = w.lock()) {
// No need to lock, setOnRecv is resetted before closing
if (shared->stream_.is_open())
@@ -197,7 +198,7 @@ IncomingFile::process()
}
return len;
});
- channel_->onShutdown([w = weak()] {
+ channel_->onShutdown([w = weak_from_this()] {
auto shared = w.lock();
if (!shared)
return;
@@ -207,24 +208,35 @@ IncomingFile::process()
shared->stream_.close();
}
auto correct = shared->sha3Sum_.empty();
+ std::error_code ec;
if (!correct) {
- // Verify shaSum
- auto sha3Sum = fileutils::sha3File(shared->info_.path);
if (shared->isUserCancelled_) {
- JAMI_WARN() << "Remove file, invalid sha3sum detected for " << shared->info_.path;
- dhtnet::fileutils::remove(shared->info_.path, true);
- } else if (shared->sha3Sum_ == sha3Sum) {
- JAMI_INFO() << "New file received: " << shared->info_.path;
- correct = true;
+ std::filesystem::remove(shared->path_, ec);
} else {
- JAMI_WARN() << "Invalid sha3sum detected, unfinished file: " << shared->info_.path;
- if (shared->info_.totalSize != 0
- && shared->info_.totalSize < shared->info_.bytesProgress) {
- JAMI_WARN() << "Remove file, larger file than announced for "
- << shared->info_.path;
- dhtnet::fileutils::remove(shared->info_.path, true);
+ auto sha3Sum = fileutils::sha3File(shared->path_);
+ if (shared->sha3Sum_ == sha3Sum) {
+ JAMI_LOG("New file received: {}", shared->info_.path);
+ correct = true;
+ } else {
+ if (shared->info_.totalSize != 0
+ && shared->info_.totalSize < shared->info_.bytesProgress) {
+ JAMI_WARNING("Removing {} larger than announced: {}/{}", shared->info_.path, shared->info_.bytesProgress, shared->info_.totalSize);
+ std::filesystem::remove(shared->path_, ec);
+ } else {
+ JAMI_WARNING("Invalid sha3sum detected for {}, incomplete file: {}/{}", shared->info_.path, shared->info_.bytesProgress, shared->info_.totalSize);
+ }
}
}
+ if (ec) {
+ JAMI_ERROR("Failed to remove file {}: {}", shared->path_, ec.message());
+ }
+ }
+ if (correct) {
+ std::filesystem::rename(shared->path_, shared->info_.path, ec);
+ if (ec) {
+ JAMI_ERROR("Failed to rename file from {} to {}: {}", shared->path_, shared->info_.path, ec.message());
+ correct = false;
+ }
}
if (shared->isUserCancelled_)
return;
diff --git a/src/data_transfer.h b/src/data_transfer.h
index fb6e9191a3..9d3a312cc8 100644
--- a/src/data_transfer.h
+++ b/src/data_transfer.h
@@ -87,19 +87,16 @@ public:
const libjami::DataTransferInfo& info,
const std::string& fileId,
const std::string& interactionId,
- const std::string& sha3Sum = "");
+ const std::string& sha3Sum);
~IncomingFile();
void process() override;
void cancel() override;
private:
- std::weak_ptr<IncomingFile> weak()
- {
- return std::static_pointer_cast<IncomingFile>(shared_from_this());
- }
std::mutex streamMtx_;
std::ofstream stream_;
std::string sha3Sum_ {};
+ std::filesystem::path path_;
};
class OutgoingFile : public FileInfo
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 90dfd41afc..1bf50809ae 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -4088,21 +4088,21 @@ JamiAccount::askForFileChannel(const std::string& conversationId,
currentDeviceId(),
fileId);
if (start != 0 || end != 0) {
- channelName += "?start=" + std::to_string(start) + "&end=" + std::to_string(end);
+ channelName += fmt::format("?start={}&end={}", start, end);
}
// We can avoid to negotiate new sessions, as the file notif
// probably come from an online device or last connected device.
connectionManager_->connectDevice(
did,
channelName,
- [this,
+ [w = weak(),
conversationId,
fileId,
interactionId](std::shared_ptr<dhtnet::ChannelSocket> channel, const DeviceId&) {
if (!channel)
return;
dht::ThreadPool::io().run(
- [w = weak(), conversationId, channel, fileId, interactionId] {
+ [w, conversationId, channel, fileId, interactionId] {
auto shared = w.lock();
if (!shared)
return;
diff --git a/src/jamidht/transfer_channel_handler.cpp b/src/jamidht/transfer_channel_handler.cpp
index 187cccfd96..8eacf2882b 100644
--- a/src/jamidht/transfer_channel_handler.cpp
+++ b/src/jamidht/transfer_channel_handler.cpp
@@ -122,8 +122,11 @@ TransferChannelHandler::onReady(const std::shared_ptr<dht::crypto::Certificate>&
sha3Sum = keyVal[1];
} else if (keyVal[0] == "modified") {
try {
- lastModified = std::stoull(std::string(keyVal[1]));
- } catch (...) {}
+ lastModified = jami::to_int<uint64_t>(keyVal[1]);
+ } catch (const std::exception& e) {
+ JAMI_WARNING("TransferChannel: can't parse modified date: {}: {}",
+ keyVal[1], e.what());
+ }
}
}
}
--
GitLab