http_server: fix bad overflow check by intrigus-lgtm · Pull Request #8391 · firedancer-io/firedancer

intrigus-lgtm · 2026-02-23T13:36:06Z

No description provided.

Copilot

Pull request overview

This PR fixes an integer overflow check when parsing the HTTP Content-Length header in the waltz HTTP server and adds a regression test to ensure overflow triggers the expected connection close path.

Changes:

Replace the content_len*10 + digit wraparound-based overflow detection with a bounds-based check against ULONG_MAX.
Add a unit test that opens a loopback TCP connection and sends an oversized Content-Length value, asserting the server closes with FD_HTTP_SERVER_CONNECTION_CLOSE_LARGE_REQUEST.

Reviewed changes

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

File	Description
`src/waltz/http/test_http_server.c`	Adds a new regression test and supporting helpers to validate overflow handling via a real socket connection.
`src/waltz/http/fd_http_server.c`	Updates `Content-Length` parsing to use a correct pre-multiply overflow check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/waltz/http/test_http_server.c

Copilot · 2026-02-23T13:39:16Z

src/waltz/http/test_http_server.c

+  for( ulong i=0UL; i<200UL && !state.close_cnt; i++ ) {
+    fd_http_server_poll( http, 1 );
+  }


The close-wait loop uses a fixed 200-iteration bound with fd_http_server_poll(http, 1), which can be flaky on slow/loaded CI machines. Consider using a wallclock-based timeout (or a larger bound) so the test fails only after a real elapsed deadline.

src/waltz/http/test_http_server.c

Copilot · 2026-02-23T13:39:16Z

src/waltz/http/fd_http_server.c

+      ulong digit = (ulong)(content_length[ i ]-'0');
+      if( FD_UNLIKELY( content_len>(ULONG_MAX-digit)/10UL ) ) { /* Overflow */
        close_conn( http, conn_idx, FD_HTTP_SERVER_CONNECTION_CLOSE_LARGE_REQUEST );
        return;
      }

-      content_len = next;
+      content_len = content_len*10UL + digit;


ULONG_MAX is used in the new overflow check, but this translation unit doesn't include <limits.h>. This can fail to compile on toolchains where ULONG_MAX isn't pulled in indirectly. Add an explicit #include <limits.h> near the other standard includes.

@copilot ULONG_MAX is provided by fd_util_base.h i think

github-actions · 2026-02-23T13:50:42Z

Performance Measurements ⏳

Suite	Baseline	New	Change
backtest `mainnet-368528500-perf` per slot	`0.063137 s`	`0.063219 s`	`0.130%` ✅
backtest `mainnet-368528500-perf` snapshot load	`3.399 s`	`2.264 s`	`-33.392%` ✅
backtest `mainnet-368528500-perf` total elapsed	`63.136608 s`	`63.219282 s`	`0.131%` ✅
`firedancer mem` usage with `mainnet.toml`	`984.34 GiB`	`984.34 GiB`	`0.000%` ✅

ripatel-fd · 2026-02-23T14:47:37Z

src/waltz/http/fd_http_server.c

-      ulong next = content_len*10UL + (ulong)(content_length[ i ]-'0');
-      if( FD_UNLIKELY( next<content_len ) ) { /* Overflow */
+      ulong digit = (ulong)(content_length[ i ]-'0');
+      if( FD_UNLIKELY( content_len>(ULONG_MAX-digit)/10UL ) ) { /* Overflow */


Can we just use __builtin_uaddl_overflow here?

intrigus-lgtm · 2026-02-24T13:47:44Z

src/waltz/http/fd_http_server.c

+      ulong digit = (ulong)(content_length[ i ]-'0');
+      if( FD_UNLIKELY( content_len>(ULONG_MAX-digit)/10UL ) ) { /* Overflow */
        close_conn( http, conn_idx, FD_HTTP_SERVER_CONNECTION_CLOSE_LARGE_REQUEST );
        return;
      }

-      content_len = next;
+      content_len = content_len*10UL + digit;
    }

    ulong total_len = (ulong)result+content_len;


@ripatel-fd something like this?
Doesn't look pretty, but would avoid a division although the compiler would have probably replaced the division by constant anyway with something smarter.

Suggested change

ulong digit = (ulong)(content_length[ i ]-'0');

if( FD_UNLIKELY( content_len>(ULONG_MAX-digit)/10UL ) ) { /* Overflow */

close_conn( http, conn_idx, FD_HTTP_SERVER_CONNECTION_CLOSE_LARGE_REQUEST );

return;

}

content_len = next;

content_len = content_len*10UL + digit;

}

ulong total_len = (ulong)result+content_len;

ulong digit = (ulong)(content_length[ i ]-'0');

ulong next_content_len;

if( FD_UNLIKELY( __builtin_mul_overflow( content_len, 10UL, &next_content_len ) ||

__builtin_add_overflow( next_content_len, digit, &next_content_len ) ) ) { /* Overflow */

close_conn( http, conn_idx, FD_HTTP_SERVER_CONNECTION_CLOSE_LARGE_REQUEST );

return;

}

content_len = next_content_len;

}

ulong total_len = (ulong)result+content_len;

Copilot · 2026-02-24T20:21:51Z

@ripatel-fd I've opened a new pull request, #8433, to work on those changes. Once the pull request is ready, I'll request review from you.

ripatel-fd · 2026-02-24T20:24:16Z

src/waltz/http/fd_http_server.c


-      content_len = next;
+      content_len = content_len*10UL + digit;
    }


@intrigus-lgtm How about we just copy out the integer string to uchar x[], zero terminate it, and then call https://man7.org/linux/man-pages/man3/strtoumax.3.html for the conversion? IMO that's the cleanest option

@copilot please implement this suggestion by modifying PR 8391 and submit it as a separate PR. Maintain original attribution

Copilot · 2026-02-24T20:25:08Z

@ripatel-fd I've opened a new pull request, #8434, to work on those changes. Once the pull request is ready, I'll request review from you.

intrigus-lgtm requested review from Copilot and mmcgee-jump February 23, 2026 13:36

Copilot started reviewing on behalf of intrigus-lgtm February 23, 2026 13:36 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

http_server: fix bad overflow check

4e4e41e

intrigus-lgtm force-pushed the intrigus/fix/http-server-overflow-check branch from 1403930 to 4e4e41e Compare February 23, 2026 13:40

ripatel-fd reviewed Feb 23, 2026

View reviewed changes

intrigus-lgtm commented Feb 24, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 24, 2026

Address review feedback on ULONG_MAX availability in http_server #8433

Closed

ripatel-fd reviewed Feb 24, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 24, 2026

Replace manual Content-Length parsing with strtoumax #8434

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

http_server: fix bad overflow check#8391

http_server: fix bad overflow check#8391
intrigus-lgtm wants to merge 1 commit intomainfrom
intrigus/fix/http-server-overflow-check

intrigus-lgtm commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

ripatel-fd Feb 24, 2026

Uh oh!

github-actions bot commented Feb 23, 2026

Uh oh!

ripatel-fd Feb 23, 2026

Uh oh!

intrigus-lgtm Feb 24, 2026

Uh oh!

Copilot AI commented Feb 24, 2026

Uh oh!

ripatel-fd Feb 24, 2026

Uh oh!

ripatel-fd Feb 24, 2026

Uh oh!

Copilot AI commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

intrigus-lgtm commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

ripatel-fd Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 23, 2026

Performance Measurements ⏳

Uh oh!

ripatel-fd Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

intrigus-lgtm Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 24, 2026

Uh oh!

ripatel-fd Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

ripatel-fd Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants