From 540a30257c51ae5f53f73d47766308296848305e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
 <sebastien.blin@savoirfairelinux.com>
Date: Wed, 3 Apr 2019 10:38:19 -0400
Subject: [PATCH] ftp_server: always get data_transfer id

Currently, when an incoming request is cancelled the transfer id
is lost. This patch avoid this case and allow the cancel part in
p2p.cpp to clear the data related to this transfer.

Change-Id: Ia5b69b514fce19e80ee3344d40eb395470212ef3
---
 src/data_transfer.cpp | 20 ++++++++++----------
 src/data_transfer.h   |  7 ++++++-
 src/ftp_server.cpp    | 26 ++++++++++++--------------
 src/ftp_server.h      |  3 ++-
 4 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/src/data_transfer.cpp b/src/data_transfer.cpp
index 566752844..924f1814b 100644
--- a/src/data_transfer.cpp
+++ b/src/data_transfer.cpp
@@ -598,7 +598,6 @@ void
 DataTransferFacade::Impl::cancel(DataTransfer& transfer)
 {
     transfer.close();
-    map_.erase(transfer.getId());
 }
 
 std::shared_ptr<DataTransfer>
@@ -773,15 +772,16 @@ DataTransferFacade::info(const DRing::DataTransferId& id,
     return DRing::DataTransferError::unknown;
 }
 
-std::shared_ptr<Stream>
-DataTransferFacade::onIncomingFileRequest(const DRing::DataTransferInfo& info)
-{
-    auto transfer = pimpl_->createIncomingFileTransfer(info);
-    auto filename = transfer->requestFilename();
-    if (!filename.empty())
-        if (transfer->start())
-            return std::static_pointer_cast<Stream>(transfer);
-    return {};
+IncomingFileInfo
+DataTransferFacade::onIncomingFileRequest(const DRing::DataTransferInfo &info) {
+  auto transfer = pimpl_->createIncomingFileTransfer(info);
+  if (!transfer)
+      return {};
+  auto filename = transfer->requestFilename();
+  if (!filename.empty())
+    if (transfer->start())
+      return {transfer->getId(), std::static_pointer_cast<Stream>(transfer)};
+  return {transfer->getId(), nullptr};
 }
 
 } // namespace jami
diff --git a/src/data_transfer.h b/src/data_transfer.h
index dfe8c2b9d..215ca5330 100644
--- a/src/data_transfer.h
+++ b/src/data_transfer.h
@@ -29,6 +29,11 @@ namespace jami {
 
 class Stream;
 
+struct IncomingFileInfo {
+  DRing::DataTransferId id;
+  std::shared_ptr<Stream> stream;
+};
+
 /// Front-end to data transfer service
 class DataTransferFacade
 {
@@ -61,7 +66,7 @@ public:
 
     /// Create an IncomingFileTransfer object.
     /// \return a shared pointer on created Stream object, or nullptr in case of error
-    std::shared_ptr<Stream> onIncomingFileRequest(const DRing::DataTransferInfo& info);
+    IncomingFileInfo onIncomingFileRequest(const DRing::DataTransferInfo &info);
 
 private:
     class Impl;
diff --git a/src/ftp_server.cpp b/src/ftp_server.cpp
index 1de9f40f5..aac997f3c 100644
--- a/src/ftp_server.cpp
+++ b/src/ftp_server.cpp
@@ -22,7 +22,6 @@
 
 #include "logger.h"
 #include "string_utils.h"
-#include "data_transfer.h"
 #include "manager.h"
 
 #include <algorithm>
@@ -46,9 +45,8 @@ DRing::DataTransferId
 FtpServer::getId() const
 {
     // Because FtpServer is just the protocol on the top of a stream so the id
-    // of the stream is the id of out_. Because the output stream can be
-    // uninitialized, a FtpServer without a stream can have an id equal to 0.
-    return out_? out_->getId() : 0;
+    // of the stream is the id of out_.
+    return out_.id;
 }
 
 void
@@ -70,21 +68,21 @@ FtpServer::startNewFile()
     info.bytesProgress = 0;
     rx_ = 0;
     out_ = Manager::instance().dataTransfers->onIncomingFileRequest(info); // we block here until answer from client
-    if (!out_) {
+    if (!out_.stream) {
         JAMI_DBG() << "[FTP] transfer aborted by client";
         closed_ = true; // send NOK msg at next read()
     } else {
         go_ = true;
     }
-    return bool(out_);
+    return bool(out_.stream);
 }
 
 void
 FtpServer::closeCurrentFile()
 {
-    if (out_) {
-        out_->close();
-        out_.reset();
+    if (out_.stream) {
+        out_.stream->close();
+        out_.stream.reset();
         closed_ = true;
     }
 }
@@ -92,7 +90,7 @@ FtpServer::closeCurrentFile()
 bool
 FtpServer::read(std::vector<uint8_t>& buffer) const
 {
-    if (!out_) {
+    if (!out_.stream) {
         if (closed_) {
             closed_ = false;
             if (rx_ < fileSize_) {
@@ -134,8 +132,8 @@ FtpServer::write(const std::vector<uint8_t>& buffer)
                         break;
                     auto size_needed = fileSize_ - rx_;
                     count = std::min(count, size_needed);
-                    if (out_)
-                        out_->write(reinterpret_cast<const uint8_t*>(&line_[0]), count);
+                    if (out_.stream)
+                        out_.stream->write(reinterpret_cast<const uint8_t*>(&line_[0]), count);
                     rx_ += count;
                     if (rx_ == fileSize_) {
                         closeCurrentFile();
@@ -150,8 +148,8 @@ FtpServer::write(const std::vector<uint8_t>& buffer)
 
         case FtpState::READ_DATA:
         {
-            if (out_)
-                out_->write(&buffer[0], buffer.size());
+            if (out_.stream)
+                out_.stream->write(&buffer[0], buffer.size());
             auto size_needed = fileSize_ - rx_;
             auto read_size = std::min(buffer.size(), size_needed);
             rx_ += read_size;
diff --git a/src/ftp_server.h b/src/ftp_server.h
index 1225f0606..ccce004fb 100644
--- a/src/ftp_server.h
+++ b/src/ftp_server.h
@@ -20,6 +20,7 @@
 
 #pragma once
 
+#include "data_transfer.h"
 #include "peer_connection.h"
 
 #include <vector>
@@ -53,7 +54,7 @@ private:
 
     const std::string accountId_;
     const std::string peerUri_;
-    std::shared_ptr<Stream> out_;
+    IncomingFileInfo out_ {0, nullptr};
     std::size_t fileSize_ {0};
     std::size_t rx_ {0};
     std::stringstream headerStream_;
-- 
2.22.0