Skip to content

[:httpd] Fix HTTP large request + response handling #14

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

Draft
harmon25 wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

[:httpd] Fix HTTP large request + response handling #14

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
78c0511
Enhance HTTP request handling with buffering support and improved sta…
harmon25 Nov 22, 2025
c7cfe7e
fix large bodies (from atomvm_httpd)
harmon25 Nov 30, 2025
1bbd2ad
implement review suggestions
harmon25 Dec 16, 2025
e5311c1
Refactor TCP send handling and improve error management in HTTP serve…
harmon25 Jan 7, 2026
067279b
re-introduce changes from previous feedback
harmon25 Jan 7, 2026
5d5c94e
use `erlang:iolist_size(Reply)` to compute content length (works for …
harmon25 Jan 11, 2026
9bed41f
Enhance WebSocket handling by buffering incomplete frames and improvi…
harmon25 Jan 11, 2026
5e92d61
Add support for socket options in HTTP server start functions
harmon25 Jan 11, 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
26 changes: 16 additions & 10 deletions src/gen_tcp_server.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
addr => any
}).
-define(DEFAULT_SOCKET_OPTIONS, #{}).
-define(MAX_SEND_CHUNK, 2048).
Copy link
Copy Markdown
Collaborator

@UncleGrumpy UncleGrumpy Jan 7, 2026

Choose a reason for hiding this comment

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

I wonder if it would be better to set this a bit smaller, like 1024, so that it is less than most network MTU size (typically 1350-1500, depending on network). Exceeding the MTU will cause packet fragmentation, which isn't much of a problem for unix machines, but may not be handled as gracefully by microcontrollers.

Copy link
Copy Markdown
Author

@harmon25 harmon25 Jan 7, 2026

Choose a reason for hiding this comment

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

in my independent fork i landed on 1460
https://github.com/harmon25/atomvm_httpd/blob/diagnostics/src/gen_tcp_server.erl#L49-L51

i will try and port those changes here also

Copy link
Copy Markdown
Collaborator

@UncleGrumpy UncleGrumpy Jan 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

That makes sense. 1500 is the most common MTU, and I think there is some tcp/ip overhead in each packet on top of what httpd is sending. If a VPN is used the MTU will be lower. I think 2048 is too high. We should actually expose this as a configuration option, with some lower default (like 1460 that you are using).


%%
%% API
Expand Down Expand Up @@ -168,16 +169,7 @@ try_send(Socket, Packet) when is_binary(Packet) ->
"Trying to send binary packet data to socket ~p. Packet (or len): ~p", [
Socket, case byte_size(Packet) < 32 of true -> Packet; _ -> byte_size(Packet) end
]),
case socket:send(Socket, Packet) of
ok ->
?TRACE("sent.", []),
ok;
{ok, Rest} ->
?TRACE("sent. remaining: ~p", [Rest]),
try_send(Socket, Rest);
Error ->
io:format("Send failed due to error ~p~n", [Error])
end;
try_send_binary(Socket, Packet);
try_send(Socket, Char) when is_integer(Char) ->
%% TODO handle unicode
?TRACE("Sending char ~p as ~p", [Char, <<Char:8>>]),
Expand All @@ -196,6 +188,20 @@ try_send_iolist(Socket, [H | T]) ->
try_send(Socket, H),
try_send_iolist(Socket, T).

try_send_binary(_Socket, <<>>) ->
ok;
try_send_binary(Socket, Packet) when is_binary(Packet) ->
ChunkSize = erlang:min(byte_size(Packet), ?MAX_SEND_CHUNK),
<<Chunk:ChunkSize/binary, Rest/binary>> = Packet,
case socket:send(Socket, Chunk) of
ok ->
try_send_binary(Socket, Rest);
{ok, Remaining} ->
try_send_binary(Socket, <<Remaining/binary, Rest/binary>>);
Error ->
io:format("Send failed due to error ~p~n", [Error])
end.

is_string([]) ->
true;
is_string([H | T]) when is_integer(H) ->
Expand Down
181 changes: 127 additions & 54 deletions src/httpd.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@
-record(state, {
config,
pending_request_map = #{},
ws_socket_map = #{}
ws_socket_map = #{},
pending_buffer_map = #{}
}).

%%
Expand Down Expand Up @@ -122,59 +123,76 @@ handle_receive(Socket, Packet, State) ->

%% @private
handle_http_request(Socket, Packet, State) ->
case maps:get(Socket, State#state.pending_request_map, undefined) of
PendingRequestMap = State#state.pending_request_map,
BufferMap = State#state.pending_buffer_map,
PendingBuffer = maps:get(Socket, BufferMap, <<>>),
AccumulatedPacket = <<PendingBuffer/binary, Packet/binary>>,
case maps:get(Socket, PendingRequestMap, undefined) of
undefined ->
HttpRequest = parse_http_request(binary_to_list(Packet)),
% ?TRACE("HttpRequest: ~p~n", [HttpRequest]),
#{
method := Method,
headers := Headers
} = HttpRequest,
case get_protocol(Method, Headers) of
http ->
case init_handler(HttpRequest, State) of
{ok, {Handler, HandlerState, PathSuffix, HandlerConfig}} ->
NewHttpRequest = HttpRequest#{
handler => Handler,
handler_state => HandlerState,
path_suffix => PathSuffix,
handler_config => HandlerConfig,
socket => Socket
},
handle_request_state(Socket, NewHttpRequest, State);
Error ->
{close, create_error(?INTERNAL_SERVER_ERROR, Error)}
end;
ws ->
?TRACE("Protocol is ws", []),
Config = State#state.config,
Path = maps:get(path, HttpRequest),
case get_handler(Path, Config) of
{ok, PathSuffix, EntryConfig} ->
WsHandler = maps:get(handler, EntryConfig),
?TRACE("Got handler ~p", [WsHandler]),
HandlerConfig = maps:get(handler_config, EntryConfig, #{}),
case WsHandler:start(Socket, PathSuffix, HandlerConfig) of
{ok, WebSocket} ->
?TRACE("Started web socket handler: ~p", [WebSocket]),
NewWebSocketMap = maps:put(Socket, WebSocket, State#state.ws_socket_map),
NewState = State#state{ws_socket_map = NewWebSocketMap},
ReplyToken = get_reply_token(maps:get(headers, HttpRequest)),
ReplyHeaders = #{"Upgrade" => "websocket", "Connection" => "Upgrade", "Sec-WebSocket-Accept" => ReplyToken},
Reply = create_reply(?SWITCHING_PROTOCOLS, ReplyHeaders, <<"">>),
?TRACE("Sending web socket upgrade reply: ~p", [Reply]),
{reply, Reply, NewState};
case maybe_parse_http_request(AccumulatedPacket) of
{more, IncompletePacket} ->
NewBufferMap = BufferMap#{Socket => IncompletePacket},
{noreply, State#state{pending_buffer_map = NewBufferMap}};
{ok, HttpRequest} ->
CleanBufferMap = maps:remove(Socket, BufferMap),
CleanState = State#state{pending_buffer_map = CleanBufferMap},
% ?TRACE("HttpRequest: ~p~n", [HttpRequest]),
#{
method := Method,
headers := Headers
} = HttpRequest,
case get_protocol(Method, Headers) of
http ->
case init_handler(HttpRequest, CleanState) of
{ok, {Handler, HandlerState, PathSuffix, HandlerConfig}} ->
NewHttpRequest = HttpRequest#{
handler => Handler,
handler_state => HandlerState,
path_suffix => PathSuffix,
handler_config => HandlerConfig,
socket => Socket
},
handle_request_state(Socket, NewHttpRequest, CleanState);
Error ->
?TRACE("Web socket error: ~p", [Error]),
{close, create_error(?INTERNAL_SERVER_ERROR, {web_socket_error, Error})}
{close, create_error(?INTERNAL_SERVER_ERROR, Error)}
end;
Error ->
Error
end
ws ->
?TRACE("Protocol is ws", []),
Config = CleanState#state.config,
Path = maps:get(path, HttpRequest),
case get_handler(Path, Config) of
{ok, PathSuffix, EntryConfig} ->
WsHandler = maps:get(handler, EntryConfig),
?TRACE("Got handler ~p", [WsHandler]),
HandlerConfig = maps:get(handler_config, EntryConfig, #{}),
case WsHandler:start(Socket, PathSuffix, HandlerConfig) of
{ok, WebSocket} ->
?TRACE("Started web socket handler: ~p", [WebSocket]),
NewWebSocketMap = maps:put(Socket, WebSocket, CleanState#state.ws_socket_map),
NewState = CleanState#state{ws_socket_map = NewWebSocketMap},
ReplyToken = get_reply_token(maps:get(headers, HttpRequest)),
ReplyHeaders = #{"Upgrade" => "websocket", "Connection" => "Upgrade", "Sec-WebSocket-Accept" => ReplyToken},
Reply = create_reply(?SWITCHING_PROTOCOLS, ReplyHeaders, <<"">>),
?TRACE("Sending web socket upgrade reply: ~p", [Reply]),
{reply, Reply, NewState};
Error ->
?TRACE("Web socket error: ~p", [Error]),
{close, create_error(?INTERNAL_SERVER_ERROR, {web_socket_error, Error})}
end;
Error ->
{close, create_error(?INTERNAL_SERVER_ERROR, {web_socket_error, Error})}
end
end;
{error, Reason} ->
{close, create_error(?BAD_REQUEST, Reason)}
end;
PendingHttpRequest ->
?TRACE("Packetlen: ~p", [erlang:byte_size(Packet)]),
handle_request_state(Socket, PendingHttpRequest#{body := Packet}, State)
ExistingBody = maps:get(body, PendingHttpRequest, <<>>),
NewBody = <<ExistingBody/binary, Packet/binary>>,
CleanBufferMap = maps:remove(Socket, BufferMap),
CleanState = State#state{pending_buffer_map = CleanBufferMap},
handle_request_state(Socket, PendingHttpRequest#{body := NewBody}, CleanState)
end.

%% @private
Expand Down Expand Up @@ -213,7 +231,7 @@ handle_request_state(Socket, HttpRequest, State) ->
{reply, Reply, State#state{pending_request_map = NewPendingRequestMap}};
wait_for_body ->
NewPendingRequestMap = PendingRequestMap#{Socket => HttpRequest},
call_http_req_handler(Socket, HttpRequest, State#state{pending_request_map = NewPendingRequestMap})
{noreply, State#state{pending_request_map = NewPendingRequestMap}}
end.

%% @private
Expand Down Expand Up @@ -290,13 +308,19 @@ update_state(Socket, HttpRequest, HandlerState, State) ->

%% @hidden
handle_tcp_closed(Socket, State) ->
case maps:get(Socket, State#state.ws_socket_map, undefined) of
NewPendingRequestMap = maps:remove(Socket, State#state.pending_request_map),
NewPendingBufferMap = maps:remove(Socket, State#state.pending_buffer_map),
CleanState = State#state{
pending_request_map = NewPendingRequestMap,
pending_buffer_map = NewPendingBufferMap
},
case maps:get(Socket, CleanState#state.ws_socket_map, undefined) of
undefined ->
State;
CleanState;
WebSocket ->
ok = httpd_ws_handler:stop(WebSocket),
NewWebSocketMap = maps:remove(Socket, State#state.ws_socket_map),
State#state{ws_socket_map = NewWebSocketMap}
NewWebSocketMap = maps:remove(Socket, CleanState#state.ws_socket_map),
CleanState#state{ws_socket_map = NewWebSocketMap}
end.

%%
Expand Down Expand Up @@ -324,6 +348,29 @@ parse_http_request(Packet) ->
}
).

maybe_parse_http_request(Packet) when is_binary(Packet) ->
case find_header_delimiter(Packet) of
nomatch ->
{more, Packet};
{_Pos, _Len} ->
try
{ok, parse_http_request(binary_to_list(Packet))}
Comment thread
UncleGrumpy marked this conversation as resolved.
catch
throw:Reason ->
{error, Reason};
error:Reason ->
{error, Reason}
end
end.

find_header_delimiter(Packet) ->
case binary:match(Packet, <<"\r\n\r\n">>) of
nomatch ->
binary:match(Packet, <<"\n\n">>);
Match ->
Match
end.

%% @private
parse_heading([$\s|Rest], start, Tmp, Accum) ->
parse_heading(Rest, start, Tmp, Accum);
Expand Down Expand Up @@ -521,21 +568,47 @@ create_error(StatusCode, Error) ->
create_reply(StatusCode, ContentType, Reply) when is_list(ContentType) orelse is_binary(ContentType) ->
create_reply(StatusCode, #{"Content-Type" => ContentType}, Reply);
create_reply(StatusCode, Headers, Reply) when is_map(Headers) ->
ReplyLen = iolist_length(Reply),
HeadersWithLen = ensure_content_length(Headers, ReplyLen),
[
<<"HTTP/1.1 ">>, erlang:integer_to_binary(StatusCode), <<" ">>, moniker(StatusCode),
<<"\r\n">>,
io_lib:format("Server: atomvm-~s\r\n", [get_version_str(erlang:system_info(atomvm_version))]),
to_headers_list(Headers),
to_headers_list(HeadersWithLen),
<<"\r\n">>,
Reply
].

%% @private
ensure_content_length(Headers, ReplyLen) ->
LenBin = erlang:integer_to_binary(ReplyLen),
CleanHeaders = remove_content_length_header(Headers),
CleanHeaders#{<<"Content-Length">> => LenBin}.

%% @private
remove_content_length_header(Headers) ->
KeysToRemove = [
"Content-Length",
<<"Content-Length">>,
"content-length",
<<"content-length">>
],
lists:foldl(fun(Key, Acc) -> maps:remove(Key, Acc) end, Headers, KeysToRemove).

%% @private
maybe_binary_to_string(Bin) when is_binary(Bin) ->
erlang:binary_to_list(Bin);
maybe_binary_to_string(Other) ->
Other.

%% @private
iolist_length(Bin) when is_binary(Bin) ->
erlang:byte_size(Bin);
iolist_length(Int) when is_integer(Int), Int >= 0, Int =< 255 ->
1;
iolist_length(List) when is_list(List) ->
erlang:length(List).
Comment thread
harmon25 marked this conversation as resolved.
Outdated

%% @private
to_headers_list(Headers) ->
[io_lib:format("~s: ~s\r\n", [maybe_binary_to_string(Key), maybe_binary_to_string(Value)]) || {Key, Value} <- maps:to_list(Headers)].
Expand Down