Save uploaded files in multipart request to disk
Instead of keeping them in memory. Pass the file descriptor/file stream
to the handler process when asking for the uploaded file data.
BUG: 24166746
Change-Id: Ife4f5b4fa422d99272b15f6f02e4b3b515e4e3b4
diff --git a/Android.mk b/Android.mk
index 22d68c4..bc8c554 100644
--- a/Android.mk
+++ b/Android.mk
@@ -41,6 +41,7 @@
libchromeos \
libchromeos-dbus \
libchromeos-http \
+ libchromeos-stream \
libdbus \
libmicrohttpd \
diff --git a/libwebserv/protocol_handler.cc b/libwebserv/protocol_handler.cc
index e7e8dc8..544281f 100644
--- a/libwebserv/protocol_handler.cc
+++ b/libwebserv/protocol_handler.cc
@@ -18,6 +18,7 @@
#include <base/logging.h>
#include <chromeos/map_utils.h>
+#include <chromeos/streams/file_stream.h>
#include "dbus_bindings/org.chromium.WebServer.RequestHandler.h"
#include "libwebserv/request.h"
@@ -226,13 +227,41 @@
void ProtocolHandler::GetFileData(
const std::string& request_id,
int file_id,
- const base::Callback<void(const std::vector<uint8_t>&)>& success_callback,
+ const base::Callback<void(chromeos::StreamPtr)>& success_callback,
const base::Callback<void(chromeos::Error*)>& error_callback) {
ProtocolHandlerProxy* proxy = GetRequestProtocolHandlerProxy(request_id);
CHECK(proxy);
- proxy->GetRequestFileDataAsync(
- request_id, file_id, success_callback, error_callback);
+ // Store the success/error callback in a shared object so it can be referenced
+ // by the two wrapper callbacks. Since the original callbacks MAY contain
+ // move-only types, copying the base::Callback object is generally unsafe and
+ // may destroy the source object of the copy (despite the fact that it is
+ // constant). So, here we move both callbacks to |Callbacks| structure and
+ // use a shared pointer to it in both success and error callback wrappers.
+ struct Callbacks {
+ base::Callback<void(chromeos::StreamPtr)> on_success;
+ base::Callback<void(chromeos::Error*)> on_error;
+ };
+ auto callbacks = std::make_shared<Callbacks>();
+ callbacks->on_success = success_callback;
+ callbacks->on_error = error_callback;
+
+ auto on_success = [callbacks](const dbus::FileDescriptor& fd) {
+ chromeos::ErrorPtr error;
+ // Unfortunately there is no way to take ownership of the file descriptor
+ // since |fd| is a const reference, so duplicate the descriptor.
+ int dupfd = dup(fd.value());
+ auto stream = chromeos::FileStream::FromFileDescriptor(dupfd, true, &error);
+ if (!stream)
+ return callbacks->on_error.Run(error.get());
+ callbacks->on_success.Run(std::move(stream));
+ };
+ auto on_error = [callbacks](chromeos::Error* error) {
+ callbacks->on_error.Run(error);
+ };
+
+ proxy->GetRequestFileDataAsync(request_id, file_id, base::Bind(on_success),
+ base::Bind(on_error));
}
ProtocolHandler::ProtocolHandlerProxy*
diff --git a/libwebserv/protocol_handler.h b/libwebserv/protocol_handler.h
index 648f0ff..91276d9 100644
--- a/libwebserv/protocol_handler.h
+++ b/libwebserv/protocol_handler.h
@@ -26,6 +26,7 @@
#include <base/memory/weak_ptr.h>
#include <chromeos/errors/error.h>
#include <chromeos/secure_blob.h>
+#include <chromeos/streams/stream.h>
#include <dbus/object_path.h>
#include <libwebserv/export.h>
@@ -174,7 +175,7 @@
LIBWEBSERV_PRIVATE void GetFileData(
const std::string& request_id,
int file_id,
- const base::Callback<void(const std::vector<uint8_t>&)>& success_callback,
+ const base::Callback<void(chromeos::StreamPtr)>& success_callback,
const base::Callback<void(chromeos::Error*)>& error_callback);
// A helper method to obtain a corresponding protocol handler D-Bus proxy for
diff --git a/libwebserv/request.cc b/libwebserv/request.cc
index cb0fa1b..7b518a4 100644
--- a/libwebserv/request.cc
+++ b/libwebserv/request.cc
@@ -36,8 +36,8 @@
}
void FileInfo::GetData(
- const base::Callback<void(const std::vector<uint8_t>&)>& success_callback,
- const base::Callback<void(chromeos::Error*)>& error_callback) {
+ const base::Callback<void(chromeos::StreamPtr)>& success_callback,
+ const base::Callback<void(chromeos::Error*)>& error_callback) const {
handler_->GetFileData(request_id_,
file_id_,
success_callback,
diff --git a/libwebserv/request.h b/libwebserv/request.h
index 4ffbb7c..161dfa8 100644
--- a/libwebserv/request.h
+++ b/libwebserv/request.h
@@ -25,6 +25,7 @@
#include <base/macros.h>
#include <base/memory/ref_counted.h>
#include <chromeos/errors/error.h>
+#include <chromeos/streams/stream.h>
#include <libwebserv/export.h>
struct MHD_Connection;
@@ -43,8 +44,8 @@
const std::string& GetContentType() const { return content_type_; }
const std::string& GetTransferEncoding() const { return transfer_encoding_; }
void GetData(
- const base::Callback<void(const std::vector<uint8_t>&)>& success_callback,
- const base::Callback<void(chromeos::Error*)>& error_callback);
+ const base::Callback<void(chromeos::StreamPtr)>& success_callback,
+ const base::Callback<void(chromeos::Error*)>& error_callback) const;
private:
friend class Server;
@@ -62,7 +63,6 @@
std::string file_name_;
std::string content_type_;
std::string transfer_encoding_;
- std::vector<uint8_t> data_;
DISALLOW_COPY_AND_ASSIGN(FileInfo);
};
diff --git a/webservd/Android.mk b/webservd/Android.mk
index 113354d..d7b7051 100644
--- a/webservd/Android.mk
+++ b/webservd/Android.mk
@@ -44,6 +44,7 @@
protocol_handler.cc \
request.cc \
server.cc \
+ temp_file_manager.cc \
utils.cc \
LOCAL_INIT_RC := init.webservd.rc
diff --git a/webservd/dbus_bindings/org.chromium.WebServer.ProtocolHandler.dbus-xml b/webservd/dbus_bindings/org.chromium.WebServer.ProtocolHandler.dbus-xml
index 3f84654..e6aa434 100644
--- a/webservd/dbus_bindings/org.chromium.WebServer.ProtocolHandler.dbus-xml
+++ b/webservd/dbus_bindings/org.chromium.WebServer.ProtocolHandler.dbus-xml
@@ -30,7 +30,7 @@
</tp:docstring>
<arg name="request_id" type="s" direction="in"/>
<arg name="file_id" type="i" direction="in"/>
- <arg name="contents" type="ay" direction="out"/>
+ <arg name="contents" type="h" direction="out"/>
<annotation name="org.chromium.DBus.Method.Kind" value="normal"/>
</method>
<method name="CompleteRequest">
diff --git a/webservd/dbus_protocol_handler.cc b/webservd/dbus_protocol_handler.cc
index a2d0d86..acb91a9 100644
--- a/webservd/dbus_protocol_handler.cc
+++ b/webservd/dbus_protocol_handler.cc
@@ -146,13 +146,17 @@
chromeos::ErrorPtr* error,
const std::string& in_request_id,
int32_t in_file_id,
- std::vector<uint8_t>* out_contents) {
+ dbus::FileDescriptor* out_contents) {
auto request = GetRequest(in_request_id, error);
if (!request)
return false;
- if (request->GetFileData(in_file_id, out_contents))
+ int data_fd = 0;
+ if (request->GetFileData(in_file_id, &data_fd)) {
+ out_contents->PutValue(data_fd);
+ out_contents->CheckValidity();
return true;
+ }
chromeos::Error::AddToPrintf(error,
FROM_HERE,
diff --git a/webservd/dbus_protocol_handler.h b/webservd/dbus_protocol_handler.h
index 20bf72f..5fbfeeb 100644
--- a/webservd/dbus_protocol_handler.h
+++ b/webservd/dbus_protocol_handler.h
@@ -71,7 +71,7 @@
bool GetRequestFileData(chromeos::ErrorPtr* error,
const std::string& in_request_id,
int32_t in_file_id,
- std::vector<uint8_t>* out_contents) override;
+ dbus::FileDescriptor* out_contents) override;
bool CompleteRequest(
chromeos::ErrorPtr* error,
diff --git a/webservd/init.webservd.rc b/webservd/init.webservd.rc
index 8e28f7b..450a2ca 100644
--- a/webservd/init.webservd.rc
+++ b/webservd/init.webservd.rc
@@ -1,6 +1,7 @@
on boot
mkdir /data/misc/webservd 0700 system system
mkdir /data/misc/webservd/logs 0700 system system
+ mkdir /data/misc/webservd/uploads 0700 system system
service webservd /system/bin/webservd
class late_start
diff --git a/webservd/request.cc b/webservd/request.cc
index 14b5793..f149b4b 100644
--- a/webservd/request.cc
+++ b/webservd/request.cc
@@ -17,14 +17,18 @@
#include <microhttpd.h>
#include <base/bind.h>
+#include <base/files/file.h>
#include <base/guid.h>
#include <chromeos/http/http_request.h>
#include <chromeos/http/http_utils.h>
#include <chromeos/mime_utils.h>
+#include <chromeos/streams/file_stream.h>
#include <chromeos/strings/string_utils.h>
#include "webservd/log_manager.h"
#include "webservd/protocol_handler.h"
#include "webservd/request_handler_interface.h"
+#include "webservd/server_interface.h"
+#include "webservd/temp_file_manager.h"
namespace webservd {
@@ -99,13 +103,18 @@
Request::~Request() {
if (post_processor_)
MHD_destroy_post_processor(post_processor_);
+ GetTempFileManager()->DeleteRequestTempFiles(id_);
protocol_handler_->RemoveRequest(this);
}
-bool Request::GetFileData(int file_id, std::vector<uint8_t>* contents) {
+bool Request::GetFileData(int file_id, int* contents_fd) {
if (file_id < 0 || static_cast<size_t>(file_id) >= file_info_.size())
return false;
- *contents = file_info_[file_id]->data;
+ base::File file(file_info_[file_id]->temp_file_name,
+ base::File::FLAG_OPEN | base::File::FLAG_READ);
+ if (!file.IsValid())
+ return false;
+ *contents_fd = file.TakePlatformFile();
return true;
}
@@ -172,6 +181,10 @@
if (state_ == State::kIdle) {
state_ = State::kWaitingForResponse;
if (!request_handler_id_.empty()) {
+ // Close all temporary file streams, if any.
+ for (auto& file : file_info_)
+ file->data_stream->CloseBlocking(nullptr);
+
protocol_handler_->AddRequest(this);
auto p = protocol_handler_->request_handlers_.find(request_handler_id_);
CHECK(p != protocol_handler_->request_handlers_.end());
@@ -228,7 +241,14 @@
std::unique_ptr<FileInfo> file_info{
new FileInfo{key, filename, content_type ? content_type : "",
transfer_encoding ? transfer_encoding : ""}};
- file_info->data.assign(data, data + size);
+ file_info->temp_file_name = GetTempFileManager()->CreateTempFileName(id_);
+ file_info->data_stream = chromeos::FileStream::Open(
+ file_info->temp_file_name, chromeos::Stream::AccessMode::READ_WRITE,
+ chromeos::FileStream::Disposition::CREATE_ALWAYS, nullptr);
+ if (!file_info->data_stream ||
+ !file_info->data_stream->WriteAllBlocking(data, size, nullptr)) {
+ return false;
+ }
file_info_.push_back(std::move(file_info));
last_posted_data_was_file_ = true;
return true;
@@ -246,8 +266,7 @@
CHECK(!file_info_.empty());
CHECK(file_info_.back()->field_name == key);
FileInfo* file_info = file_info_.back().get();
- file_info->data.insert(file_info->data.end(), data, data + size);
- return true;
+ return file_info->data_stream->WriteAllBlocking(data, size, nullptr);
}
CHECK(!post_data_.empty());
@@ -256,4 +275,9 @@
return true;
}
+TempFileManager* Request::GetTempFileManager() {
+ return protocol_handler_->GetServer()->GetTempFileManager();
+}
+
+
} // namespace webservd
diff --git a/webservd/request.h b/webservd/request.h
index 179c3e6..c09b6e5 100644
--- a/webservd/request.h
+++ b/webservd/request.h
@@ -22,6 +22,8 @@
#include <vector>
#include <base/macros.h>
+#include <base/files/file_path.h>
+#include <chromeos/streams/stream.h>
struct MHD_Connection;
struct MHD_PostProcessor;
@@ -29,6 +31,7 @@
namespace webservd {
class ProtocolHandler;
+class TempFileManager;
using PairOfStrings = std::pair<std::string, std::string>;
@@ -51,7 +54,9 @@
// was specified.
std::string transfer_encoding;
// The file content data.
- std::vector<uint8_t> data;
+ chromeos::StreamPtr data_stream;
+ // The temporary file containing the file part data.
+ base::FilePath temp_file_name;
private:
DISALLOW_COPY_AND_ASSIGN(FileInfo);
@@ -68,8 +73,9 @@
ProtocolHandler* protocol_handler);
~Request();
- // Obtains the content data of uploaded file identified by |file_id|.
- bool GetFileData(int file_id, std::vector<uint8_t>* contents);
+ // Obtains the file descriptor containing data of uploaded file identified
+ // by |file_id|.
+ bool GetFileData(int file_id, int* contents_fd);
// Finishes the request and provides the reply data.
bool Complete(
@@ -153,6 +159,8 @@
size_t size);
bool AppendPostFieldData(const char* key, const char* data, size_t size);
+ TempFileManager* GetTempFileManager();
+
std::string id_;
std::string request_handler_id_;
std::string url_;
diff --git a/webservd/server.cc b/webservd/server.cc
index 17935bf..2119402 100644
--- a/webservd/server.cc
+++ b/webservd/server.cc
@@ -223,4 +223,14 @@
}
}
+base::FilePath Server::GetUploadDirectory() const {
+ base::FilePath upload_dir;
+#ifdef __ANDROID__
+ upload_dir = base::FilePath{"/data/misc/webservd/uploads"};
+#else
+ CHECK(base::GetTempDir(&upload_dir));
+#endif
+ return upload_dir;
+}
+
} // namespace webservd
diff --git a/webservd/server.h b/webservd/server.h
index 3964a11..e7873c1 100644
--- a/webservd/server.h
+++ b/webservd/server.h
@@ -30,6 +30,7 @@
#include "webservd/encryptor.h"
#include "webservd/firewall_interface.h"
#include "webservd/server_interface.h"
+#include "webservd/temp_file_manager.h"
namespace webservd {
@@ -56,6 +57,7 @@
void ProtocolHandlerStarted(ProtocolHandler* handler) override;
void ProtocolHandlerStopped(ProtocolHandler* handler) override;
const Config& GetConfig() const override { return config_; }
+ TempFileManager* GetTempFileManager() override { return &temp_file_manager_; }
scoped_refptr<dbus::Bus> GetBus() { return dbus_object_->GetBus(); }
@@ -63,6 +65,7 @@
void CreateProtocolHandler(Config::ProtocolHandler* handler_config);
void InitTlsData();
void OnFirewallServiceOnline();
+ base::FilePath GetUploadDirectory() const;
org::chromium::WebServer::ServerAdaptor dbus_adaptor_{this};
std::unique_ptr<chromeos::dbus_utils::DBusObject> dbus_object_;
@@ -85,6 +88,9 @@
// The firewall service handler.
const std::unique_ptr<FirewallInterface> firewall_;
+ FileDeleter file_deleter_;
+ TempFileManager temp_file_manager_{GetUploadDirectory(), &file_deleter_};
+
base::WeakPtrFactory<Server> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(Server);
};
diff --git a/webservd/server_interface.h b/webservd/server_interface.h
index afcb0bf..b7dab76 100644
--- a/webservd/server_interface.h
+++ b/webservd/server_interface.h
@@ -22,6 +22,7 @@
namespace webservd {
class ProtocolHandler;
+class TempFileManager;
// An abstract interface to expose Server object to IPC transport layer such as
// D-Bus.
@@ -37,6 +38,11 @@
// Returns the server configuration data.
virtual const Config& GetConfig() const = 0;
+ // Returns the temp file manager used to track life-times of temporary files.
+ // The returned pointer is still owned by the server, so it must not be
+ // stored or deleted.
+ virtual TempFileManager* GetTempFileManager() = 0;
+
protected:
// This interface should not be used to control the life-time of the class
// that derives from this interface. This is especially important when a mock
diff --git a/webservd/temp_file_manager.cc b/webservd/temp_file_manager.cc
new file mode 100644
index 0000000..87606a6
--- /dev/null
+++ b/webservd/temp_file_manager.cc
@@ -0,0 +1,60 @@
+// Copyright 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "webservd/temp_file_manager.h"
+
+#include <base/format_macros.h>
+#include <base/files/file_util.h>
+#include <base/strings/stringprintf.h>
+
+namespace webservd {
+
+TempFileManager::TempFileManager(const base::FilePath& temp_dir_path,
+ FileDeleterInterface* file_deleter)
+ : temp_dir_path_{temp_dir_path}, file_deleter_{file_deleter} {
+}
+
+TempFileManager::~TempFileManager() {
+ for (const auto& pair : request_files_)
+ DeleteFiles(pair.second);
+}
+
+base::FilePath TempFileManager::CreateTempFileName(
+ const std::string& request_id) {
+ std::vector<base::FilePath>& file_list_ref = request_files_[request_id];
+ std::string name = base::StringPrintf("%s-%" PRIuS, request_id.c_str(),
+ file_list_ref.size() + 1);
+ base::FilePath file_name = temp_dir_path_.AppendASCII(name);
+ file_list_ref.push_back(file_name);
+ return file_name;
+}
+
+void TempFileManager::DeleteRequestTempFiles(const std::string& request_id) {
+ auto iter = request_files_.find(request_id);
+ if (iter == request_files_.end())
+ return;
+ DeleteFiles(iter->second);
+ request_files_.erase(iter);
+}
+
+void TempFileManager::DeleteFiles(const std::vector<base::FilePath>& files) {
+ for (const base::FilePath& file : files)
+ CHECK(file_deleter_->DeleteFile(file));
+}
+
+bool FileDeleter::DeleteFile(const base::FilePath& path) {
+ return base::DeleteFile(path, false);
+}
+
+} // namespace webservd
diff --git a/webservd/temp_file_manager.h b/webservd/temp_file_manager.h
new file mode 100644
index 0000000..061309e
--- /dev/null
+++ b/webservd/temp_file_manager.h
@@ -0,0 +1,79 @@
+// Copyright 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef WEBSERVER_WEBSERVD_TEMP_FILE_MANAGER_H_
+#define WEBSERVER_WEBSERVD_TEMP_FILE_MANAGER_H_
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include <base/files/file_path.h>
+#include <base/macros.h>
+
+namespace webservd {
+
+// TempFileManager maintains life-times of temporary files associated with HTTP
+// requests. Web server might require temporary storage to back certain large
+// requests and this class is used to track those files and make sure that all
+// the temporary files are deleted when the request is complete.
+class TempFileManager {
+ public:
+ // FileSystemInterface allows to abstract the file system in tests.
+ class FileDeleterInterface {
+ public:
+ virtual bool DeleteFile(const base::FilePath& path) = 0;
+
+ protected:
+ virtual ~FileDeleterInterface() = default;
+ };
+
+ TempFileManager(const base::FilePath& temp_dir_path,
+ FileDeleterInterface* file_deleter);
+ ~TempFileManager();
+
+ // Generate a new temporary file name for a request with unique ID
+ // |request_id|. No actual file is created on the file system at this time.
+ // The file name is registered with the request ID so it can be later deleted
+ // when request is completed.
+ base::FilePath CreateTempFileName(const std::string& request_id);
+
+ // Deletes all the files belonging to the given request.
+ void DeleteRequestTempFiles(const std::string& request_id);
+
+ private:
+ // Deletes all the files in the list.
+ void DeleteFiles(const std::vector<base::FilePath>& files);
+
+ // Root temp directory to store temporary files into.
+ base::FilePath temp_dir_path_;
+ // File system interface to abstract underlying file system for testing.
+ FileDeleterInterface* file_deleter_;
+
+ // List of files belonging to a particular request.
+ std::map<std::string, std::vector<base::FilePath>> request_files_;
+
+ DISALLOW_COPY_AND_ASSIGN(TempFileManager);
+};
+
+// Actual implementation of FileDeleterInterface to delete temporary files
+// on the real file system.
+class FileDeleter : public TempFileManager::FileDeleterInterface {
+ public:
+ bool DeleteFile(const base::FilePath& path) override;
+};
+
+} // namespace webservd
+
+#endif // WEBSERVER_WEBSERVD_TEMP_FILE_MANAGER_H_