Add temp file and FD support to TemporaryFileHandle
This commit adds file descriptor and temporary file management to
DuplicatableFileHandle, removing the redundant test-only
TemporaryFileHandle utility.
Changes:
- Add file descriptor constructor and setFd() method
- Add temporary file constructor with string_view content
- Add filePath member and automatic cleanup in destructor
- Add configurable temp-dir meson option (default: /tmp/bmcweb)
- Remove include/file_test_utilities.hpp
- Update all tests to use DuplicatableFileHandle
- Rename stringPath to filePath
These features will be used by the multipart parser to stream
large uploads to temporary files instead of keeping them in memory,
and by the update service to pass file descriptors over D-Bus.
Change-Id: I982f5928d453f9f0c13d91c3525006134ddc87b3
Signed-off-by: Rajeev Ranjan <ranjan.rajeev1609@gmail.com>
diff --git a/README.md b/README.md
index ed4d106..89207b7 100644
--- a/README.md
+++ b/README.md
@@ -70,6 +70,12 @@
internal to the process. Details on the exact data stored and when it is
read/written can seen from the `persistent_data` namespace.
+## Temporary files
+
+bmcweb uses `/tmp/bmcweb` for temporary file storage during multipart form
+uploads. This directory is automatically cleaned up by systemd on service
+restart via the `TemporaryFileSystem` directive in the service file.
+
## TLS certificate generation
When SSL support is enabled and a usable certificate is not found, bmcweb will
diff --git a/config/bmcweb.service.in b/config/bmcweb.service.in
index 847c106..c9089ce 100644
--- a/config/bmcweb.service.in
+++ b/config/bmcweb.service.in
@@ -14,6 +14,7 @@
StateDirectory=@BMCWEB_STATE_DIRECTORY@
SyslogLevelPrefix=true
WatchdogSec=@BMCWEB_WATCHDOG_TIMEOUT_SECONDS@s
+TemporaryFileSystem=/tmp/bmcweb:noexec,nodev,nosuid
WorkingDirectory=@BMCWEB_WORKING_DIRECTORY@
# bmcweb currently uses /tmp as a mechanism to share files.
diff --git a/include/duplicatable_file_handle.hpp b/include/duplicatable_file_handle.hpp
index f199485..e346096 100644
--- a/include/duplicatable_file_handle.hpp
+++ b/include/duplicatable_file_handle.hpp
@@ -2,13 +2,63 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright OpenBMC Authors
+#include "bmcweb_config.h"
+
+#include "logging.hpp"
+
#include <unistd.h>
#include <boost/beast/core/file_posix.hpp>
+#include <cerrno>
+#include <filesystem>
+#include <string>
+#include <string_view>
+
struct DuplicatableFileHandle
{
boost::beast::file_posix fileHandle;
+ std::string filePath;
+
+ // Construct from a file descriptor
+ explicit DuplicatableFileHandle(int fd)
+ {
+ fileHandle.native_handle(fd);
+ }
+
+ // Creates a temporary file with the contents provided, removes it on
+ // destruction.
+ explicit DuplicatableFileHandle(std::string_view contents)
+ {
+ std::filesystem::path tempDir("/tmp/bmcweb");
+ std::error_code ec;
+ std::filesystem::create_directories(tempDir, ec);
+ if (ec)
+ {
+ BMCWEB_LOG_ERROR("Failed to create directory {}: {}",
+ tempDir.string(), ec.value());
+ }
+
+ filePath = (tempDir / "XXXXXXXXXXX").string();
+
+ int fd = mkstemp(filePath.data());
+ if (fd < 0)
+ {
+ BMCWEB_LOG_ERROR("Failed to create temporary file: {}", errno);
+ return;
+ }
+ ssize_t written = write(fd, contents.data(), contents.size());
+ if (written < 0 || static_cast<size_t>(written) != contents.size())
+ {
+ BMCWEB_LOG_ERROR("Failed to write to temporary file: {}", errno);
+ }
+ close(fd);
+ }
+
+ void setFd(int fd)
+ {
+ fileHandle.native_handle(fd);
+ }
DuplicatableFileHandle() = default;
DuplicatableFileHandle(DuplicatableFileHandle&&) noexcept = default;
@@ -29,5 +79,12 @@
}
DuplicatableFileHandle& operator=(DuplicatableFileHandle&& other) noexcept =
default;
- ~DuplicatableFileHandle() = default;
+
+ ~DuplicatableFileHandle()
+ {
+ if (!filePath.empty())
+ {
+ std::filesystem::remove(filePath);
+ }
+ }
};
diff --git a/include/file_test_utilities.hpp b/include/file_test_utilities.hpp
deleted file mode 100644
index b12fbca..0000000
--- a/include/file_test_utilities.hpp
+++ /dev/null
@@ -1,42 +0,0 @@
-// SPDX-License-Identifier: Apache-2.0
-// SPDX-FileCopyrightText: Copyright OpenBMC Authors
-#pragma once
-#include <unistd.h>
-
-#include <filesystem>
-#include <string>
-#include <string_view>
-
-#include <gtest/gtest.h>
-
-struct TemporaryFileHandle
-{
- std::filesystem::path path;
- std::string stringPath;
-
- // Creates a temporary file with the contents provided, removes it on
- // destruction.
- explicit TemporaryFileHandle(std::string_view sampleData) :
- path(std::filesystem::temp_directory_path() /
- "bmcweb_http_response_test_XXXXXXXXXXX")
- {
- stringPath = path.string();
-
- // NOLINTNEXTLINE(misc-include-cleaner)
- int fd = mkstemp(stringPath.data());
- EXPECT_GT(fd, 0);
- EXPECT_EQ(write(fd, sampleData.data(), sampleData.size()),
- sampleData.size());
- close(fd);
- }
-
- TemporaryFileHandle(const TemporaryFileHandle&) = delete;
- TemporaryFileHandle(TemporaryFileHandle&&) = delete;
- TemporaryFileHandle& operator=(const TemporaryFileHandle&) = delete;
- TemporaryFileHandle& operator=(TemporaryFileHandle&&) = delete;
-
- ~TemporaryFileHandle()
- {
- std::filesystem::remove(path);
- }
-};
diff --git a/meson.options b/meson.options
index d598026..f255f81 100644
--- a/meson.options
+++ b/meson.options
@@ -563,3 +563,4 @@
enable on production systems at this time. Other query
parameters such as only are not controlled by this option.''',
)
+
diff --git a/test/http/http2_connection_test.cpp b/test/http/http2_connection_test.cpp
index a6bc81c..3648892 100644
--- a/test/http/http2_connection_test.cpp
+++ b/test/http/http2_connection_test.cpp
@@ -9,7 +9,7 @@
#include "test_stream.hpp"
#include <nghttp2/nghttp2.h>
-#include <unistd.h>
+#include <sys/types.h>
#include <boost/asio/buffer.hpp>
#include <boost/asio/io_context.hpp>
diff --git a/test/http/http_body_test.cpp b/test/http/http_body_test.cpp
index 889161c..b96fd2c 100644
--- a/test/http/http_body_test.cpp
+++ b/test/http/http_body_test.cpp
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright OpenBMC Authors
-#include "file_test_utilities.hpp"
+#include "duplicatable_file_handle.hpp"
#include "http_body.hpp"
#include <boost/beast/core/file_base.hpp>
@@ -70,9 +70,9 @@
TEST(HttpHttpBodyValueType, MoveFile)
{
HttpBody::value_type value(EncodingType::Base64, CompressionType::Raw);
- TemporaryFileHandle temporaryFile("teststring");
+ DuplicatableFileHandle temporaryFile("teststring");
boost::system::error_code ec;
- value.open(temporaryFile.stringPath.c_str(), boost::beast::file_mode::read,
+ value.open(temporaryFile.filePath.c_str(), boost::beast::file_mode::read,
ec);
ASSERT_FALSE(ec);
// Move constructor
@@ -95,9 +95,9 @@
TEST(HttpHttpBodyValueType, MoveOperatorFile)
{
HttpBody::value_type value(EncodingType::Base64, CompressionType::Raw);
- TemporaryFileHandle temporaryFile("teststring");
+ DuplicatableFileHandle temporaryFile("teststring");
boost::system::error_code ec;
- value.open(temporaryFile.stringPath.c_str(), boost::beast::file_mode::read,
+ value.open(temporaryFile.filePath.c_str(), boost::beast::file_mode::read,
ec);
ASSERT_FALSE(ec);
// Move constructor
@@ -119,9 +119,9 @@
TEST(HttpFileBodyValueType, SetFd)
{
HttpBody::value_type value(EncodingType::Base64, CompressionType::Raw);
- TemporaryFileHandle temporaryFile("teststring");
+ DuplicatableFileHandle temporaryFile("teststring");
boost::system::error_code ec;
- FILE* r = fopen(temporaryFile.stringPath.c_str(), "r");
+ FILE* r = fopen(temporaryFile.filePath.c_str(), "r");
ASSERT_NE(r, nullptr);
value.setFd(fileno(r), ec);
ASSERT_FALSE(ec);
diff --git a/test/http/http_response_test.cpp b/test/http/http_response_test.cpp
index 536415e..69eb637 100644
--- a/test/http/http_response_test.cpp
+++ b/test/http/http_response_test.cpp
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright OpenBMC Authors
-#include "file_test_utilities.hpp"
+#include "duplicatable_file_handle.hpp"
#include "http/http_body.hpp"
#include "http/http_response.hpp"
#include "utility.hpp"
@@ -86,8 +86,8 @@
{
Response res;
addHeaders(res);
- TemporaryFileHandle temporaryFile("sample text");
- res.openFile(temporaryFile.stringPath);
+ DuplicatableFileHandle temporaryFile("sample text");
+ res.openFile(temporaryFile.filePath);
verifyHeaders(res);
}
@@ -95,8 +95,9 @@
{
Response res;
addHeaders(res);
- TemporaryFileHandle temporaryFile("sample text");
- FILE* fd = fopen(temporaryFile.stringPath.c_str(), "r+");
+ DuplicatableFileHandle temporaryFile("sample text");
+ FILE* fd = fopen(temporaryFile.filePath.c_str(), "r+");
+ ASSERT_NE(fd, nullptr);
res.openFd(fileno(fd));
verifyHeaders(res);
fclose(fd);
@@ -106,8 +107,8 @@
{
Response res;
addHeaders(res);
- TemporaryFileHandle temporaryFile("sample text");
- FILE* fd = fopen(temporaryFile.stringPath.c_str(), "r");
+ DuplicatableFileHandle temporaryFile("sample text");
+ FILE* fd = fopen(temporaryFile.filePath.c_str(), "r");
ASSERT_NE(fd, nullptr);
res.openFd(fileno(fd), bmcweb::EncodingType::Base64);
verifyHeaders(res);
@@ -118,8 +119,8 @@
{
Response res;
addHeaders(res);
- TemporaryFileHandle temporaryFile("sample text");
- res.openFile(temporaryFile.stringPath);
+ DuplicatableFileHandle temporaryFile("sample text");
+ res.openFile(temporaryFile.filePath);
verifyHeaders(res);
res.write("body text");
@@ -149,21 +150,23 @@
{
Response res;
std::string data = "sample text";
- TemporaryFileHandle temporaryFile(data);
- FILE* f = fopen(temporaryFile.stringPath.c_str(), "r+");
+ DuplicatableFileHandle temporaryFile(data);
+ FILE* f = fopen(temporaryFile.filePath.c_str(), "r+");
+ ASSERT_NE(f, nullptr);
res.openFd(fileno(f), bmcweb::EncodingType::Base64);
EXPECT_EQ(getData(res.response), "c2FtcGxlIHRleHQ=");
+ fclose(f);
}
TEST(HttpResponse, Base64HttpBodyWriterLarge)
{
Response res;
std::string data = generateBigdata();
- TemporaryFileHandle temporaryFile(data);
+ DuplicatableFileHandle temporaryFile(data);
boost::beast::file_posix file;
boost::system::error_code ec;
- file.open(temporaryFile.stringPath.c_str(), boost::beast::file_mode::read,
+ file.open(temporaryFile.filePath.c_str(), boost::beast::file_mode::read,
ec);
EXPECT_EQ(ec.value(), 0);
res.openFd(file.native_handle(), bmcweb::EncodingType::Base64);
@@ -174,11 +177,11 @@
{
Response res;
std::string data = generateBigdata();
- TemporaryFileHandle temporaryFile(data);
+ DuplicatableFileHandle temporaryFile(data);
boost::beast::file_posix file;
boost::system::error_code ec;
- file.open(temporaryFile.stringPath.c_str(), boost::beast::file_mode::read,
+ file.open(temporaryFile.filePath.c_str(), boost::beast::file_mode::read,
ec);
EXPECT_EQ(ec.value(), 0);
res.openFd(file.native_handle());
diff --git a/test/include/ssl_key_handler_test.cpp b/test/include/ssl_key_handler_test.cpp
index 44da038..0680dcc 100644
--- a/test/include/ssl_key_handler_test.cpp
+++ b/test/include/ssl_key_handler_test.cpp
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright OpenBMC Authors
-#include "file_test_utilities.hpp"
+#include "duplicatable_file_handle.hpp"
#include "ssl_key_handler.hpp"
#include <string>
@@ -14,14 +14,14 @@
{
/* Verifies that we can generate a certificate, then read back in the
* certificate that was read */
- TemporaryFileHandle myFile("");
+ DuplicatableFileHandle myFile("");
std::string cert = generateSslCertificate("TestCommonName");
EXPECT_FALSE(cert.empty());
- writeCertificateToFile(myFile.stringPath, cert);
+ writeCertificateToFile(myFile.filePath, cert);
- std::string cert2 = verifyOpensslKeyCert(myFile.stringPath);
+ std::string cert2 = verifyOpensslKeyCert(myFile.filePath);
EXPECT_FALSE(cert2.empty());
EXPECT_EQ(cert, cert2);
}
diff --git a/test/redfish-core/lib/metadata_test.cpp b/test/redfish-core/lib/metadata_test.cpp
index e382a1e..a175244 100644
--- a/test/redfish-core/lib/metadata_test.cpp
+++ b/test/redfish-core/lib/metadata_test.cpp
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright OpenBMC Authors
-#include "file_test_utilities.hpp"
+#include "duplicatable_file_handle.hpp"
#include "metadata.hpp"
#include <filesystem>
@@ -38,9 +38,9 @@
TEST(MetadataGet, GetOneFile)
{
- TemporaryFileHandle file(content);
+ DuplicatableFileHandle file(content);
- std::filesystem::path path{file.stringPath};
+ std::filesystem::path path{file.filePath};
EXPECT_EQ(
getMetadataPieceForFile(path),
std::format(" <edmx:Reference Uri=\"/redfish/v1/schema/{}\">\n"