Tests: add HttpServer unit coverage

[heidi-dang]

Revives test coverage from closed PR #26.

Changes

• Make parse_request() and format_response() public for testability
• Create heidi-dashd-lib static library for shared use by daemon and tests
• Add comprehensive HttpServer tests (9 new test cases)
• No production behavior changes - security fixes remain intact

Test Coverage

• Valid GET/POST request parsing
• Request body handling
• Incomplete/empty request handling
• Response formatting (200, 404, custom headers)
• Path with query strings
• Method case preservation

Verification

• All 29 tests passing (10 config + 2 ipc + 2 metrics + 6 job + 9 http)
• Production http.cpp unchanged (security fixes preserved)
• verify_local.sh passes

Safety Check

Production http.cpp diff is empty - no security regressions.

[gemini-code-assist]

Summary of Changes

Hello @heidi-dang, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the test coverage for the HttpServer component by introducing a dedicated unit test suite. It achieves this by adjusting method visibility for testability and refactoring the HTTP server implementation into a shared static library, ensuring that the core daemon logic remains unchanged while gaining robust testing capabilities.

Highlights

• Testability Improvements: The parse_request() and format_response() methods of HttpServer were made public to allow for direct unit testing.
• New Static Library: A new static library, heidi-dashd-lib, was created from http.cpp to enable shared use between the daemon and the new unit tests.
• Comprehensive HttpServer Unit Tests: Nine new unit test cases were added for the HttpServer class, covering various scenarios like valid GET/POST requests, body handling, incomplete/empty requests, response formatting, query strings, and method case preservation.
• No Production Behavior Changes: The changes are purely for testing infrastructure and do not alter the production behavior or introduce any security regressions.

Changelog

• include/heidi-kernel/http.h
  • Changed access specifiers for parse_request and format_response methods from private to public to facilitate testing.
• src/dashd/CMakeLists.txt
  • Created a new static library heidi-dashd-lib from http.cpp.
  • Updated the heidi-dashd executable to link against the newly created heidi-dashd-lib instead of directly compiling http.cpp.
• tests/CMakeLists.txt
  • Added http_test.cpp to the unit_tests executable sources.
  • Linked the unit_tests executable against the new heidi-dashd-lib.
• tests/http_test.cpp
  • Added a new file containing comprehensive unit tests for the HttpServer class, including tests for request parsing and response formatting.

Activity

• No specific activity (comments, reviews, etc.) was provided in the context for this pull request.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces unit test coverage for the HttpServer component, utilizing a static library to share code between the daemon and tests. However, making parse_request public for testing exposes a memory safety risk. The HttpRequest struct, with its non-owning std::string_view members, can lead to dangling pointers and Use-After-Free vulnerabilities if the input string's lifetime is not carefully managed. It is recommended to ensure HttpRequest owns its data or add explicit lifetime documentation. Additionally, some test assertions could be made more precise by checking for exact string matches rather than substring presence to improve robustness.

[gemini-code-assist]

The HttpRequest struct uses std::string_view for its method, path, and body members, which means it does not own the underlying string data. By making parse_request public, you are exposing an API where the returned HttpRequest object contains pointers into the data string passed as an argument.

If a caller passes a temporary string (e.g., server.parse_request(std::string(buffer))) or if the source string is destroyed while the HttpRequest is still in use, these members will become dangling pointers. This leads to a Use-After-Free (UAF) vulnerability, which can cause crashes or memory corruption.

To fix this, consider changing the HttpRequest struct members to std::string so they own their data, or clearly document the lifetime requirements if performance is critical.

[gemini-code-assist]

The current assertions using s.find(...) are not strict enough, as they only check for the presence of substrings and not their order or the overall structure of the HTTP response. This could allow formatting errors to go unnoticed. It's better to assert against the exact expected response string for a more robust test.

Suggested change

	EXPECT_NE(s.find("HTTP/1.1 200 OK\r\n"), std::string::npos);
	EXPECT_NE(s.find("Content-Type: text/plain\r\n"), std::string::npos);
	EXPECT_NE(s.find("Content-Length: 5\r\n"), std::string::npos);
	// Check end of headers and body
	EXPECT_NE(s.find("\r\n\r\nhello"), std::string::npos);
	EXPECT_EQ(s, "HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nContent-Length: 5\r\n\r\nhello");

[gemini-code-assist]

Similar to the other response formatting tests, using s.find(...) is not very strict. Asserting against the exact expected string ensures the response is perfectly formatted and makes the test more robust against future changes.

Suggested change

	EXPECT_NE(s.find("HTTP/1.1 404 Not Found\r\n"), std::string::npos);
	EXPECT_NE(s.find("Content-Length: 0\r\n"), std::string::npos);
	EXPECT_EQ(s, "HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\n\r\n");

[gemini-code-assist]

These checks are not very strict. They don't verify the order of headers or the exact structure of the response. Also, the automatically added Content-Length header is not being checked. A single, exact match assertion would be more robust. Since resp.headers is a std::map, header order is predictable ("Content-Type" then "Location").

Suggested change

	EXPECT_NE(s.find("HTTP/1.1 201 Created\r\n"), std::string::npos);
	EXPECT_NE(s.find("Content-Type: application/json\r\n"), std::string::npos);
	EXPECT_NE(s.find("Location: /items/123\r\n"), std::string::npos);
	EXPECT_NE(s.find("\r\n\r\n{\"id\": 123}"), std::string::npos);
	EXPECT_EQ(s, "HTTP/1.1 201 Created\r\nContent-Type: application/json\r\nLocation: /items/123\r\nContent-Length: 12\r\n\r\n{\"id\": 123}");