Code cleanup

This commit is contained in:
pmb
2025-06-25 17:25:50 -07:00
parent e880f6ba1d
commit 2a0c8b6dc8
4 changed files with 42 additions and 79 deletions
+36 -4
View File
@@ -3,14 +3,46 @@
#include <fstream> #include <fstream>
#include <string_view> #include <string_view>
const std::filesystem::path kPATH{"/var/finger/users/"}; bool RealFilesystemWrapper::exists(const std::filesystem::path &path) const {
return std::filesystem::exists(path);
}
std::string
RealFilesystemWrapper::read_file(const std::filesystem::path &path) const {
std::ifstream file(path);
if (!file.is_open()) {
return "";
}
std::string content;
std::string line;
while (std::getline(file, line)) {
content += line + "\n";
}
if (content.empty()) {
return "";
}
// Return the content with proper line endings
if (content.back() == '\n') {
content.pop_back(); // Remove the last newline
content += "\r\n";
return content;
}
content += "\r\n";
return content;
}
std::string process(const std::string &username) { std::string process(const std::string &username) {
RealFilesystemWrapper fs; RealFilesystemWrapper fs;
return process(username, fs); return process(username, fs, kPATH);
} }
std::string process(const std::string &username, const IFilesystemWrapper &fs) { std::string process(const std::string &username, const IFilesystemWrapper &fs,
const std::filesystem::path &basepath) {
try { try {
// Check for directory traversal patterns // Check for directory traversal patterns
if (username.find("../") != std::string::npos || if (username.find("../") != std::string::npos ||
@@ -34,7 +66,7 @@ std::string process(const std::string &username, const IFilesystemWrapper &fs) {
} }
// Attempt to open the plan file (if any) and return the contents as a string // Attempt to open the plan file (if any) and return the contents as a string
std::filesystem::path planPath = kPATH / username; std::filesystem::path planPath = basepath / username;
// Check if the plan file exists using the filesystem wrapper // Check if the plan file exists using the filesystem wrapper
if (!fs.exists(planPath)) { if (!fs.exists(planPath)) {
+6 -29
View File
@@ -15,34 +15,8 @@ public:
class RealFilesystemWrapper : public IFilesystemWrapper { class RealFilesystemWrapper : public IFilesystemWrapper {
public: public:
bool exists(const std::filesystem::path &path) const override { bool exists(const std::filesystem::path &path) const override;
return std::filesystem::exists(path); std::string read_file(const std::filesystem::path &path) const override;
}
std::string read_file(const std::filesystem::path &path) const override {
std::ifstream file(path);
if (!file.is_open()) {
return "";
}
std::string content;
std::string line;
while (std::getline(file, line)) {
content += line + "\n";
}
// Return the content with proper line endings
if (!content.empty() && content.back() == '\n') {
content.pop_back(); // Remove the last newline
content += "\r\n";
return content;
} else if (!content.empty()) {
content += "\r\n";
return content;
}
return "";
}
}; };
class InvalidInput : public std::runtime_error { class InvalidInput : public std::runtime_error {
@@ -50,5 +24,8 @@ public:
InvalidInput(const std::string &what = "") : std::runtime_error(what) {} InvalidInput(const std::string &what = "") : std::runtime_error(what) {}
}; };
const std::filesystem::path kPATH{"/var/finger/users/"};
std::string process(const std::string &username); std::string process(const std::string &username);
std::string process(const std::string &username, const IFilesystemWrapper &fs); std::string process(const std::string &username, const IFilesystemWrapper &fs,
const std::filesystem::path &basepath = kPATH);
-5
View File
@@ -1,5 +1,3 @@
#include <iostream> #include <iostream>
#include <boost/asio/co_spawn.hpp> #include <boost/asio/co_spawn.hpp>
@@ -22,8 +20,6 @@ namespace this_coro = boost::asio::this_coro;
awaitable<std::string> dofinger(const std::string &username) { awaitable<std::string> dofinger(const std::string &username) {
co_return process(username); co_return process(username);
// co_return std::string("this is some finger information for ") + username +
// std::string("\r\n");
} }
awaitable<void> echo(tcp::socket socket) { awaitable<void> echo(tcp::socket socket) {
@@ -31,7 +27,6 @@ awaitable<void> echo(tcp::socket socket) {
char data[1024]; char data[1024];
auto bytes_read = auto bytes_read =
co_await socket.async_read_some(boost::asio::buffer(data), deferred); co_await socket.async_read_some(boost::asio::buffer(data), deferred);
// strip \r\n from data
std::string username(data, bytes_read); std::string username(data, bytes_read);
// Remove trailing \r\n characters // Remove trailing \r\n characters
while (!username.empty() && while (!username.empty() &&
-41
View File
@@ -4,23 +4,6 @@
#include <gtest/gtest.h> #include <gtest/gtest.h>
#include <memory> #include <memory>
// Note: This is a demonstration of how to set up Google Mock for filesystem
// operations. To properly mock std::filesystem::exists, you would need to
// refactor handler.cpp to use dependency injection or create a filesystem
// wrapper interface.
// Mock interface for filesystem operations
/*
class IFilesystemWrapper {
public:
virtual ~IFilesystemWrapper() = default;
virtual bool exists(const std::filesystem::path& path) const = 0;
virtual std::string read_file(const std::filesystem::path& path) const = 0;
};
*/
#include "handler.hpp"
// Mock implementation // Mock implementation
class MockFilesystemWrapper : public IFilesystemWrapper { class MockFilesystemWrapper : public IFilesystemWrapper {
public: public:
@@ -100,30 +83,6 @@ TEST_F(ProcessMockTest, MultipleFileOperations) {
EXPECT_FALSE(mock_filesystem->exists("/path2")); EXPECT_FALSE(mock_filesystem->exists("/path2"));
} }
/*
* REFACTORING SUGGESTION:
*
* To properly mock std::filesystem::exists in your handler.cpp, consider:
*
* 1. Create a filesystem wrapper interface:
* class IFilesystemWrapper {
* public:
* virtual bool exists(const std::filesystem::path& path) const = 0;
* virtual std::string read_file(const std::filesystem::path& path) const
* = 0;
* };
*
* 2. Modify process() function to accept the wrapper:
* std::string process(const std::string& username,
* const IFilesystemWrapper& fs =
* RealFilesystemWrapper{});
*
* 3. Use dependency injection in tests:
* MockFilesystemWrapper mock_fs;
* EXPECT_CALL(mock_fs, exists(_)).WillOnce(Return(true));
* std::string result = process("testuser", mock_fs);
*/
int main(int argc, char **argv) { int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv); ::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS(); return RUN_ALL_TESTS();