Skip to content

Commit fdb9f83

Browse files
committed
fix: Replace deprecated tmpnam with mkstemp in test utilities
Replace std::tmpnam() with a safer mkstemp()-based implementation in daemon_utils_test.cpp and file_logging_test.cpp. The deprecated tmpnam() function has security vulnerabilities and is not recommended for modern C++ code. Changes: - Add GenerateTempFilePath() helper using mkstemp() - Replace all tmpnam() calls with the new helper - Add required headers (unistd.h, stdexcept) - Improve temporary file path generation security
1 parent 4898c2d commit fdb9f83

File tree

2 files changed

+55
-20
lines changed

2 files changed

+55
-20
lines changed

tests/utils/daemon_utils_test.cpp

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,37 @@
1717
#include <cstdio>
1818
#include <filesystem>
1919
#include <fstream>
20+
#include <stdexcept>
2021
#endif
2122

2223
using namespace mygramdb::utils;
2324

2425
#ifndef _WIN32
2526

27+
namespace {
28+
/**
29+
* @brief Generate a unique temporary file path using mkstemp
30+
* @return Temporary file path
31+
*/
32+
std::string GenerateTempFilePath() {
33+
char temp_template[] = "/tmp/mygramdb_test_XXXXXX";
34+
int fd = mkstemp(temp_template);
35+
if (fd == -1) {
36+
throw std::runtime_error("Failed to create temporary file");
37+
}
38+
close(fd);
39+
unlink(temp_template); // Remove the file, we just need the path
40+
return std::string(temp_template);
41+
}
42+
} // namespace
43+
2644
/**
2745
* @brief Test that Daemonize() successfully creates a background process
2846
*/
2947
TEST(DaemonUtilsTest, DaemonizeSuccess) {
3048
// Create a test file to verify daemon execution
31-
const char* temp_file = std::tmpnam(nullptr);
32-
std::string test_file = std::string(temp_file) + ".daemon_test";
49+
std::string temp_file = GenerateTempFilePath();
50+
std::string test_file = temp_file + ".daemon_test";
3351

3452
// Remove if exists
3553
std::filesystem::remove(test_file);
@@ -81,8 +99,8 @@ TEST(DaemonUtilsTest, DaemonizeSuccess) {
8199
* @brief Test that daemon process has no controlling terminal
82100
*/
83101
TEST(DaemonUtilsTest, NoControllingTerminal) {
84-
const char* temp_file = std::tmpnam(nullptr);
85-
std::string test_file = std::string(temp_file) + ".ctty_test";
102+
std::string temp_file = GenerateTempFilePath();
103+
std::string test_file = temp_file + ".ctty_test";
86104

87105
std::filesystem::remove(test_file);
88106

@@ -134,8 +152,8 @@ TEST(DaemonUtilsTest, NoControllingTerminal) {
134152
* @brief Test that daemon process is in a new session
135153
*/
136154
TEST(DaemonUtilsTest, NewSessionCreated) {
137-
const char* temp_file = std::tmpnam(nullptr);
138-
std::string test_file = std::string(temp_file) + ".session_test";
155+
std::string temp_file = GenerateTempFilePath();
156+
std::string test_file = temp_file + ".session_test";
139157

140158
std::filesystem::remove(test_file);
141159

@@ -184,8 +202,8 @@ TEST(DaemonUtilsTest, NewSessionCreated) {
184202
* @brief Test daemon working directory is root
185203
*/
186204
TEST(DaemonUtilsTest, WorkingDirectoryIsRoot) {
187-
const char* temp_file = std::tmpnam(nullptr);
188-
std::string test_file = std::string(temp_file) + ".cwd_test";
205+
std::string temp_file = GenerateTempFilePath();
206+
std::string test_file = temp_file + ".cwd_test";
189207

190208
std::filesystem::remove(test_file);
191209

tests/utils/file_logging_test.cpp

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,31 @@
66
#include <gtest/gtest.h>
77
#include <spdlog/sinks/basic_file_sink.h>
88
#include <spdlog/spdlog.h>
9+
#include <unistd.h>
910

1011
#include <cstdio>
1112
#include <filesystem>
1213
#include <fstream>
14+
#include <stdexcept>
1315
#include <string>
1416

1517
namespace {
1618

19+
/**
20+
* @brief Generate a unique temporary file path using mkstemp
21+
* @return Temporary file path
22+
*/
23+
std::string GenerateTempFilePath() {
24+
char temp_template[] = "/tmp/mygramdb_test_XXXXXX";
25+
int fd = mkstemp(temp_template);
26+
if (fd == -1) {
27+
throw std::runtime_error("Failed to create temporary file");
28+
}
29+
close(fd);
30+
unlink(temp_template); // Remove the file, we just need the path
31+
return std::string(temp_template);
32+
}
33+
1734
/**
1835
* @brief Read entire file content
1936
*/
@@ -32,8 +49,8 @@ std::string ReadFileContent(const std::string& filepath) {
3249
* @brief Test basic file logging
3350
*/
3451
TEST(FileLoggingTest, BasicFileLogging) {
35-
const char* temp_file = std::tmpnam(nullptr);
36-
std::string log_file = std::string(temp_file) + ".log";
52+
std::string temp_file = GenerateTempFilePath();
53+
std::string log_file = temp_file + ".log";
3754

3855
// Ensure file doesn't exist
3956
std::filesystem::remove(log_file);
@@ -66,8 +83,8 @@ TEST(FileLoggingTest, BasicFileLogging) {
6683
* @brief Test file logging with directory creation
6784
*/
6885
TEST(FileLoggingTest, DirectoryCreation) {
69-
const char* temp_dir = std::tmpnam(nullptr);
70-
std::string log_dir = std::string(temp_dir) + "_logs";
86+
std::string temp_dir = GenerateTempFilePath();
87+
std::string log_dir = temp_dir + "_logs";
7188
std::string log_file = log_dir + "/test.log";
7289

7390
// Ensure directory doesn't exist
@@ -105,8 +122,8 @@ TEST(FileLoggingTest, DirectoryCreation) {
105122
* @brief Test file logging with multiple messages
106123
*/
107124
TEST(FileLoggingTest, MultipleMessages) {
108-
const char* temp_file = std::tmpnam(nullptr);
109-
std::string log_file = std::string(temp_file) + ".log";
125+
std::string temp_file = GenerateTempFilePath();
126+
std::string log_file = temp_file + ".log";
110127

111128
std::filesystem::remove(log_file);
112129

@@ -141,11 +158,11 @@ TEST(FileLoggingTest, MultipleMessages) {
141158
* @brief Test file logging with nested directory path
142159
*/
143160
TEST(FileLoggingTest, NestedDirectoryPath) {
144-
const char* temp_base = std::tmpnam(nullptr);
145-
std::string log_dir = std::string(temp_base) + "_logs/subdir1/subdir2";
161+
std::string temp_base = GenerateTempFilePath();
162+
std::string log_dir = temp_base + "_logs/subdir1/subdir2";
146163
std::string log_file = log_dir + "/nested.log";
147164

148-
std::filesystem::remove_all(std::string(temp_base) + "_logs");
165+
std::filesystem::remove_all(temp_base + "_logs");
149166

150167
try {
151168
// Create nested directories
@@ -167,7 +184,7 @@ TEST(FileLoggingTest, NestedDirectoryPath) {
167184

168185
// Cleanup
169186
spdlog::drop("test_logger_nested");
170-
std::filesystem::remove_all(std::string(temp_base) + "_logs");
187+
std::filesystem::remove_all(temp_base + "_logs");
171188
} catch (const spdlog::spdlog_ex& ex) {
172189
FAIL() << "spdlog exception: " << ex.what();
173190
}
@@ -177,8 +194,8 @@ TEST(FileLoggingTest, NestedDirectoryPath) {
177194
* @brief Test switching from stdout to file logger
178195
*/
179196
TEST(FileLoggingTest, SwitchToFileLogger) {
180-
const char* temp_file = std::tmpnam(nullptr);
181-
std::string log_file = std::string(temp_file) + ".log";
197+
std::string temp_file = GenerateTempFilePath();
198+
std::string log_file = temp_file + ".log";
182199

183200
std::filesystem::remove(log_file);
184201

0 commit comments

Comments
 (0)