Skip to content

Harden multi-threaded http server#5149

Open
SirTyson wants to merge 1 commit intostellar:masterfrom
SirTyson:harden-query-server
Open

Harden multi-threaded http server#5149
SirTyson wants to merge 1 commit intostellar:masterfrom
SirTyson:harden-query-server

Conversation

@SirTyson
Copy link
Contributor

Description

Fixes bug where malformed HTTP headers would crash the multithreaded HTTP server.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a crash vulnerability in the multi-threaded HTTP server by adding exception handling around the stoi call when parsing the Content-Length header. The fix prevents malformed HTTP headers (e.g., non-numeric Content-Length values) from crashing the server.

Changes:

  • Added try-catch block around stoi conversion of Content-Length header value to handle parsing exceptions

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment on lines +299 to +303
body_content_length_ = stoull(header.value);
}
catch (std::exception const&)
{
return bad;
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stoull(header.value) will parse prefixes like "5abc" without throwing, so malformed Content-Length headers may be accepted. Consider validating full consumption (use the pos overload and reject any non-whitespace trailing characters) to avoid inconsistent parsing (including potential request-smuggling issues).

Copilot uses AI. Check for mistakes.
Comment on lines +299 to +300
body_content_length_ = stoull(header.value);
}
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

body_content_length_ is a size_t, but stoull returns unsigned long long; on 32-bit builds a large Content-Length can successfully parse yet truncate when assigned to size_t. Consider parsing into a wider temp, rejecting values > std::numeric_limits<size_t>::max() before assigning, and returning bad on overflow.

Copilot uses AI. Check for mistakes.
body_content_length_ = stoi(header.value);
try
{
body_content_length_ = stoull(header.value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but can we ask claude for a quick unit test that repros the issue and confirms the fix?

@SirTyson SirTyson force-pushed the harden-query-server branch from 35103ba to 3a8d20a Compare February 20, 2026 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments