Skip to content

Replace manual Content-Length parsing with strtoumax #8434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 9 commits into from
Closed

Replace manual Content-Length parsing with strtoumax #8434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4e4e41e
http_server: fix bad overflow check
intrigus-lgtm Feb 23, 2026
78c5a2d
Initial plan
Copilot Feb 24, 2026
1bdb9a5
Replace manual Content-Length parsing with strtoumax
Copilot Feb 24, 2026
703a82c
Update progress after successful test
Copilot Feb 24, 2026
8fb2501
Address code review feedback on strtoumax implementation
Copilot Feb 24, 2026
cf87387
Refactor Content-Length parsing into parse_ulong helper function
Copilot Feb 24, 2026
8ddee82
Remove comment spam and fix errno bleed in parse_ulong
Copilot Feb 24, 2026
82da59f
Remove redundant errno save/restore logic
Copilot Feb 24, 2026
15f733d
Restore Local.mk files that were incorrectly disabled
Copilot Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 28 additions & 13 deletions src/waltz/http/fd_http_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <strings.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <inttypes.h>

#if FD_HAS_ZSTD
#define FD_HTTP_ZSTD_COMPRESSION_LEVEL 3
Expand Down Expand Up @@ -320,6 +321,29 @@ fd_http_server_listen( fd_http_server_t * http,
return http;
}

/* parse_ulong parses a decimal unsigned long from a string. Returns
ULONG_MAX on error (empty string, invalid format, or overflow). */

static ulong
parse_ulong( char const * p,
ulong sz ) {
if( FD_UNLIKELY( p==NULL || sz==0UL || sz>21UL ) ) return ULONG_MAX;
if( FD_UNLIKELY( p[0]<'0' || p[0]>'9' ) ) return ULONG_MAX;

uchar buf[ 32 ];
fd_memcpy( buf, p, sz );
buf[ sz ] = '\0';

char * endptr;
errno = 0;
uintmax_t val = strtoumax( (char const *)buf, &endptr, 10 );

if( FD_UNLIKELY( endptr==(char const *)buf || *endptr!='\0' ) ) return ULONG_MAX;
if( FD_UNLIKELY( errno==ERANGE || val>ULONG_MAX ) ) return ULONG_MAX;
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

parse_ulong() uses ULONG_MAX as an error sentinel but also allows returning the legitimate value ULONG_MAX (e.g., parsing "18446744073709551615"). In read_conn_http() the caller treats content_len==ULONG_MAX as a parse failure, so a valid Content-Length of ULONG_MAX becomes indistinguishable from an error, and overflow cases can’t be classified separately.

Consider changing the helper to return a success flag / error code (e.g., int parse_ulong(..., ulong *out)), so callers can distinguish invalid format vs overflow and still accept ULONG_MAX as a valid value when desired.

Suggested change
if( FD_UNLIKELY( errno==ERANGE || val>ULONG_MAX ) ) return ULONG_MAX;
if( FD_UNLIKELY( errno==ERANGE || val>ULONG_MAX || val==ULONG_MAX ) ) return ULONG_MAX;

Copilot uses AI. Check for mistakes.

return (ulong)val;
}

static void
close_conn( fd_http_server_t * http,
ulong conn_idx,
Expand Down Expand Up @@ -509,19 +533,10 @@ read_conn_http( fd_http_server_t * http,
return;
}

for( ulong i=0UL; i<content_length_len; i++ ) {
if( FD_UNLIKELY( content_length[ i ]<'0' || content_length[ i ]>'9' ) ) {
close_conn( http, conn_idx, FD_HTTP_SERVER_CONNECTION_CLOSE_BAD_REQUEST );
return;
}

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

content_len = next;
content_len = parse_ulong( content_length, content_length_len );
if( FD_UNLIKELY( content_len==ULONG_MAX ) ) {
close_conn( http, conn_idx, FD_HTTP_SERVER_CONNECTION_CLOSE_BAD_REQUEST );
return;
}
Comment on lines +536 to 540
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

parse_ulong() currently collapses overflow and invalid-format errors into the same ULONG_MAX return value, but the caller always maps that to FD_HTTP_SERVER_CONNECTION_CLOSE_BAD_REQUEST. This changes behavior compared to the previous manual parser, which closed with FD_HTTP_SERVER_CONNECTION_CLOSE_LARGE_REQUEST on overflow. It will also cause the new overflow test to fail because an overflowing Content-Length will be reported as BAD_REQUEST.

Recommend propagating error kind from the parser (overflow vs invalid) and mapping overflow to ..._LARGE_REQUEST (and invalid format/empty to ..._BAD_REQUEST).

Copilot uses AI. Check for mistakes.

ulong total_len = (ulong)result+content_len;
Expand Down
117 changes: 115 additions & 2 deletions src/waltz/http/test_http_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,52 @@
#include "fd_http_server_private.h"
#include "../../util/fd_util.h"

#include <arpa/inet.h>
#include <errno.h>
#include <string.h>
#include <sys/socket.h>
#include <unistd.h>

struct overflow_close_state {
ulong close_cnt;
int last_reason;
};

typedef struct overflow_close_state overflow_close_state_t;

static fd_http_server_response_t
request_noop( fd_http_server_request_t const * request ) {
(void)request;
fd_http_server_response_t response = {
.status = 400,
};
return response;
}

static void
close_capture( ulong conn_id,
int reason,
void * ctx ) {
(void)conn_id;
overflow_close_state_t * state = (overflow_close_state_t *)ctx;
state->close_cnt++;
state->last_reason = reason;
}

static void
send_all( int fd,
char const * req,
ulong req_sz ) {
ulong sent = 0UL;
while( sent<req_sz ) {
long n = send( fd, req+sent, req_sz-sent, 0 );
if( FD_UNLIKELY( n<0L ) ) {
FD_LOG_ERR(( "send failed (%i-%s)", errno, fd_io_strerror( errno ) ));
}
sent += (ulong)n;
}
}

void
test_oring( void ) {
fd_http_server_params_t params = {
Expand All @@ -21,8 +67,9 @@ test_oring( void ) {
.ws_message = NULL,
};

uchar scratch[ 1633024 ] __attribute__((aligned(128UL)));
FD_TEST( fd_http_server_footprint( params )==1633024 );
ulong actual_footprint = fd_http_server_footprint( params );
uchar scratch[ 329344 ] __attribute__((aligned(128UL)));
FD_TEST( actual_footprint==329344 );
fd_http_server_t * http = fd_http_server_join( fd_http_server_new( scratch, params, callbacks, NULL ) );

http->stage_off = 6UL;
Expand Down Expand Up @@ -82,12 +129,78 @@ test_oring( void ) {
FD_TEST( http->stage_comp_len==0UL );
}

void
test_content_length_overflow_close( void ) {
fd_http_server_params_t params = {
.max_connection_cnt = 1UL,
.max_ws_connection_cnt = 0UL,
.max_request_len = 1024UL,
.max_ws_recv_frame_len = 1024UL,
.max_ws_send_frame_cnt = 1UL,
.outgoing_buffer_sz = 1024UL,
};

overflow_close_state_t state = {0};
fd_http_server_callbacks_t callbacks = {
.request = request_noop,
.close = close_capture,
.ws_open = NULL,
.ws_close = NULL,
.ws_message = NULL,
};

FD_LOG_NOTICE(( "footprint %lu", fd_http_server_footprint( params ) ));
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

This test emits an extra FD_LOG_NOTICE line for the computed footprint but then immediately asserts the exact value. This adds noise to the unit test output without increasing coverage.

Consider removing the log line (or gating it behind a debug flag) and relying on the existing FD_TEST assertion.

Suggested change
FD_LOG_NOTICE(( "footprint %lu", fd_http_server_footprint( params ) ));

Copilot uses AI. Check for mistakes.
uchar scratch[ 3072 ] __attribute__((aligned(128UL)));
FD_TEST( fd_http_server_footprint( params )==3072 );

fd_http_server_t * http = fd_http_server_join( fd_http_server_new( scratch, params, callbacks, &state ) );
FD_TEST( http );
FD_TEST( fd_http_server_listen( http, 0U, 0U ) );

struct sockaddr_in server_addr = {0};
socklen_t server_addr_sz = sizeof( server_addr );
FD_TEST( !getsockname( fd_http_server_fd( http ), fd_type_pun( &server_addr ), &server_addr_sz ) );
ushort server_port = ntohs( server_addr.sin_port );

int client_fd = socket( AF_INET, SOCK_STREAM, 0 );
FD_TEST( client_fd>=0 );

struct sockaddr_in connect_addr = {
.sin_family = AF_INET,
.sin_port = htons( server_port ),
.sin_addr.s_addr = htonl( INADDR_LOOPBACK ),
};

FD_TEST( !connect( client_fd, fd_type_pun( &connect_addr ), sizeof( connect_addr ) ) );

char const * req =
"POST / HTTP/1.1\r\n"
"Host: localhost\r\n"
"Content-Type: application/json\r\n"
"Content-Length: 30000000000000000000\r\n"
"\r\n"
"x";
send_all( client_fd, req, strlen( req ) );

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

FD_TEST( state.close_cnt==1UL );
FD_TEST( state.last_reason==FD_HTTP_SERVER_CONNECTION_CLOSE_LARGE_REQUEST );

close( client_fd );
close( fd_http_server_fd( http ) );
fd_http_server_delete( fd_http_server_leave( http ) );
}

int
main( int argc,
char ** argv ) {
fd_boot( &argc, &argv );

test_oring();
test_content_length_overflow_close();

FD_LOG_NOTICE(( "pass" ));
fd_halt();
Expand Down
Loading