From 30ce264074ddd5fe8ac7a9ba1b482063b2dfa8e8 Mon Sep 17 00:00:00 2001 From: harmon25 Date: Tue, 26 May 2026 10:03:54 -0400 Subject: [PATCH 1/8] Add support for HTTP status code 405 and enhance request handling with connection limits and timeouts --- include/httpd.hrl | 1 + src/gen_tcp_server.erl | 66 ++++++++---- src/httpd.erl | 184 +++++++++++++++++++++----------- src/httpd_env_api_handler.erl | 135 +++++++++++++---------- src/httpd_handler.erl | 3 +- src/httpd_ota_handler.erl | 2 +- src/httpd_ws_handler.erl | 63 ++++++----- test/atomvm_httpd_test.exs | 10 +- test/httpd_integration_test.exs | 30 +++--- 9 files changed, 303 insertions(+), 191 deletions(-) diff --git a/include/httpd.hrl b/include/httpd.hrl index 791b784..0ac3b2a 100644 --- a/include/httpd.hrl +++ b/include/httpd.hrl @@ -18,6 +18,7 @@ -define(INTERNAL_SERVER_ERROR, 500). -define(BAD_REQUEST, 400). -define(NOT_FOUND, 404). +-define(NOT_ALLOWED, 405). -define(OK, 200). -define(CONTINUE, 100). -define(SWITCHING_PROTOCOLS, 101). diff --git a/src/gen_tcp_server.erl b/src/gen_tcp_server.erl index 8acaf10..575d5d2 100644 --- a/src/gen_tcp_server.erl +++ b/src/gen_tcp_server.erl @@ -32,14 +32,16 @@ -callback handle_receive(Socket :: term(), Packet :: binary(), State :: term()) -> {reply, Packet :: iolist(), NewState :: term()} | {noreply, NewState :: term()} | {close, Packet :: iolist()} | close. --callback handle_tcp_closed(Socket :: term(), State :: term()) -> ok. +-callback handle_tcp_closed(Socket :: term(), State :: term()) -> NewState :: term(). % -define(TRACE_ENABLED, true). -include_lib("atomvm_httpd/include/trace.hrl"). -record(state, { handler, - handler_state + handler_state, + connections = #{}, + max_connections = 0 }). -define(DEFAULT_BIND_OPTIONS, #{ @@ -78,6 +80,7 @@ stop(Server) -> %% @hidden init({BindOptions, SocketOptions, Handler, Args}) -> Self = self(), + MaxConnections = maps:get(max_connections, SocketOptions, 0), case socket:open(inet, stream, tcp) of {ok, Socket} -> ok = set_socket_options(Socket, SocketOptions), @@ -85,10 +88,10 @@ init({BindOptions, SocketOptions, Handler, Args}) -> ok -> case socket:listen(Socket) of ok -> - spawn(fun() -> accept(Self, Socket) end), + spawn_link(fun() -> accept(Self, Socket) end), case Handler:init(Args) of {ok, HandlerState} -> - {ok, #state{handler = Handler, handler_state = HandlerState}}; + {ok, #state{handler = Handler, handler_state = HandlerState, max_connections = MaxConnections}}; HandlerError -> try_close(Socket), {stop, {handler_error, HandlerError}} @@ -108,7 +111,7 @@ init({Socket, Handler, Args}) -> Self = self(), case Handler:init(Args) of {ok, HandlerState} -> - spawn(fun() -> loop(Self, Socket) end), + spawn_link(fun() -> loop(Self, Socket) end), {ok, #state{handler = Handler, handler_state = HandlerState}}; HandlerError -> {stop, {handler_error, HandlerError}} @@ -125,12 +128,43 @@ handle_cast(_Msg, State) -> %% @hidden handle_info({tcp_closed, Socket}, State) -> ?TRACE("TCP Socket closed ~p", [Socket]), - #state{handler=Handler, handler_state=HandlerState} = State, + #state{handler=Handler, handler_state=HandlerState, connections=Conns} = State, NewHandlerState = Handler:handle_tcp_closed(Socket, HandlerState), - {noreply, State#state{handler_state=NewHandlerState}}; + NewConns = maps:remove(Socket, Conns), + {noreply, State#state{handler_state=NewHandlerState, connections=NewConns}}; +handle_info({request_timeout, Socket}, State) -> + ?TRACE("Request timeout for socket ~p", [Socket]), + try_close(Socket), + {noreply, State}; handle_info({tcp, Socket, Packet}, State) -> - #state{handler=Handler, handler_state=HandlerState} = State, + #state{connections=Conns, max_connections=MaxConns} = State, ?TRACE("received packet: len(~p) from ~p", [erlang:byte_size(Packet), socket:peername(Socket)]), + case maps:is_key(Socket, Conns) of + false when MaxConns > 0, map_size(Conns) >= MaxConns -> + ?TRACE("Connection limit reached (~p), closing ~p", [MaxConns, Socket]), + try_close(Socket), + {noreply, State}; + _ -> + NewConns = case maps:is_key(Socket, Conns) of + false -> Conns#{Socket => true}; + true -> Conns + end, + handle_tcp_data(Socket, Packet, State#state{connections = NewConns}) + end; +handle_info({'EXIT', _Pid, _Reason}, State) -> + ?TRACE("Linked process ~p exited: ~p", [_Pid, _Reason]), + {noreply, State}; +handle_info(Info, State) -> + io:format("Received spurious info msg: ~p~n", [Info]), + {noreply, State}. + +%% @hidden +terminate(_Reason, _State) -> + ok. + +%% @private +handle_tcp_data(Socket, Packet, State) -> + #state{handler=Handler, handler_state=HandlerState} = State, case Handler:handle_receive(Socket, Packet, HandlerState) of {reply, ResponsePacket, ResponseState} -> ?TRACE("Sending reply to endpoint ~p", [socket:peername(Socket)]), @@ -153,7 +187,7 @@ handle_info({tcp, Socket, Packet}, State) -> ok -> try_close(Socket); {error, closed} -> - ok; %% Already closed, nothing to do + ok; {error, _Reason} -> try_close(Socket) end, @@ -166,14 +200,7 @@ handle_info({tcp, Socket, Packet}, State) -> ?TRACE("Unexpected response from handler ~p: ~p", [Handler, _SomethingElse]), try_close(Socket), {noreply, State} - end; -handle_info(Info, State) -> - io:format("Received spurious info msg: ~p~n", [Info]), - {noreply, State}. - -%% @hidden -terminate(_Reason, _State) -> - ok. + end. %% %% internal functions @@ -275,7 +302,7 @@ accept(ControllingProcess, ListenSocket) -> case socket:accept(ListenSocket) of {ok, Connection} -> ?TRACE("Accepted connection from ~p", [socket:peername(Connection)]), - spawn(fun() -> accept(ControllingProcess, ListenSocket) end), + spawn_link(fun() -> accept(ControllingProcess, ListenSocket) end), loop(ControllingProcess, Connection); _Error -> ?TRACE("Error accepting connection: ~p", [_Error]), @@ -295,6 +322,9 @@ loop(ControllingProcess, Connection) -> ?TRACE("Peer closed connection ~p", [Connection]), ControllingProcess ! {tcp_closed, Connection}, ok; + {error, timeout} -> + ?TRACE("Timeout on recv from ~p, retrying", [Connection]), + loop(ControllingProcess, Connection); {error, _SomethingElse} -> ?TRACE("Some other error occurred ~p: ~p", [Connection, _SomethingElse]), try_close(Connection) diff --git a/src/httpd.erl b/src/httpd.erl index 4d1dc76..48ca157 100644 --- a/src/httpd.erl +++ b/src/httpd.erl @@ -43,7 +43,8 @@ query_params := query_params(), headers := #{binary() := binary()}, body := binary(), - socket := term() + socket := term(), + version := binary() }. -type handler_config() :: #{ module := module(), @@ -70,7 +71,9 @@ config, pending_request_map = #{}, ws_socket_map = #{}, - pending_buffer_map = #{} + pending_buffer_map = #{}, + pending_timer_map = #{}, + request_timeout = 30000 }). %% @@ -143,7 +146,7 @@ handle_http_request(Socket, Packet, State) -> case maybe_parse_http_request(AccumulatedPacket) of {more, IncompletePacket} -> NewBufferMap = BufferMap#{Socket => IncompletePacket}, - {noreply, State#state{pending_buffer_map = NewBufferMap}}; + {noreply, start_request_timer(Socket, State#state{pending_buffer_map = NewBufferMap})}; {ok, HttpRequest} -> CleanBufferMap = maps:remove(Socket, BufferMap), CleanState = State#state{pending_buffer_map = CleanBufferMap}, @@ -152,7 +155,11 @@ handle_http_request(Socket, Packet, State) -> method := Method, headers := Headers } = HttpRequest, - case get_protocol(Method, Headers) of + case Method of + undefined -> + {close, create_error(?NOT_ALLOWED, method_not_allowed)}; + _ -> + case get_protocol(Method, Headers) of http -> case init_handler(HttpRequest, CleanState) of {ok, {Handler, HandlerState, PathSuffix, HandlerConfig}} -> @@ -169,30 +176,37 @@ handle_http_request(Socket, Packet, State) -> 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}; + Headers = maps:get(headers, HttpRequest, #{}), + case get_ws_key(Headers) of + {ok, WebSocketKey} -> + ReplyToken = get_reply_token(WebSocketKey), + 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}, + 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 -> - ?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})} + error -> + {close, create_error(?BAD_REQUEST, missing_websocket_key)} end + end end; {error, Reason} -> {close, create_error(?BAD_REQUEST, Reason)} @@ -232,28 +246,29 @@ handle_request_state(Socket, HttpRequest, State) -> complete -> ?TRACE("Request complete. Handling...", []), NewPendingRequestMap = maps:remove(Socket, PendingRequestMap), - call_http_req_handler(Socket, HttpRequest, State#state{pending_request_map = NewPendingRequestMap}); + CleanState = stop_request_timer(Socket, State#state{pending_request_map = NewPendingRequestMap}), + call_http_req_handler(Socket, HttpRequest, CleanState); expect_continue -> Headers = maps:get(headers, HttpRequest), - NewHeaders = maps:remove(<<"Expect">>, Headers), + NewHeaders = maps:remove(<<"expect">>, Headers), NewHttpRequest = HttpRequest#{headers := NewHeaders}, Reply = create_reply(?CONTINUE, #{}, <<"">>), NewPendingRequestMap = PendingRequestMap#{Socket => NewHttpRequest}, - {reply, Reply, State#state{pending_request_map = NewPendingRequestMap}}; + {reply, Reply, start_request_timer(Socket, State#state{pending_request_map = NewPendingRequestMap})}; wait_for_body -> NewPendingRequestMap = PendingRequestMap#{Socket => HttpRequest}, - {noreply, State#state{pending_request_map = NewPendingRequestMap}} + {noreply, start_request_timer(Socket, State#state{pending_request_map = NewPendingRequestMap})} end. %% @private get_request_state(HttpRequest) -> Headers = maps:get(headers, HttpRequest), - case maps:get(<<"Expect">>, Headers, undefined) of + case maps:get(<<"expect">>, Headers, undefined) of <<"100-continue">> -> ?TRACE("Expect: 100-continue", []), expect_continue; undefined -> - case maps:get(<<"Content-Length">>, Headers, undefined) of + case maps:get(<<"content-length">>, Headers, undefined) of undefined -> ?TRACE("No content length; request complete", []), complete; @@ -279,6 +294,7 @@ call_http_req_handler(Socket, HttpRequest, State) -> handler := Handler, handler_state := HandlerState } = HttpRequest, + KeepAlive = is_keep_alive(HttpRequest), case Handler:handle_http_req(HttpRequest, HandlerState) of %% noreply {noreply, NewHandlerState} -> @@ -293,11 +309,22 @@ call_http_req_handler(Socket, HttpRequest, State) -> {reply, create_reply(?OK, ReplyHeaders, Reply), NewState}; %% close close -> - {close, State}; + case KeepAlive of + true -> {reply, create_reply(?OK, #{"Content-Type" => "text/plain"}, <<"">>), State}; + false -> {close, State} + end; {close, Reply} -> - {close, create_reply(?OK, #{"Content-Type" => "application/octet-stream"}, Reply)}; + ReplyPacket = create_reply(?OK, #{"Content-Type" => "application/octet-stream"}, Reply), + case KeepAlive of + true -> {reply, ReplyPacket, State}; + false -> {close, ReplyPacket} + end; {close, ReplyHeaders, Reply} -> - {close, create_reply(?OK, ReplyHeaders, Reply)}; + ReplyPacket = create_reply(?OK, ReplyHeaders, Reply), + case KeepAlive of + true -> {reply, ReplyPacket, State}; + false -> {close, ReplyPacket} + end; %% errors {error, not_found} -> {close, create_error(?NOT_FOUND, not_found)}; @@ -309,6 +336,11 @@ call_http_req_handler(Socket, HttpRequest, State) -> {close, create_error(?INTERNAL_SERVER_ERROR, HandlerError)} end. +%% @private +is_keep_alive(HttpRequest) -> + Headers = maps:get(headers, HttpRequest, #{}), + maps:get(<<"connection">>, Headers, undefined) =:= <<"keep-alive">>. + %% @private update_state(Socket, HttpRequest, HandlerState, State) -> NewHttpRequest = HttpRequest#{handler_state := HandlerState}, @@ -321,9 +353,11 @@ update_state(Socket, HttpRequest, HandlerState, State) -> handle_tcp_closed(Socket, State) -> NewPendingRequestMap = maps:remove(Socket, State#state.pending_request_map), NewPendingBufferMap = maps:remove(Socket, State#state.pending_buffer_map), + NewTimerMap = maps:remove(Socket, State#state.pending_timer_map), CleanState = State#state{ pending_request_map = NewPendingRequestMap, - pending_buffer_map = NewPendingBufferMap + pending_buffer_map = NewPendingBufferMap, + pending_timer_map = NewTimerMap }, case maps:get(Socket, CleanState#state.ws_socket_map, undefined) of undefined -> @@ -339,8 +373,13 @@ handle_tcp_closed(Socket, State) -> %% %% @private -get_reply_token(Headers) -> - #{<<"Sec-WebSocket-Key">> := WebSocketKey} = Headers, +get_ws_key(#{<<"sec-websocket-key">> := Key}) -> + {ok, Key}; +get_ws_key(_) -> + error. + +%% @private +get_reply_token(WebSocketKey) -> MagicKey = <<"258EAFA5-E914-47DA-95CA-C5AB0DC85B11">>, PreImage = <>, ReplyToken = base64:encode(crypto:hash(sha, PreImage)), @@ -348,14 +387,14 @@ get_reply_token(Headers) -> ReplyToken. %% @private -parse_http_request(Packet) -> - {Heading, HeadingRest} = parse_heading(Packet, start, [], #{}), - {Headers, Body} = parse_header(HeadingRest, #{}), +parse_http_request(HeadingList, Body) -> + {Heading, _HeadingRest} = parse_heading(HeadingList, start, [], #{}), + {Headers, _} = parse_header(_HeadingRest, #{}), maps:merge( Heading, #{ headers => Headers, - body => erlang:list_to_binary(Body) + body => Body } ). @@ -363,9 +402,11 @@ maybe_parse_http_request(Packet) when is_binary(Packet) -> case find_header_delimiter(Packet) of nomatch -> {more, Packet}; - {_Pos, _Len} -> + {Pos, Len} -> try - {ok, parse_http_request(binary_to_list(Packet))} + HeaderEnd = Pos + Len, + <> = Packet, + {ok, parse_http_request(binary_to_list(HeadingPart), Body)} catch throw:Reason -> {error, Reason}; @@ -410,8 +451,13 @@ parse_heading([$\s|Rest], wait_version, Tmp, Accum) -> parse_heading(Packet, wait_version, Tmp, Accum) -> parse_heading(Packet, in_version, Tmp, Accum); %% in_version state -parse_heading([$\n|Rest], in_version, _Tmp, Accum) -> - {Accum, Rest}; +parse_heading([$\n|Rest], in_version, Tmp, Accum) -> + RawVersion = lists:reverse(Tmp), + Version = case RawVersion of + [$\r | Clean] -> list_to_binary(Clean); + _ -> list_to_binary(RawVersion) + end, + {Accum#{version => Version}, Rest}; parse_heading([C|Rest], in_version, Tmp, Accum) -> parse_heading(Rest, in_version, [C|Tmp], Accum); %% error state @@ -439,9 +485,12 @@ parse_line(_Packet, _Accum) -> %% @private split_header(Header) -> - [Key, Value] = string:split(Header, ":"), - %% TODO to_lower the key - {list_to_binary(string:trim(Key)), list_to_binary(string:trim(Value))}. + case string:split(Header, ":") of + [Key, Value] -> + {list_to_binary(string:to_lower(string:trim(Key))), list_to_binary(string:trim(Value))}; + _ -> + throw(bad_header) + end. normalize_uri(Uri) -> case string:split(Uri, "?", leading) of @@ -458,8 +507,15 @@ tokenize_path(Path) -> %% @private parse_query_params(QueryParamString) -> NVPairsStrings = string:split(QueryParamString, "&", all), - NVPairLists = [string:split(NVPairString, "=") || NVPairString <- NVPairsStrings], - maps:from_list([{list_to_atom(Key), url_decode(Value, [])} || [Key, Value] <- NVPairLists]). + maps:from_list([parse_query_param(NVPairString) || NVPairString <- NVPairsStrings]). + +parse_query_param(NVPairString) -> + case string:split(NVPairString, "=") of + [Key] -> + {list_to_binary(Key), <<"">>}; + [Key, Value] -> + {list_to_binary(Key), url_decode(Value, [])} + end. % from https://docs.microfocus.com/OMi/10.62/Content/OMi/ExtGuide/ExtApps/URL_encoding.htm url_decode([], Accum) -> @@ -558,7 +614,7 @@ starts_with([_H1|_], [_H2|_]) -> %% @private -get_protocol(get, #{<<"Upgrade">> := <<"websocket">>, <<"Connection">> := Upgrade, <<"Sec-WebSocket-Key">> := _, <<"Sec-WebSocket-Version">> := <<"13">>} = _Headers) -> +get_protocol(get, #{<<"upgrade">> := <<"websocket">>, <<"connection">> := Upgrade, <<"sec-websocket-key">> := _, <<"sec-websocket-version">> := <<"13">>} = _Headers) -> case str(string:to_upper(binary_to_list(Upgrade)), "UPGRADE") of 0 -> http; @@ -593,18 +649,8 @@ create_reply(StatusCode, Headers, Reply) when is_map(Headers) -> %% @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). + CleanHeaders = maps:remove(<<"content-length">>, Headers), + CleanHeaders#{<<"content-length">> => LenBin}. %% @private maybe_binary_to_string(Bin) when is_binary(Bin) -> @@ -640,6 +686,8 @@ moniker(?BAD_REQUEST) -> <<"BAD_REQUEST">>; moniker(?NOT_FOUND) -> <<"NOT_FOUND">>; +moniker(?NOT_ALLOWED) -> + <<"METHOD_NOT_ALLOWED">>; moniker(?CONTINUE) -> <<"Continue">>; moniker(?SWITCHING_PROTOCOLS) -> @@ -658,3 +706,15 @@ method_to_atom("DELETE") -> delete; method_to_atom(_) -> undefined. + +%% @private +start_request_timer(Socket, State) -> + Timeout = State#state.request_timeout, + TimerRef = erlang:send_after(Timeout, self(), {request_timeout, Socket}), + TimerMap = State#state.pending_timer_map, + State#state{pending_timer_map = TimerMap#{Socket => TimerRef}}. + +%% @private +stop_request_timer(Socket, State) -> + NewTimerMap = maps:remove(Socket, State#state.pending_timer_map), + State#state{pending_timer_map = NewTimerMap}. diff --git a/src/httpd_env_api_handler.erl b/src/httpd_env_api_handler.erl index 8d2df4b..976e4cb 100644 --- a/src/httpd_env_api_handler.erl +++ b/src/httpd_env_api_handler.erl @@ -31,64 +31,69 @@ handle_api_request(get, [Application, Param | Rest], _HttpRequest, _Args) -> ?TRACE("Application: ~p Param: ~p, Rest: ~p", [Application, Param, Rest]), - ApplicationAtom = bin_to_atom(Application), - ParamAtom = bin_to_atom(Param), - Result = case avm_application:get_env(ApplicationAtom, ParamAtom) of - undefined -> - undefined; - {ok, Value} -> - find_value_in_path(Value, Rest) - end, - case Result of - undefined -> - {error, not_found}; - _ -> - {ok, Result} + case to_existing_atoms(Application, Param) of + {ok, ApplicationAtom, ParamAtom} -> + Result = case avm_application:get_env(ApplicationAtom, ParamAtom) of + undefined -> + undefined; + {ok, Value} -> + find_value_in_path(Value, Rest) + end, + case Result of + undefined -> + {error, not_found}; + _ -> + {ok, Result} + end; + error -> + {error, not_found} end; handle_api_request(post, [Application, Param | Rest], HttpRequest, _Args) -> ?TRACE("Application: ~p Param: ~p, Rest: ~p", [Application, Param, Rest]), - QueryParams = maps:get(query_params, HttpRequest, #{}), - ?TRACE("QueryParams: ~p", [QueryParams]), - - ApplicationAtom = bin_to_atom(Application), - ParamAtom = bin_to_atom(Param), - - NewValue = create_value(Rest, QueryParams, #{}), - ?TRACE("NewValue: ~p", [NewValue]), - MergedValue = case avm_application:get_env(ApplicationAtom, ParamAtom) of - undefined -> - NewValue; - {ok, OldValue} -> - ?TRACE("merging OldValue: ~p NewValue: ~p", [OldValue, NewValue]), - map_utils:deep_maps_merge(OldValue, NewValue) - end, - - ?TRACE("QueryParams: ~p MergedValue: ~p", [QueryParams, MergedValue]), - ok = avm_application:set_env(ApplicationAtom, ParamAtom, MergedValue, [{persistent, true}]); + case to_existing_atoms(Application, Param) of + {ok, ApplicationAtom, ParamAtom} -> + QueryParams = maps:get(query_params, HttpRequest, #{}), + ?TRACE("QueryParams: ~p", [QueryParams]), + + NewValue = create_value(Rest, QueryParams, #{}), + ?TRACE("NewValue: ~p", [NewValue]), + MergedValue = case avm_application:get_env(ApplicationAtom, ParamAtom) of + undefined -> + NewValue; + {ok, OldValue} -> + ?TRACE("merging OldValue: ~p NewValue: ~p", [OldValue, NewValue]), + map_utils:deep_maps_merge(OldValue, NewValue) + end, + + ?TRACE("QueryParams: ~p MergedValue: ~p", [QueryParams, MergedValue]), + ok = avm_application:set_env(ApplicationAtom, ParamAtom, MergedValue, [{persistent, true}]); + error -> + {error, not_found} + end; handle_api_request(delete, [Application, Param | Rest], _HttpRequest, _Args) -> ?TRACE("Application: ~p Param: ~p, Rest: ~p", [Application, Param, Rest]), - ApplicationAtom = bin_to_atom(Application), - ParamAtom = bin_to_atom(Param), - Result = case avm_application:get_env(ApplicationAtom, ParamAtom) of - undefined -> - undefined; - {ok, Env} -> - %% TODO memory leak - Path = [bin_to_atom(P) || P <- Rest], - ?TRACE("Removing path ~p from env ~p", [Path, Env]), - map_utils:remove_entry_in_path(Env, Path) - end, - case Result of - undefined -> - {error, not_found}; - NewEnv -> - ?TRACE("NewEnv: ~p", [NewEnv]), - avm_application:set_env(ApplicationAtom, ParamAtom, NewEnv), - ok + case to_existing_atoms(Application, Param) of + {ok, ApplicationAtom, ParamAtom} -> + Result = case avm_application:get_env(ApplicationAtom, ParamAtom) of + undefined -> + undefined; + {ok, Env} -> + map_utils:remove_entry_in_path(Env, [binary_to_list(P) || P <- Rest]) + end, + case Result of + undefined -> + {error, not_found}; + NewEnv -> + ?TRACE("NewEnv: ~p", [NewEnv]), + avm_application:set_env(ApplicationAtom, ParamAtom, NewEnv), + ok + end; + error -> + {error, not_found} end; handle_api_request(Method, Path, _HttpRequest, _Args) -> @@ -98,21 +103,37 @@ handle_api_request(Method, Path, _HttpRequest, _Args) -> find_value_in_path(Map, []) -> Map; find_value_in_path(Value, [H | T]) when is_map(Value) -> - %% TODO binary to atom here is bad - case maps:get(bin_to_atom(H), Value, undefined) of + case maps:get(H, Value, undefined) of undefined -> - undefined; + case to_existing_atom(H) of + {ok, Atom} -> find_value_in_path(maps:get(Atom, Value, undefined), T); + error -> undefined + end; V -> find_value_in_path(V, T) end; find_value_in_path(_Value, _Path) -> undefined. -bin_to_atom(Bin) -> - list_to_atom(binary_to_list(Bin)). - create_value([], QueryParams, Accum) -> maps:merge(Accum, QueryParams); create_value([H | T], QueryParams, Accum) -> - %% TODO binary to atom here is bad - #{bin_to_atom(H) => create_value(T, QueryParams, Accum)}. + #{H => create_value(T, QueryParams, Accum)}. + +to_existing_atoms(A, B) -> + case to_existing_atom(A) of + {ok, AtomA} -> + case to_existing_atom(B) of + {ok, AtomB} -> {ok, AtomA, AtomB}; + error -> error + end; + error -> + error + end. + +to_existing_atom(Bin) -> + try list_to_existing_atom(binary_to_list(Bin)) of + Atom -> {ok, Atom} + catch + error:badarg -> error + end. diff --git a/src/httpd_handler.erl b/src/httpd_handler.erl index d6a1ba0..dab32f8 100644 --- a/src/httpd_handler.erl +++ b/src/httpd_handler.erl @@ -31,7 +31,8 @@ method => http_method(), path => http_path(), headers => http_headers(), - body => binary() + body => binary(), + version => binary() }. %% diff --git a/src/httpd_ota_handler.erl b/src/httpd_ota_handler.erl index 45b5475..baad923 100644 --- a/src/httpd_ota_handler.erl +++ b/src/httpd_ota_handler.erl @@ -76,4 +76,4 @@ handle_http_req(_HttpRequest, _State) -> get_content_length(Headers) -> %% TODO handle case - erlang:binary_to_integer(maps:get(<<"Content-Length">>, Headers, <<"0">>)). + erlang:binary_to_integer(maps:get(<<"content-length">>, Headers, <<"0">>)). diff --git a/src/httpd_ws_handler.erl b/src/httpd_ws_handler.erl index 0493503..1fa6f01 100644 --- a/src/httpd_ws_handler.erl +++ b/src/httpd_ws_handler.erl @@ -22,7 +22,7 @@ -export([send/2]). -behavior(gen_server). --export([init/1, handle_cast/2, handle_call/3, handle_info/2, terminate/2]). +-export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2]). % -define(TRACE_ENABLED, true). -include_lib("atomvm_httpd/include/trace.hrl"). @@ -65,12 +65,7 @@ handle_web_socket_message(WebSocket, Packet) -> gen_server:cast(WebSocket, {message, Packet}). send(WebSocket, Packet) -> - case self() of - WebSocket -> - throw(badarg); - _ -> - gen_server:call(WebSocket, {send, Packet}) - end. + gen_server:cast(WebSocket, {send, Packet}). %% @@ -138,13 +133,17 @@ handle_cast({message, Packet}, State) -> ?TRACE("ParseFrameError: ~p", [ParseFrameError]), socket:close(Socket), {stop, ParseFrameError, State} - end. - + end; %% @hidden -handle_call({send, Packet}, _From, State) -> +handle_cast({send, Packet}, State) -> ?TRACE("Sending packet ~p", [Packet]), - Reply = do_send(State#state.socket, Packet, text), - {reply, Reply, State}. + do_send(State#state.socket, Packet, text), + {noreply, State}. + +%% @hidden +handle_call(_Request, _From, State) -> + {reply, ok, State}. + %% @hidden handle_info(_Msg, State) -> @@ -242,26 +241,26 @@ extract_payload(Mask, PayloadLen, Data) -> end. %% @private -unmask(MaskingKey, MaskedPayload) -> - unmask(MaskingKey, MaskedPayload, 0, []). - -unmask(_MaskingKey, <<"">>, _I, Accum) -> - % ?TRACE("unmasked Accum: ~p", [Accum]), - list_to_binary(lists:reverse(Accum)); -unmask(MaskingKey, <>, I, Accum) -> - MaskingOctet = octet(MaskingKey, I rem 4), - % ?TRACE("H: ~p, MaskingOctet: ~p", [H, MaskingOctet]), - unmask(MaskingKey, T, I + 1, [MaskingOctet bxor H | Accum]). +unmask(<>, Payload) -> + Size = byte_size(Payload), + FullChunks = Size bsr 2, + Rem = Size band 3, + case Rem of + 0 -> + << <<(A bxor K0), (B bxor K1), (C bxor K2), (D bxor K3)>> || + <> <= Payload >>; + _ -> + ChunkSize = FullChunks bsl 2, + <> = Payload, + Unmasked = << <<(A bxor K0), (B bxor K1), (C bxor K2), (D bxor K3)>> || + <> <= ChunkedPart >>, + <> + end. -%% @private -octet(<>, 0) -> - First; -octet(<<_:1/binary, Second:8, _/binary>>, 1) -> - Second; -octet(<<_:2/binary, Third:8, _/binary>>, 2) -> - Third; -octet(<<_:3/binary, Fourth:8, _/binary>>, 3) -> - Fourth. +unmask_rem(<<>>, _, _, _, _, 0) -> <<>>; +unmask_rem(<>, K0, _, _, _, 1) -> <<(B bxor K0)>>; +unmask_rem(<>, K0, K1, _, _, 2) -> <<(B bxor K0), (C bxor K1)>>; +unmask_rem(<>, K0, K1, K2, _, 3) -> <<(B bxor K0), (C bxor K1), (D bxor K2)>>. %% @private do_send(Socket, Packet, Mode) -> @@ -277,7 +276,7 @@ frame(Packet, Mode) when is_binary(Packet) -> Opcode = case Mode of text -> 16#01; binary -> 16#02; _ -> 16#01 end, FinOpcode = Fin bor Opcode, PayloadLen = erlang:byte_size(Packet), - case {PayloadLen =< 125, PayloadLen =< 65536} of + case {PayloadLen =< 125, PayloadLen < 65536} of {true, _} -> NoMask = 16#7F, MaskLen = NoMask band PayloadLen, diff --git a/test/atomvm_httpd_test.exs b/test/atomvm_httpd_test.exs index a51c2aa..057bfe7 100644 --- a/test/atomvm_httpd_test.exs +++ b/test/atomvm_httpd_test.exs @@ -13,7 +13,7 @@ defmodule HttpdUnitTest do assert :post = Map.fetch!(http_request, :method) headers = Map.fetch!(http_request, :headers) - assert <<"11">> = Map.fetch!(headers, <<"Content-Length">>) + assert <<"11">> = Map.fetch!(headers, <<"content-length">>) assert <<"hello=world">> = Map.fetch!(http_request, :body) end @@ -23,15 +23,15 @@ defmodule HttpdUnitTest do assert {:ok, http_request} = :httpd.maybe_parse_http_request(request) headers = Map.fetch!(http_request, :headers) - assert <<"value200">> = Map.fetch!(headers, <<"X-Test-200">>) + assert <<"value200">> = Map.fetch!(headers, <<"x-test-200">>) end test "handle_request_state stores partial body until complete" do socket = make_ref() - http_request = %{headers: %{<<"Content-Length">> => <<"5">>}, body: <<"12">>} - state = {:state, [], %{}, %{}, %{}} + http_request = %{headers: %{<<"content-length">> => <<"5">>}, body: <<"12">>} + state = {:state, [], %{}, %{}, %{}, %{}, 30000} - assert {:noreply, {:state, [], %{^socket => ^http_request}, %{}, %{}}} = + assert {:noreply, {:state, [], %{^socket => ^http_request}, %{}, %{}, %{}, 30000}} = :httpd.handle_request_state(socket, http_request, state) assert :wait_for_body = :httpd.get_request_state(http_request) diff --git a/test/httpd_integration_test.exs b/test/httpd_integration_test.exs index 496e2a6..1146bb0 100644 --- a/test/httpd_integration_test.exs +++ b/test/httpd_integration_test.exs @@ -38,7 +38,7 @@ defmodule HttpdIntegrationTest do try do request_chunks = [ - "POST / HTTP/1.1\r\nHost: example.com\r\nContent-Length: 11\r\n\r\nhe", + "POST / HTTP/1.1\r\nHost: example.com\r\ncontent-length: 11\r\n\r\nhe", "llo=", "world" ] @@ -69,7 +69,7 @@ defmodule HttpdIntegrationTest do assert_receive {:http_request, request}, @receive_timeout headers = Map.fetch!(request, :headers) - assert <<"value123">> = Map.fetch!(headers, <<"X-Custom-Header">>) + assert <<"value123">> = Map.fetch!(headers, <<"x-custom-header">>) assert {:ok, response} = :gen_tcp.recv(socket, 0, @receive_timeout) assert response =~ "HTTP/1.1 200 OK" @@ -114,7 +114,7 @@ defmodule HttpdIntegrationTest do [headers, body] = :binary.split(response, <<"\r\n\r\n">>) assert String.contains?(headers, "HTTP/1.1 200 OK") - assert String.contains?(headers, "Content-Length: " <> @large_iolist_len) + assert String.contains?(headers, "content-length: " <> @large_iolist_len) assert byte_size(body) == :erlang.iolist_size(@large_iolist) after :gen_tcp.close(socket) @@ -140,8 +140,8 @@ defmodule HttpdIntegrationTest do assert String.contains?(headers, "HTTP/1.1 200 OK") expected_length = :erlang.iolist_size(iolist) - assert String.contains?(headers, "Content-Length: #{expected_length}"), - "Expected Content-Length: #{expected_length}" + assert String.contains?(headers, "content-length: #{expected_length}"), + "Expected content-length: #{expected_length}" assert body == expected_body assert byte_size(body) == expected_length @@ -169,8 +169,8 @@ defmodule HttpdIntegrationTest do assert String.contains?(headers, "HTTP/1.1 200 OK") expected_length = :erlang.iolist_size(iolist) - assert String.contains?(headers, "Content-Length: #{expected_length}"), - "Expected Content-Length: #{expected_length}" + assert String.contains?(headers, "content-length: #{expected_length}"), + "Expected content-length: #{expected_length}" assert body == expected_body assert byte_size(body) == expected_length @@ -198,8 +198,8 @@ defmodule HttpdIntegrationTest do assert String.contains?(headers, "HTTP/1.1 200 OK") expected_length = :erlang.iolist_size(iolist) - assert String.contains?(headers, "Content-Length: #{expected_length}"), - "Expected Content-Length: #{expected_length}" + assert String.contains?(headers, "content-length: #{expected_length}"), + "Expected content-length: #{expected_length}" assert body == expected_body assert byte_size(body) == expected_length @@ -227,8 +227,8 @@ defmodule HttpdIntegrationTest do assert String.contains?(headers, "HTTP/1.1 200 OK") expected_length = :erlang.iolist_size(iolist) - assert String.contains?(headers, "Content-Length: #{expected_length}"), - "Expected Content-Length: #{expected_length}" + assert String.contains?(headers, "content-length: #{expected_length}"), + "Expected content-length: #{expected_length}" assert body == expected_body assert byte_size(body) == expected_length @@ -256,8 +256,8 @@ defmodule HttpdIntegrationTest do assert String.contains?(headers, "HTTP/1.1 200 OK") expected_length = :erlang.iolist_size(iolist) - assert String.contains?(headers, "Content-Length: #{expected_length}"), - "Expected Content-Length: #{expected_length}" + assert String.contains?(headers, "content-length: #{expected_length}"), + "Expected content-length: #{expected_length}" assert body == expected_body assert byte_size(body) == expected_length @@ -285,8 +285,8 @@ defmodule HttpdIntegrationTest do assert String.contains?(headers, "HTTP/1.1 200 OK") expected_length = :erlang.iolist_size(iolist) - assert String.contains?(headers, "Content-Length: #{expected_length}"), - "Expected Content-Length: #{expected_length}" + assert String.contains?(headers, "content-length: #{expected_length}"), + "Expected content-length: #{expected_length}" assert body == expected_body assert byte_size(body) == expected_length From 489abc895bd1765798932dcf02ee959e900d1dd3 Mon Sep 17 00:00:00 2001 From: harmon25 Date: Mon, 1 Jun 2026 20:50:05 -0400 Subject: [PATCH 2/8] Address PR review comments: timer leaks, conn tracking, close semantics, binary types - gen_tcp_server: strip max_connections from SocketOptions before passing to set_socket_options/2 to prevent socket:setopt/3 crashing on unknown key - gen_tcp_server: track connections at accept time (not first-packet) by sending {new_connection, Socket} immediately after accept; close socket there if max_connections is exceeded, closing the idle-socket flood gap - httpd: cancel pre-existing timer ref in start_request_timer/2 before creating a new one to prevent stale {request_timeout, Socket} messages on keep-alive - httpd: cancel timer in stop_request_timer/2 (not just remove map entry) - httpd: cancel timer via stop_request_timer/2 in handle_tcp_closed/2 so the timer cannot fire after the socket is gone - httpd: honour handler {close, ...} return unconditionally regardless of client keep-alive header, preserving the documented httpd_handler contract - httpd: convert url_decode/2 charlist result to binary in parse_query_param/1 so all query_params() values are binaries as declared in the type spec - httpd_env_api_handler: drop binary_to_list/1 in DELETE path so segment keys remain binaries, consistent with the binary keys stored by POST/GET - test: update handle_request_state assertion to expect a timer ref in pending_timer_map instead of an empty map --- src/gen_tcp_server.erl | 34 ++++++++++-------- src/httpd.erl | 67 +++++++++++++++++++---------------- src/httpd_env_api_handler.erl | 2 +- test/atomvm_httpd_test.exs | 14 ++++++-- 4 files changed, 70 insertions(+), 47 deletions(-) diff --git a/src/gen_tcp_server.erl b/src/gen_tcp_server.erl index 575d5d2..bbfba49 100644 --- a/src/gen_tcp_server.erl +++ b/src/gen_tcp_server.erl @@ -81,9 +81,12 @@ stop(Server) -> init({BindOptions, SocketOptions, Handler, Args}) -> Self = self(), MaxConnections = maps:get(max_connections, SocketOptions, 0), + %% Strip max_connections before passing to set_socket_options/2 so that + %% socket:setopt/3 is never called with an unknown option key. + CleanSocketOptions = maps:remove(max_connections, SocketOptions), case socket:open(inet, stream, tcp) of {ok, Socket} -> - ok = set_socket_options(Socket, SocketOptions), + ok = set_socket_options(Socket, CleanSocketOptions), case socket:bind(Socket, BindOptions) of ok -> case socket:listen(Socket) of @@ -126,6 +129,17 @@ handle_cast(_Msg, State) -> {noreply, State}. %% @hidden +handle_info({new_connection, Socket}, State) -> + #state{connections=Conns, max_connections=MaxConns} = State, + case MaxConns > 0 andalso map_size(Conns) >= MaxConns of + true -> + ?TRACE("Connection limit reached (~p), rejecting ~p at accept", [MaxConns, Socket]), + try_close(Socket), + {noreply, State}; + false -> + ?TRACE("Tracking new connection ~p (~p/~p)", [Socket, map_size(Conns) + 1, MaxConns]), + {noreply, State#state{connections = Conns#{Socket => true}}} + end; handle_info({tcp_closed, Socket}, State) -> ?TRACE("TCP Socket closed ~p", [Socket]), #state{handler=Handler, handler_state=HandlerState, connections=Conns} = State, @@ -137,20 +151,8 @@ handle_info({request_timeout, Socket}, State) -> try_close(Socket), {noreply, State}; handle_info({tcp, Socket, Packet}, State) -> - #state{connections=Conns, max_connections=MaxConns} = State, ?TRACE("received packet: len(~p) from ~p", [erlang:byte_size(Packet), socket:peername(Socket)]), - case maps:is_key(Socket, Conns) of - false when MaxConns > 0, map_size(Conns) >= MaxConns -> - ?TRACE("Connection limit reached (~p), closing ~p", [MaxConns, Socket]), - try_close(Socket), - {noreply, State}; - _ -> - NewConns = case maps:is_key(Socket, Conns) of - false -> Conns#{Socket => true}; - true -> Conns - end, - handle_tcp_data(Socket, Packet, State#state{connections = NewConns}) - end; + handle_tcp_data(Socket, Packet, State); handle_info({'EXIT', _Pid, _Reason}, State) -> ?TRACE("Linked process ~p exited: ~p", [_Pid, _Reason]), {noreply, State}; @@ -302,6 +304,10 @@ accept(ControllingProcess, ListenSocket) -> case socket:accept(ListenSocket) of {ok, Connection} -> ?TRACE("Accepted connection from ~p", [socket:peername(Connection)]), + %% Notify controlling process immediately so max_connections is enforced + %% at accept time (before any data arrives). The controlling process may + %% close the socket if the limit is exceeded; loop/2 will detect the close. + ControllingProcess ! {new_connection, Connection}, spawn_link(fun() -> accept(ControllingProcess, ListenSocket) end), loop(ControllingProcess, Connection); _Error -> diff --git a/src/httpd.erl b/src/httpd.erl index 48ca157..2e375e3 100644 --- a/src/httpd.erl +++ b/src/httpd.erl @@ -294,37 +294,29 @@ call_http_req_handler(Socket, HttpRequest, State) -> handler := Handler, handler_state := HandlerState } = HttpRequest, - KeepAlive = is_keep_alive(HttpRequest), case Handler:handle_http_req(HttpRequest, HandlerState) of %% noreply {noreply, NewHandlerState} -> NewState = update_state(Socket, HttpRequest, NewHandlerState, State), {noreply, NewState}; - %% reply + %% reply — handler wants to keep the connection open; honour keep-alive if requested {reply, Reply, NewHandlerState} -> NewState = update_state(Socket, HttpRequest, NewHandlerState, State), {reply, create_reply(?OK, #{"Content-Type" => "application/octet-stream"}, Reply), NewState}; {reply, ReplyHeaders, Reply, NewHandlerState} -> NewState = update_state(Socket, HttpRequest, NewHandlerState, State), {reply, create_reply(?OK, ReplyHeaders, Reply), NewState}; - %% close + %% close — handler explicitly requests connection close; always honour it + %% regardless of the client's keep-alive preference, preserving the documented + %% httpd_handler contract that {close, ...} means "send response, close connection". close -> - case KeepAlive of - true -> {reply, create_reply(?OK, #{"Content-Type" => "text/plain"}, <<"">>), State}; - false -> {close, State} - end; + {close, State}; {close, Reply} -> ReplyPacket = create_reply(?OK, #{"Content-Type" => "application/octet-stream"}, Reply), - case KeepAlive of - true -> {reply, ReplyPacket, State}; - false -> {close, ReplyPacket} - end; + {close, ReplyPacket}; {close, ReplyHeaders, Reply} -> ReplyPacket = create_reply(?OK, ReplyHeaders, Reply), - case KeepAlive of - true -> {reply, ReplyPacket, State}; - false -> {close, ReplyPacket} - end; + {close, ReplyPacket}; %% errors {error, not_found} -> {close, create_error(?NOT_FOUND, not_found)}; @@ -336,11 +328,6 @@ call_http_req_handler(Socket, HttpRequest, State) -> {close, create_error(?INTERNAL_SERVER_ERROR, HandlerError)} end. -%% @private -is_keep_alive(HttpRequest) -> - Headers = maps:get(headers, HttpRequest, #{}), - maps:get(<<"connection">>, Headers, undefined) =:= <<"keep-alive">>. - %% @private update_state(Socket, HttpRequest, HandlerState, State) -> NewHttpRequest = HttpRequest#{handler_state := HandlerState}, @@ -351,13 +338,15 @@ update_state(Socket, HttpRequest, HandlerState, State) -> %% @hidden handle_tcp_closed(Socket, State) -> - NewPendingRequestMap = maps:remove(Socket, State#state.pending_request_map), - NewPendingBufferMap = maps:remove(Socket, State#state.pending_buffer_map), - NewTimerMap = maps:remove(Socket, State#state.pending_timer_map), - CleanState = State#state{ + %% Cancel any pending request timer so it cannot fire after the socket is gone + %% and deliver a stale {request_timeout, Socket} message that might accidentally + %% close a future connection reusing the same socket term. + TimerCancelledState = stop_request_timer(Socket, State), + NewPendingRequestMap = maps:remove(Socket, TimerCancelledState#state.pending_request_map), + NewPendingBufferMap = maps:remove(Socket, TimerCancelledState#state.pending_buffer_map), + CleanState = TimerCancelledState#state{ pending_request_map = NewPendingRequestMap, - pending_buffer_map = NewPendingBufferMap, - pending_timer_map = NewTimerMap + pending_buffer_map = NewPendingBufferMap }, case maps:get(Socket, CleanState#state.ws_socket_map, undefined) of undefined -> @@ -514,7 +503,9 @@ parse_query_param(NVPairString) -> [Key] -> {list_to_binary(Key), <<"">>}; [Key, Value] -> - {list_to_binary(Key), url_decode(Value, [])} + %% url_decode/2 returns a charlist; convert to binary so all + %% query param values are binaries as declared in query_params(). + {list_to_binary(Key), list_to_binary(url_decode(Value, []))} end. % from https://docs.microfocus.com/OMi/10.62/Content/OMi/ExtGuide/ExtApps/URL_encoding.htm @@ -710,11 +701,27 @@ method_to_atom(_) -> %% @private start_request_timer(Socket, State) -> Timeout = State#state.request_timeout, - TimerRef = erlang:send_after(Timeout, self(), {request_timeout, Socket}), TimerMap = State#state.pending_timer_map, + %% Cancel any pre-existing timer for this socket to avoid stale timeout + %% messages being delivered to a keep-alive connection. + case maps:get(Socket, TimerMap, undefined) of + undefined -> ok; + OldRef -> + erlang:cancel_timer(OldRef), + %% Flush a stale timeout message that may already be in the mailbox. + receive {request_timeout, Socket} -> ok after 0 -> ok end + end, + TimerRef = erlang:send_after(Timeout, self(), {request_timeout, Socket}), State#state{pending_timer_map = TimerMap#{Socket => TimerRef}}. %% @private stop_request_timer(Socket, State) -> - NewTimerMap = maps:remove(Socket, State#state.pending_timer_map), - State#state{pending_timer_map = NewTimerMap}. + TimerMap = State#state.pending_timer_map, + %% Cancel the timer so it never fires after the request completes. + case maps:get(Socket, TimerMap, undefined) of + undefined -> ok; + Ref -> + erlang:cancel_timer(Ref), + receive {request_timeout, Socket} -> ok after 0 -> ok end + end, + State#state{pending_timer_map = maps:remove(Socket, TimerMap)}. diff --git a/src/httpd_env_api_handler.erl b/src/httpd_env_api_handler.erl index 976e4cb..c251e55 100644 --- a/src/httpd_env_api_handler.erl +++ b/src/httpd_env_api_handler.erl @@ -82,7 +82,7 @@ handle_api_request(delete, [Application, Param | Rest], _HttpRequest, _Args) -> undefined -> undefined; {ok, Env} -> - map_utils:remove_entry_in_path(Env, [binary_to_list(P) || P <- Rest]) + map_utils:remove_entry_in_path(Env, Rest) end, case Result of undefined -> diff --git a/test/atomvm_httpd_test.exs b/test/atomvm_httpd_test.exs index 057bfe7..98f7d84 100644 --- a/test/atomvm_httpd_test.exs +++ b/test/atomvm_httpd_test.exs @@ -31,8 +31,18 @@ defmodule HttpdUnitTest do http_request = %{headers: %{<<"content-length">> => <<"5">>}, body: <<"12">>} state = {:state, [], %{}, %{}, %{}, %{}, 30000} - assert {:noreply, {:state, [], %{^socket => ^http_request}, %{}, %{}, %{}, 30000}} = - :httpd.handle_request_state(socket, http_request, state) + assert {:noreply, result_state} = :httpd.handle_request_state(socket, http_request, state) + + # Destructure the result state tuple: {state, config, pending_request_map, + # ws_socket_map, pending_buffer_map, pending_timer_map, request_timeout} + {:state, _config, pending_request_map, _ws, _buf, pending_timer_map, _timeout} = result_state + + # Partial request should be stored in the pending map + assert %{^socket => ^http_request} = pending_request_map + + # A request timer should have been started for the socket + assert is_reference(Map.get(pending_timer_map, socket)), + "expected a timer ref in pending_timer_map for the socket" assert :wait_for_body = :httpd.get_request_state(http_request) end From aae97d18a8bf419666a8858f06009f30f23ac76f Mon Sep 17 00:00:00 2001 From: harmon25 Date: Mon, 1 Jun 2026 23:33:23 -0400 Subject: [PATCH 3/8] Address second round of PR review comments ensure_content_length: normalize response headers to lowercase binary before writing Content-Length to prevent duplicate headers when callers pass mixed-case or string keys (e.g. 'Content-Length', <<"Content-Length">>). Add normalize_headers/1 and normalize_header_key/1 helpers. Update the WebSocket handshake test to expect lowercase header names in the response. keep-alive comment: replace misleading 'honour keep-alive if requested' with an accurate note that {reply,...} always keeps the socket open; defer proper Connection/version-aware negotiation to a follow-up PR. Race-free request timeout: - Change timeout message tag from {request_timeout, Socket} to {request_timeout, Socket, Tag} where Tag = make_ref() is unique per timer. - Store {TimerRef, Tag} in pending_timer_map instead of bare TimerRef. - Remove receive-flush from start_request_timer/2 and stop_request_timer/2; stale messages are now safely ignored in handle_info/2 via Tag validation. - Add optional handle_info/2 callback to gen_tcp_server behaviour; the catch-all handle_info clause forwards unknown messages to the handler when it exports handle_info/2, expecting {noreply,S} or {close,Socket,S}. - Remove the old {request_timeout,Socket} clause from gen_tcp_server. - Implement httpd:handle_info/2: validates Tag against pending_timer_map and closes the socket only if it matches (genuine timeout), ignoring stale refs. Configurable request_timeout: - Add start/5 and start_link/5 arities accepting an Options map before Config, e.g. start_link(Address, Port, SocketOpts, #{request_timeout => 200}, Config). - Existing 2/3/4-arity calls still work via backwards-compatible defaults. - init/1 reads request_timeout from Options (default 30000 ms). Request timeout tests: - Add 'request timeout' describe block in httpd_integration_test.exs with its own setup that starts httpd with a 300ms request_timeout. - Test: server closes socket when headers are never terminated (no \r\n\r\n). - Test: server closes socket when declared body is never fully delivered. --- src/gen_tcp_server.erl | 30 ++++++-- src/httpd.erl | 119 ++++++++++++++++++++++++++------ test/atomvm_httpd_test.exs | 11 ++- test/httpd_integration_test.exs | 49 +++++++++++++ test/httpd_websocket_test.exs | 7 +- 5 files changed, 182 insertions(+), 34 deletions(-) diff --git a/src/gen_tcp_server.erl b/src/gen_tcp_server.erl index bbfba49..7802736 100644 --- a/src/gen_tcp_server.erl +++ b/src/gen_tcp_server.erl @@ -34,6 +34,13 @@ -callback handle_tcp_closed(Socket :: term(), State :: term()) -> NewState :: term(). +%% Optional callback: invoked for messages that gen_tcp_server does not handle +%% itself (e.g. internal timer messages). Return {noreply, NewState} to keep +%% the connection open, or {close, Socket, NewState} to close a specific socket. +-callback handle_info(Msg :: term(), State :: term()) -> + {noreply, NewState :: term()} | {close, Socket :: term(), NewState :: term()}. +-optional_callbacks([handle_info/2]). + % -define(TRACE_ENABLED, true). -include_lib("atomvm_httpd/include/trace.hrl"). @@ -146,10 +153,6 @@ handle_info({tcp_closed, Socket}, State) -> NewHandlerState = Handler:handle_tcp_closed(Socket, HandlerState), NewConns = maps:remove(Socket, Conns), {noreply, State#state{handler_state=NewHandlerState, connections=NewConns}}; -handle_info({request_timeout, Socket}, State) -> - ?TRACE("Request timeout for socket ~p", [Socket]), - try_close(Socket), - {noreply, State}; handle_info({tcp, Socket, Packet}, State) -> ?TRACE("received packet: len(~p) from ~p", [erlang:byte_size(Packet), socket:peername(Socket)]), handle_tcp_data(Socket, Packet, State); @@ -157,8 +160,23 @@ handle_info({'EXIT', _Pid, _Reason}, State) -> ?TRACE("Linked process ~p exited: ~p", [_Pid, _Reason]), {noreply, State}; handle_info(Info, State) -> - io:format("Received spurious info msg: ~p~n", [Info]), - {noreply, State}. + %% Forward unrecognised messages to the handler if it exports handle_info/2. + %% The handler may return {noreply, NewState} or {close, Socket, NewState}. + #state{handler=Handler, handler_state=HandlerState} = State, + case erlang:function_exported(Handler, handle_info, 2) of + true -> + case Handler:handle_info(Info, HandlerState) of + {noreply, NewHandlerState} -> + {noreply, State#state{handler_state = NewHandlerState}}; + {close, Socket, NewHandlerState} -> + ?TRACE("handle_info requested close for socket ~p", [Socket]), + try_close(Socket), + {noreply, State#state{handler_state = NewHandlerState}} + end; + false -> + io:format("Received spurious info msg: ~p~n", [Info]), + {noreply, State} + end. %% @hidden terminate(_Reason, _State) -> diff --git a/src/httpd.erl b/src/httpd.erl index 2e375e3..7b227d4 100644 --- a/src/httpd.erl +++ b/src/httpd.erl @@ -17,8 +17,8 @@ -module(httpd). --export([start/2, start/3, start/4, start_link/2, start_link/3, start_link/4, stop/1]). --export([init/1, handle_receive/3, handle_tcp_closed/2]). +-export([start/2, start/3, start/4, start/5, start_link/2, start_link/3, start_link/4, start_link/5, stop/1]). +-export([init/1, handle_receive/3, handle_tcp_closed/2, handle_info/2]). -ifdef(TEST). -export([maybe_parse_http_request/1, handle_request_state/3, get_request_state/1]). @@ -80,29 +80,41 @@ %% API %% +-type options() :: #{ + request_timeout => pos_integer() +}. + -spec start(Port :: portnum(), Config :: config()) -> {ok, HTTPD :: pid()} | {error, Reason :: term()}. start(Port, Config) -> - start(any, Port, #{}, Config). + start(any, Port, #{}, #{}, Config). -spec start(Address :: address(), Port :: portnum(), Config :: config()) -> {ok, HTTPD :: pid()} | {error, Reason :: term()}. start(Address, Port, Config) -> - start(Address, Port, #{}, Config). + start(Address, Port, #{}, #{}, Config). -spec start(Address :: address(), Port :: portnum(), SocketOptions :: map(), Config :: config()) -> {ok, HTTPD :: pid()} | {error, Reason :: term()}. start(Address, Port, SocketOptions, Config) -> - gen_tcp_server:start(#{addr => Address, port => Port}, SocketOptions, ?MODULE, Config). + start(Address, Port, SocketOptions, #{}, Config). + +-spec start(Address :: address(), Port :: portnum(), SocketOptions :: map(), Options :: options(), Config :: config()) -> {ok, HTTPD :: pid()} | {error, Reason :: term()}. +start(Address, Port, SocketOptions, Options, Config) -> + gen_tcp_server:start(#{addr => Address, port => Port}, SocketOptions, ?MODULE, {Options, Config}). -spec start_link(Port :: portnum(), Config :: config()) -> {ok, HTTPD :: pid()} | {error, Reason :: term()}. start_link(Port, Config) -> - start_link(any, Port, #{}, Config). + start_link(any, Port, #{}, #{}, Config). -spec start_link(Address :: address(), Port :: portnum(), Config :: config()) -> {ok, HTTPD :: pid()} | {error, Reason :: term()}. start_link(Address, Port, Config) -> - start_link(Address, Port, #{}, Config). + start_link(Address, Port, #{}, #{}, Config). -spec start_link(Address :: address(), Port :: portnum(), SocketOptions :: map(), Config :: config()) -> {ok, HTTPD :: pid()} | {error, Reason :: term()}. start_link(Address, Port, SocketOptions, Config) -> - gen_tcp_server:start_link(#{addr => Address, port => Port}, SocketOptions, ?MODULE, Config). + start_link(Address, Port, SocketOptions, #{}, Config). + +-spec start_link(Address :: address(), Port :: portnum(), SocketOptions :: map(), Options :: options(), Config :: config()) -> {ok, HTTPD :: pid()} | {error, Reason :: term()}. +start_link(Address, Port, SocketOptions, Options, Config) -> + gen_tcp_server:start_link(#{addr => Address, port => Port}, SocketOptions, ?MODULE, {Options, Config}). stop(Httpd) -> gen_tcp_server:stop(Httpd). @@ -112,7 +124,11 @@ stop(Httpd) -> %% %% @hidden +init({Options, Config}) -> + Timeout = maps:get(request_timeout, Options, 30000), + {ok, #state{config = Config, request_timeout = Timeout}}; init(Config) -> + %% Backwards-compatible: called with just Config (no Options). {ok, #state{config = Config}}. %% @hidden @@ -299,7 +315,10 @@ call_http_req_handler(Socket, HttpRequest, State) -> {noreply, NewHandlerState} -> NewState = update_state(Socket, HttpRequest, NewHandlerState, State), {noreply, NewState}; - %% reply — handler wants to keep the connection open; honour keep-alive if requested + %% reply — always keeps the socket open (gen_tcp_server treats {reply,...} as keep-open). + %% NOTE: HTTP/1.0 default-close and Connection: close semantics are not yet implemented; + %% that negotiation is deferred to a follow-up. Handlers that need to force a close + %% should return {close, ...} instead. {reply, Reply, NewHandlerState} -> NewState = update_state(Socket, HttpRequest, NewHandlerState, State), {reply, create_reply(?OK, #{"Content-Type" => "application/octet-stream"}, Reply), NewState}; @@ -357,6 +376,31 @@ handle_tcp_closed(Socket, State) -> CleanState#state{ws_socket_map = NewWebSocketMap} end. +%% @hidden +%% Validate timer-tagged request timeout messages. Using the TimerRef in the +%% message tag makes this race-free: if stop_request_timer/2 cancelled the +%% timer before the message was delivered, the ref will no longer be in +%% pending_timer_map and we ignore the stale message; if the timer fired first, +%% the ref matches and we correctly close the socket. +handle_info({request_timeout, Socket, Tag}, State) -> + TimerMap = State#state.pending_timer_map, + case maps:get(Socket, TimerMap, undefined) of + {_TimerRef, Tag} -> + %% Tag matches the current timer — the request genuinely timed out. + ?TRACE("Request timeout confirmed for socket ~p (tag ~p)", [Socket, Tag]), + NewTimerMap = maps:remove(Socket, TimerMap), + NewState = State#state{pending_timer_map = NewTimerMap}, + {close, Socket, NewState}; + _ -> + %% Tag does not match: timer was cancelled and a new one installed + %% (keep-alive), or the entry was already removed (request completed). + %% Ignore the stale message. + ?TRACE("Ignoring stale request_timeout for socket ~p (tag ~p)", [Socket, Tag]), + {noreply, State} + end; +handle_info(_Msg, State) -> + {noreply, State}. + %% %% Internal functions %% @@ -627,7 +671,12 @@ create_reply(StatusCode, ContentType, Reply) when is_list(ContentType) orelse is create_reply(StatusCode, #{"Content-Type" => ContentType}, Reply); create_reply(StatusCode, Headers, Reply) when is_map(Headers) -> ReplyLen = erlang:iolist_size(Reply), - HeadersWithLen = ensure_content_length(Headers, ReplyLen), + %% Normalize all header keys to lowercase binary before computing + %% Content-Length so that ensure_content_length/2 can reliably strip any + %% pre-existing content-length variant (e.g. "Content-Length", <<"Content-Length">>) + %% and avoid emitting duplicate headers. + NormalizedHeaders = normalize_headers(Headers), + HeadersWithLen = ensure_content_length(NormalizedHeaders, ReplyLen), [ <<"HTTP/1.1 ">>, erlang:integer_to_binary(StatusCode), <<" ">>, moniker(StatusCode), <<"\r\n">>, @@ -637,9 +686,32 @@ create_reply(StatusCode, Headers, Reply) when is_map(Headers) -> Reply ]. +%% @private +%% Rewrite every key in a response-header map to a lowercase binary so that +%% ensure_content_length/2 and to_headers_list/1 always operate on a uniform +%% representation regardless of whether the caller used strings, binaries, or +%% mixed-case atoms. +normalize_headers(Headers) -> + maps:fold( + fun(Key, Value, Acc) -> + NormKey = normalize_header_key(Key), + Acc#{NormKey => Value} + end, + #{}, + Headers + ). + +normalize_header_key(Key) when is_binary(Key) -> + list_to_binary(string:to_lower(binary_to_list(Key))); +normalize_header_key(Key) when is_list(Key) -> + list_to_binary(string:to_lower(Key)); +normalize_header_key(Key) when is_atom(Key) -> + list_to_binary(string:to_lower(atom_to_list(Key))). + %% @private ensure_content_length(Headers, ReplyLen) -> LenBin = erlang:integer_to_binary(ReplyLen), + %% After normalize_headers/1 the key is always <<"content-length">>. CleanHeaders = maps:remove(<<"content-length">>, Headers), CleanHeaders#{<<"content-length">> => LenBin}. @@ -699,29 +771,32 @@ method_to_atom(_) -> undefined. %% @private +%% Each timer entry is stored as {TimerRef, Tag} where Tag = make_ref(). +%% Tag is embedded in the {request_timeout, Socket, Tag} message so that +%% handle_info/2 can compare it against the current map entry and safely +%% ignore any stale messages that arrive after cancel_timer/1 was called +%% (they carry an old Tag that no longer matches). This makes the timeout +%% handling race-free without needing a receive-flush. start_request_timer(Socket, State) -> Timeout = State#state.request_timeout, TimerMap = State#state.pending_timer_map, - %% Cancel any pre-existing timer for this socket to avoid stale timeout - %% messages being delivered to a keep-alive connection. + %% Cancel any pre-existing timer for this socket. case maps:get(Socket, TimerMap, undefined) of undefined -> ok; - OldRef -> - erlang:cancel_timer(OldRef), - %% Flush a stale timeout message that may already be in the mailbox. - receive {request_timeout, Socket} -> ok after 0 -> ok end + {OldRef, _OldTag} -> erlang:cancel_timer(OldRef) end, - TimerRef = erlang:send_after(Timeout, self(), {request_timeout, Socket}), - State#state{pending_timer_map = TimerMap#{Socket => TimerRef}}. + Tag = make_ref(), + TimerRef = erlang:send_after(Timeout, self(), {request_timeout, Socket, Tag}), + State#state{pending_timer_map = TimerMap#{Socket => {TimerRef, Tag}}}. %% @private stop_request_timer(Socket, State) -> TimerMap = State#state.pending_timer_map, - %% Cancel the timer so it never fires after the request completes. + %% Cancel the timer. Any {request_timeout, Socket, Tag} already in the + %% mailbox carries the old Tag; handle_info/2 will ignore it because we + %% remove the entry from pending_timer_map here — no receive-flush needed. case maps:get(Socket, TimerMap, undefined) of undefined -> ok; - Ref -> - erlang:cancel_timer(Ref), - receive {request_timeout, Socket} -> ok after 0 -> ok end + {Ref, _Tag} -> erlang:cancel_timer(Ref) end, State#state{pending_timer_map = maps:remove(Socket, TimerMap)}. diff --git a/test/atomvm_httpd_test.exs b/test/atomvm_httpd_test.exs index 98f7d84..49c71f0 100644 --- a/test/atomvm_httpd_test.exs +++ b/test/atomvm_httpd_test.exs @@ -40,9 +40,14 @@ defmodule HttpdUnitTest do # Partial request should be stored in the pending map assert %{^socket => ^http_request} = pending_request_map - # A request timer should have been started for the socket - assert is_reference(Map.get(pending_timer_map, socket)), - "expected a timer ref in pending_timer_map for the socket" + # A request timer should have been started for the socket. + # The entry is {TimerRef, Tag} — both are opaque references. + timer_entry = Map.get(pending_timer_map, socket) + assert is_tuple(timer_entry) and tuple_size(timer_entry) == 2, + "expected a {timer_ref, tag} tuple in pending_timer_map for the socket" + {t_ref, t_tag} = timer_entry + assert is_reference(t_ref) + assert is_reference(t_tag) assert :wait_for_body = :httpd.get_request_state(http_request) end diff --git a/test/httpd_integration_test.exs b/test/httpd_integration_test.exs index 1146bb0..9ca5068 100644 --- a/test/httpd_integration_test.exs +++ b/test/httpd_integration_test.exs @@ -295,6 +295,55 @@ defmodule HttpdIntegrationTest do end end + describe "request timeout" do + # A deliberately short timeout so tests finish quickly. + @timeout_ms 300 + + # Start a second httpd with the short timeout; override :port in context. + setup do + port = find_free_tcp_port() + config = [{[], %{handler: TestEchoHandler, handler_config: %{test_pid: self()}}}] + + {:ok, server} = + :httpd.start_link(:any, port, %{}, %{request_timeout: @timeout_ms}, config) + + Process.sleep(20) + + on_exit(fn -> + if Process.alive?(server), do: :httpd.stop(server) + end) + + {:ok, port: port} + end + + test "closes socket when request headers are never completed", %{port: port} do + {:ok, socket} = connect(port) + # Send an incomplete request — no \r\n\r\n header terminator, so the + # server buffers and starts the request timer. + :ok = :gen_tcp.send(socket, "GET / HTTP/1.1\r\nHost: example.com\r\n") + + # Wait well past the configured timeout and expect the server to close. + Process.sleep(@timeout_ms + 200) + assert {:error, :closed} = :gen_tcp.recv(socket, 0, 500) + :gen_tcp.close(socket) + end + + test "closes socket when declared body is never fully delivered", %{port: port} do + {:ok, socket} = connect(port) + # Headers are complete, but Content-Length claims 100 bytes and we only + # send 5. The server should start waiting for the rest and time out. + :ok = + :gen_tcp.send( + socket, + "POST / HTTP/1.1\r\nHost: example.com\r\ncontent-length: 100\r\n\r\nhello" + ) + + Process.sleep(@timeout_ms + 200) + assert {:error, :closed} = :gen_tcp.recv(socket, 0, 500) + :gen_tcp.close(socket) + end + end + defp connect(port) do :gen_tcp.connect(~c"localhost", port, [:binary, active: false, packet: :raw]) end diff --git a/test/httpd_websocket_test.exs b/test/httpd_websocket_test.exs index 3216a04..dae8bd5 100644 --- a/test/httpd_websocket_test.exs +++ b/test/httpd_websocket_test.exs @@ -57,9 +57,10 @@ defmodule HttpdWebsocketTest do # Receive complete upgrade response response = read_http_response(socket) assert response =~ "HTTP/1.1 101 Switching Protocols" - assert response =~ "Upgrade: websocket" - assert response =~ "Connection: Upgrade" - assert response =~ "Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=" + # Response headers are normalized to lowercase keys by the server. + assert response =~ "upgrade: websocket" + assert response =~ "connection: Upgrade" + assert response =~ "sec-websocket-accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=" # Verify handler received init assert_receive {:ws_init, _websocket, _path}, @receive_timeout From 2045bcc804148aec5f71901d89834bcb46e7a046 Mon Sep 17 00:00:00 2001 From: harmon25 Date: Thu, 18 Jun 2026 21:21:50 -0400 Subject: [PATCH 4/8] Demote mid-transfer close log to TRACE; make send chunk size configurable (default 4096) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue 1: 'Connection closed mid-transfer' was logged at io:format level on every client-initiated cancel (tab close, image swap, parallel race). This fires dozens of times per minute on a camera stream and obscures real errors. Demoted to ?TRACE — a compile-time no-op at the default log level. Issue 3: MAX_SEND_CHUNK was hardcoded at 1460 bytes (single TCP MSS), causing ~90 socket:send NIF crossings + receive-after-0 yields per 130 KB JPEG frame. Replaced with a configurable chunk_size key in the SocketOptions map (default 4096 bytes), reducing NIF crossings by ~65% for typical camera payloads. - -define(DEFAULT_SEND_CHUNK, 4096) replaces the old 1460 constant - #state gains send_chunk_size field; init/1 reads + strips chunk_size from SocketOptions via maps:without/2 (same pattern as max_connections, prevents badmatch crash if key reached socket:setopt) - try_send/try_send_iolist/try_send_binary all take MaxChunk as 3rd arg, threaded from State in handle_tcp_data/3 - README documents chunk_size alongside max_connections in the application-level socket options section - Two new integration tests: guards the option-stripping fix and verifies a 16 KB response body arrives intact through a 512-byte chunk size Issues 2 (TCP_NODELAY) and 4 (per-connection workers) deferred: - TCP_NODELAY is not supported by AtomVM's socket NIF (otp_socket.c only exposes reuseaddr/linger/recvbuf); needs an upstream AtomVM change. - Per-connection worker refactor is the larger concurrent-send win and will be addressed in a follow-up. --- README.md | 6 ++- src/gen_tcp_server.erl | 83 +++++++++++++++++-------------- test/httpd_integration_test.exs | 88 +++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index a1fe1e1..42ae308 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ At a high level, this server supports the following features: - Server-initiated push messages - **ESP32-optimized networking** - Configurable socket options (`SO_REUSEADDR`, buffer sizes, etc.) - - 1460-byte send chunking for lwIP compatibility + - Configurable send chunking (default 4096 bytes) for lwIP compatibility - Incremental I/O list processing to minimize heap pressure The HTTPd server is designed around a callback architecture, whereby users implement behaviors to handle various requests into the HTTP server. This architecture allows developers to focus on the logic of their applications, as opposed to the nitty gritty details of the HTTP protocol, while still providing access to contextual information about the request, including: @@ -139,6 +139,10 @@ Supported socket options (per AtomVM's `socket` module): - `{otp, recvbuf}` - `non_neg_integer()` - Receive buffer size in bytes - `{ip, add_membership}` - Multicast group membership (advanced) +The following keys are handled by `gen_tcp_server` itself and are **not** passed to `socket:setopt`: +- `max_connections` - `non_neg_integer()` - Maximum concurrent connections (0 = unlimited, default) +- `chunk_size` - `pos_integer()` - Maximum bytes per `socket:send/2` call (default: `4096`). Tune this to match your platform's lwIP send-buffer headroom. A 100 KB JPEG at 4096 bytes/chunk requires ~25 send calls; at 1460 bytes it required ~70. ESP32 lwIP defaults to an 8 KB send buffer, so values up to 8192 are generally safe. + The default configuration enables `SO_REUSEADDR` which is particularly useful on ESP32 for quick restarts during development. > Note. The configuration for the HTTPd server is described in more detail below. diff --git a/src/gen_tcp_server.erl b/src/gen_tcp_server.erl index 7802736..acd3f3c 100644 --- a/src/gen_tcp_server.erl +++ b/src/gen_tcp_server.erl @@ -44,13 +44,6 @@ % -define(TRACE_ENABLED, true). -include_lib("atomvm_httpd/include/trace.hrl"). --record(state, { - handler, - handler_state, - connections = #{}, - max_connections = 0 -}). - -define(DEFAULT_BIND_OPTIONS, #{ family => inet, addr => any @@ -58,8 +51,19 @@ -define(DEFAULT_SOCKET_OPTIONS, #{ {socket, reuseaddr} => true }). -%% Smaller chunks work better with lwIP's limited buffers --define(MAX_SEND_CHUNK, 1460). %% TCP MSS - fits in single packet without fragmentation +%% Default send-chunk size. 4096 is a comfortable fit within the ESP32 lwIP +%% send-buffer (8 KB by default) and cuts NIF-crossing overhead by ~3× versus +%% the old 1460-byte (single TCP MSS) default. Callers can override via the +%% `chunk_size` key in the SocketOptions map passed to start/4 / start_link/4. +-define(DEFAULT_SEND_CHUNK, 4096). + +-record(state, { + handler, + handler_state, + connections = #{}, + max_connections = 0, + send_chunk_size = ?DEFAULT_SEND_CHUNK +}). %% %% API @@ -88,9 +92,10 @@ stop(Server) -> init({BindOptions, SocketOptions, Handler, Args}) -> Self = self(), MaxConnections = maps:get(max_connections, SocketOptions, 0), - %% Strip max_connections before passing to set_socket_options/2 so that - %% socket:setopt/3 is never called with an unknown option key. - CleanSocketOptions = maps:remove(max_connections, SocketOptions), + ChunkSize = maps:get(chunk_size, SocketOptions, ?DEFAULT_SEND_CHUNK), + %% Strip application-level keys before passing to set_socket_options/2 so + %% that socket:setopt/3 is never called with an unknown option key. + CleanSocketOptions = maps:without([max_connections, chunk_size], SocketOptions), case socket:open(inet, stream, tcp) of {ok, Socket} -> ok = set_socket_options(Socket, CleanSocketOptions), @@ -101,7 +106,12 @@ init({BindOptions, SocketOptions, Handler, Args}) -> spawn_link(fun() -> accept(Self, Socket) end), case Handler:init(Args) of {ok, HandlerState} -> - {ok, #state{handler = Handler, handler_state = HandlerState, max_connections = MaxConnections}}; + {ok, #state{ + handler = Handler, + handler_state = HandlerState, + max_connections = MaxConnections, + send_chunk_size = ChunkSize + }}; HandlerError -> try_close(Socket), {stop, {handler_error, HandlerError}} @@ -184,11 +194,11 @@ terminate(_Reason, _State) -> %% @private handle_tcp_data(Socket, Packet, State) -> - #state{handler=Handler, handler_state=HandlerState} = State, + #state{handler=Handler, handler_state=HandlerState, send_chunk_size=MaxChunk} = State, case Handler:handle_receive(Socket, Packet, HandlerState) of {reply, ResponsePacket, ResponseState} -> ?TRACE("Sending reply to endpoint ~p", [socket:peername(Socket)]), - case try_send(Socket, ResponsePacket) of + case try_send(Socket, ResponsePacket, MaxChunk) of ok -> {noreply, State#state{handler_state=ResponseState}}; {error, closed} -> @@ -203,7 +213,7 @@ handle_tcp_data(Socket, Packet, State) -> {noreply, State#state{handler_state=ResponseState}}; {close, ResponsePacket} -> ?TRACE("Sending reply to endpoint ~p and closing socket: ~p", [socket:peername(Socket), Socket]), - case try_send(Socket, ResponsePacket) of + case try_send(Socket, ResponsePacket, MaxChunk) of ok -> try_close(Socket); {error, closed} -> @@ -227,55 +237,52 @@ handle_tcp_data(Socket, Packet, State) -> %% %% @private -try_send(Socket, Packet) when is_binary(Packet) -> +try_send(Socket, Packet, MaxChunk) when is_binary(Packet) -> ?TRACE( "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 ]), - try_send_binary(Socket, Packet); -try_send(Socket, Byte) when is_integer(Byte) -> + try_send_binary(Socket, Packet, MaxChunk); +try_send(Socket, Byte, MaxChunk) when is_integer(Byte) -> %% Handles bytes (0-255) in iolists. Unicode must be pre-encoded to UTF-8. ?TRACE("Sending byte ~p as ~p", [Byte, <>]), - try_send(Socket, <>); -try_send(Socket, List) when is_list(List) -> + try_send(Socket, <>, MaxChunk); +try_send(Socket, List, MaxChunk) when is_list(List) -> case is_string(List) of true -> - try_send(Socket, list_to_binary(List)); + try_send(Socket, list_to_binary(List), MaxChunk); _ -> - try_send_iolist(Socket, List) + try_send_iolist(Socket, List, MaxChunk) end. -try_send_iolist(_Socket, []) -> +try_send_iolist(_Socket, [], _MaxChunk) -> ok; -try_send_iolist(Socket, [H | T]) -> - case try_send(Socket, H) of +try_send_iolist(Socket, [H | T], MaxChunk) -> + case try_send(Socket, H, MaxChunk) of ok -> - try_send_iolist(Socket, T); + try_send_iolist(Socket, T, MaxChunk); {error, _Reason} = Error -> Error end. -try_send_binary(_Socket, <<>>) -> +try_send_binary(_Socket, <<>>, _MaxChunk) -> ok; -try_send_binary(Socket, Packet) when is_binary(Packet) -> +try_send_binary(Socket, Packet, MaxChunk) when is_binary(Packet) -> TotalSize = byte_size(Packet), - ChunkSize = erlang:min(TotalSize, ?MAX_SEND_CHUNK), + ChunkSize = erlang:min(TotalSize, MaxChunk), <> = Packet, case socket:send(Socket, Chunk) of ok -> %% Give the scheduler a chance to run and let TCP drain maybe_yield(Rest), - try_send_binary(Socket, Rest); + try_send_binary(Socket, Rest, MaxChunk); {ok, Remaining} -> %% Partial send - combine remaining with rest and retry - try_send_binary(Socket, <>); + try_send_binary(Socket, <>, MaxChunk); {error, closed} -> - %% Only log if we actually had more data to send - case byte_size(Rest) of - 0 -> ok; %% Sent everything, client just closed after - that's fine - _ -> io:format("Connection closed mid-transfer (~p/~p bytes sent)~n", - [ChunkSize, TotalSize]) - end, + %% Normal client behaviour (tab close, image swap, parallel request racing ahead). + %% Log at TRACE only — this fires dozens of times per minute on camera streams. + ?TRACE("Connection closed mid-transfer (~p/~p bytes sent)", [ChunkSize, TotalSize]), {error, closed}; {error, Reason} -> io:format("Send error: ~p (chunk: ~p, total: ~p)~n", diff --git a/test/httpd_integration_test.exs b/test/httpd_integration_test.exs index 9ca5068..7b4e3e5 100644 --- a/test/httpd_integration_test.exs +++ b/test/httpd_integration_test.exs @@ -295,6 +295,94 @@ defmodule HttpdIntegrationTest do end end + describe "chunk_size socket option" do + # Deliberately small chunk size to exercise multi-chunk send path on a + # small test body, and verify the key is stripped so socket:setopt is + # never called with it (which would crash with a badmatch). + @small_chunk 512 + @large_response_body :binary.copy("x", 16_384) + + setup do + port = find_free_tcp_port() + + config = [ + {[], %{handler: TestEchoHandler, handler_config: %{test_pid: self()}}} + ] + + {:ok, server} = + :httpd.start_link(:any, port, %{chunk_size: @small_chunk}, config) + + Process.sleep(20) + + on_exit(fn -> + if Process.alive?(server), do: :httpd.stop(server) + end) + + {:ok, port: port} + end + + test "server starts successfully with chunk_size option (option is not passed to socket:setopt)", + %{port: port} do + # If chunk_size leaked into set_socket_options/2 the server would have + # crashed at startup — this assertion also validates that. + {:ok, socket} = connect(port) + + try do + :ok = + :gen_tcp.send(socket, "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n") + + assert {:ok, response} = recv_all(socket) + assert response =~ "HTTP/1.1 200 OK" + after + :gen_tcp.close(socket) + end + end + + test "large response body is delivered intact across many small chunks" do + # Start a fresh server whose handler is configured to reply with the large + # body — this exercises the multi-chunk send path without coupling to the + # echo handler's default behaviour. + port = find_free_tcp_port() + body_size = byte_size(@large_response_body) + + config = [ + {[], + %{ + handler: TestEchoHandler, + handler_config: %{ + test_pid: self(), + reply_body: @large_response_body, + reply_headers: %{"Content-Type" => "application/octet-stream"} + } + }} + ] + + {:ok, server} = + :httpd.start_link(:any, port, %{chunk_size: @small_chunk}, config) + + Process.sleep(20) + + {:ok, socket} = connect(port) + + try do + :ok = :gen_tcp.send(socket, "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n") + + assert_receive {:http_request, _}, @receive_timeout + + assert {:ok, response} = recv_all(socket) + assert response =~ "HTTP/1.1 200 OK" + + [_headers, body] = :binary.split(response, <<"\r\n\r\n">>) + + assert byte_size(body) == body_size, + "Expected #{body_size} bytes; got #{byte_size(body)}" + after + :gen_tcp.close(socket) + :httpd.stop(server) + end + end + end + describe "request timeout" do # A deliberately short timeout so tests finish quickly. @timeout_ms 300 From 4772805cd548b66ab7a0afce402e73ebb829851a Mon Sep 17 00:00:00 2001 From: harmon25 Date: Thu, 18 Jun 2026 21:28:03 -0400 Subject: [PATCH 5/8] Consolidate agent guidance into AGENTS.md; copilot-instructions now points to it AGENTS.md absorbs the valuable architecture, handler, routing, and TRACE content from .github/copilot-instructions.md and adds hard-won repo-specific gotchas: - AtomVM socket setopt allow-list (no {tcp, nodelay}; crashes on unknown keys) - app-level keys (max_connections, chunk_size) must be stripped before setopt fold - socket:send/2 partial-send semantics - send serialization bottleneck in gen_tcp_server - TestEchoHandler reply_body semantics (does not echo request body) - mix format scope (Elixir only), deps no-op, no CI, no priv/ in this repo - improvements branch vs main .github/copilot-instructions.md is now a single-paragraph pointer to AGENTS.md. --- .github/copilot-instructions.md | 99 +------------------ AGENTS.md | 163 +++++++++++++++++++------------- 2 files changed, 102 insertions(+), 160 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 91e8c50..ee69152 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,96 +1,7 @@ -# AtomVM HTTPD - AI Coding Guide +# AtomVM HTTPD — AI Coding Guide -## Project Overview +This project's agent and AI guidance lives in **[`AGENTS.md`](../AGENTS.md)** at the repo root. -HTTP server library for **AtomVM** - a lightweight Erlang VM for microcontrollers (ESP32, STM32). Mixed Erlang/Elixir codebase using Mix build system. - -## Architecture - -### Core Components -``` -gen_tcp_server.erl → httpd.erl → Handler Modules - (TCP layer) (HTTP parsing, (Request processing) - routing) -``` - -- **`gen_tcp_server`**: Generic TCP server behavior wrapping socket operations. Handlers implement `init/1`, `handle_receive/3`, `handle_tcp_closed/2`. -- **`httpd`**: HTTP 1.1 protocol implementation. Routes requests to handlers based on path prefix matching. Implements `gen_tcp_server` behavior. -- **Handler behaviors**: Three handler types for different use cases: - - `httpd_handler` - Low-level HTTP request handling - - `httpd_api_handler` - REST APIs with JSON encoding (implement `handle_api_request/4`) - - `httpd_ws_handler` - WebSocket communication (implement `handle_ws_init/3`, `handle_ws_message/2`) - -### Handler Return Values -Handlers return tuples indicating response behavior: -```erlang -{reply, Headers, Body, State} %% Send response, keep connection -{close, Headers, Body} %% Send response, close connection -{noreply, State} %% Continue accumulating request (streaming) -{error, not_found | bad_request | internal_server_error} -``` - -## Key Patterns - -### Path-Based Routing -Configuration maps URL path prefixes to handler modules: -```erlang -Config = [ - {[<<"api">>], #{handler => httpd_api_handler, handler_config => #{module => MyApi}}}, - {[<<"ws">>], #{handler => httpd_ws_handler, handler_config => #{module => MyWs}}}, - {[], #{handler => httpd_file_handler, handler_config => #{app => my_app}}} -] -``` -The first matching prefix wins. Path prefix is stripped before passing to handler. - -### Creating API Handlers -Implement `httpd_api_handler` behavior - see `httpd_stats_api_handler.erl`: -```erlang --behavior(httpd_api_handler). --export([handle_api_request/4]). - -handle_api_request(get, [<<"endpoint">>], HttpRequest, Args) -> - {ok, #{status => <<"ok">>}}; %% Auto-encoded to JSON -handle_api_request(_Method, _Path, _HttpRequest, _Args) -> - not_found. -``` - -### Tracing/Debugging -Enable per-module tracing by uncommenting the define before the include: -```erlang --define(TRACE_ENABLED, true). --include_lib("atomvm_httpd/include/trace.hrl"). -``` -Use `?TRACE("format ~p", [args])` macro. Disabled traces compile to `ok`. - -## AtomVM Constraints - -- **No hot code loading** - full redeploy required -- **Limited OTP** - subset of standard library; no `handle_continue` -- **Memory-sensitive** - prefer binaries, avoid large data structures, use streaming for big payloads -- **Platform modules**: `atomvm:platform/0`, `esp:*`, `atomvm:read_priv/2` for embedded resources - -## Development Commands - -```bash -mix deps.get # Fetch dependencies -mix compile # Build (uses erlc_paths: ["src"]) -mix test # Run tests (on host Erlang VM, not AtomVM) -``` - -Tests run on standard Erlang VM. The `-ifdef(TEST)` guard exposes internal functions for testing. - -## Testing Patterns -Tests use ExUnit. See `test/httpd_integration_test.exs` for socket-level testing: -- Create test handlers in `test/support/` (added to elixirc_paths in test env) -- Use `@tag handler_config: %{...}` to customize handler per-test -- Start server with `:httpd.start_link(port, config)`, test via `:gen_tcp` - -## File Organization - -| Directory | Purpose | -|-----------|---------| -| `src/*.erl` | Erlang source - handlers and core modules | -| `lib/*.ex` | Elixir source (thin wrapper) | -| `include/*.hrl` | Header files - HTTP codes, trace macros | -| `priv/` | Static assets (served via `httpd_file_handler`) | -| `test/support/` | Test-only modules | +See it for architecture, handler contracts, routing, commands, testing, and +repo-specific gotchas (socket setopt allow-list, app-level option stripping, +send serialization, etc.). Update `AGENTS.md` rather than duplicating guidance here. diff --git a/AGENTS.md b/AGENTS.md index 99b5963..70d17c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,83 +1,114 @@ -# AI Agent Reference for AtomVM HTTPD +# AtomVM HTTPD — Agent Guide -This project is an HTTP server library for **AtomVM** - a lightweight Erlang/Elixir virtual machine designed for microcontrollers and embedded systems. +HTTP/WebSocket server **library** for AtomVM (lightweight Erlang VM for +microcontrollers: ESP32, STM32). Implementation is Erlang (`src/`); `lib/` is a +thin Elixir convenience wrapper. Built with Mix. -## What is AtomVM? +## Architecture -AtomVM is a tiny Erlang VM that runs on resource-constrained devices like ESP32, STM32, and other microcontrollers. It allows you to write Erlang and Elixir code for embedded systems. - -- **GitHub**: https://github.com/atomvm/AtomVM -- **Documentation**: https://www.atomvm.net/doc/master/ -- **Programming Guide**: https://www.atomvm.net/doc/master/programmers-guide.html - -## Key Differences from Standard Erlang/OTP - -When working on this codebase, keep in mind: - -1. **Limited OTP Support**: AtomVM implements a subset of OTP. Not all standard library modules are available. - -2. **No `gen_server` callbacks with `handle_continue`**: Some newer OTP features may not be implemented. - -3. **Memory Constraints**: Code should be memory-efficient. Avoid large data structures and prefer streaming/chunked processing. - -4. **No Hot Code Loading**: Unlike standard Erlang, AtomVM doesn't support hot code swapping. - -5. **Limited Process Dictionary**: Use sparingly if at all. - -6. **Binary Handling**: Binaries are well-supported and preferred for string/data handling. - -## AtomVM-Specific Modules - -AtomVM provides platform-specific modules: - -- `atomvm:platform/0` - Returns the current platform (e.g., `esp32`, `stm32`, `generic_unix`) -- `esp:*` - ESP32-specific functions (GPIO, WiFi, etc.) -- `network:*` - Network configuration for embedded platforms - -## Project Structure - -- `src/` - Erlang source files (.erl) -- `lib/` - Elixir source files (.ex) -- `include/` - Header files (.hrl) -- `priv/` - Static assets and resources -- `test/` - Test files - -## Building - -This is a mixed Erlang/Elixir project using Mix: - -```bash -mix deps.get -mix compile +``` +gen_tcp_server.erl → httpd.erl → Handler modules +(TCP/socket layer) (HTTP/1.1 parse, (request processing) + routing, send) ``` -For deploying to ESP32, you'll need to create an AVM file and flash it. - -## Useful Tips +- **`gen_tcp_server`**: generic TCP server behavior wrapping AtomVM `socket`. + Implementers provide `init/1`, `handle_receive/3`, `handle_tcp_closed/2` + (optional `handle_info/2`). A single gen_server owns the listen socket AND all + handler state; per-connection processes only own `recv` and forward data to it. + Parsing, dispatch, and the chunked `send` all run in that one gen_server — so + responses are serialized across connections (known throughput bottleneck for + large/parallel responses). +- **`httpd`**: HTTP/1.1 protocol + path-prefix routing; implements the + `gen_tcp_server` behavior. +- **Handler behaviors**: + - `httpd_handler` — low-level HTTP (`init_handler/2`, `handle_http_req/2`) + - `httpd_api_handler` — REST/JSON (`handle_api_request/4`) + - `httpd_ws_handler` — WebSocket (`handle_ws_init/3`, `handle_ws_message/2`) + +### Handler return values + +```erlang +{reply, Headers, Body, State} %% send response, keep connection open +{close, Headers, Body} %% send response, then close +{noreply, State} %% keep accumulating request (streaming) +{error, not_found | bad_request | internal_server_error} +``` -1. **Debugging**: Use `io:format/2` or `erlang:display/1` for debugging - standard Erlang debugger is not available. +### Path-based routing -2. **Testing Locally**: Tests run on the host machine using standard Erlang VM, not AtomVM itself. +First matching prefix wins; the prefix is stripped before reaching the handler. -3. **Handler Pattern**: This httpd uses a handler-based architecture. Each `*_handler.erl` module handles specific route patterns. +```erlang +Config = [ + {[<<"api">>], #{handler => httpd_api_handler, handler_config => #{module => MyApi}}}, + {[<<"ws">>], #{handler => httpd_ws_handler, handler_config => #{module => MyWs}}}, + {[], #{handler => httpd_file_handler, handler_config => #{app => my_app}}} +] +``` -4. **WebSocket Support**: `httpd_ws_handler.erl` provides WebSocket functionality. +File handler is the catch-all and must be last. -5. **API Handlers**: Files ending in `_api_handler.erl` are REST API endpoints. +### API handler example (see `httpd_stats_api_handler.erl`) -## Common Patterns in This Codebase +```erlang +handle_api_request(get, [<<"endpoint">>], _HttpRequest, _Args) -> + {ok, #{status => <<"ok">>}}; %% map auto-encoded to JSON +handle_api_request(_M, _P, _R, _A) -> + not_found. +``` -### Handler Behavior +## Commands + +- `mix compile` — build (`erlc_paths: ["src"]`). +- `mix test` — full suite, runs on **host Erlang/OTP**, not AtomVM. +- `mix test test/httpd_integration_test.exs:NN` — run a single test. +- `mix format` — Elixir only (`lib`, `test`); does NOT touch Erlang `src/`. +- `mix deps.get` is a no-op here (no deps). The README dep snippet is for *consumers*. +- No CI exists; local `mix test` is the only gate. + +## Repo gotchas + +- **AtomVM `socket` setopt is an allow-list**: only `{socket, reuseaddr|linger|type}`, + `{otp, recvbuf}`, `{ip, add_membership}` are supported — **no `{tcp, nodelay}`**. + `gen_tcp_server:set_socket_options/2` uses strict `ok = socket:setopt(...)`, so an + unsupported key **crashes the server at startup**. +- **App-level keys are not socket options**: `max_connections` and `chunk_size` ride in + the same `SocketOptions` map but are stripped via `maps:without/2` in `init/1` before the + setopt fold. Add any new app-level key to that strip list. +- `socket:send/2` returns `ok | {ok, Rest} | {error, Reason}`; partial sends must be retried + (see `try_send_binary/3`). +- Responses are sent in `chunk_size` slices (default 4096; configurable per server). +- No `priv/` in this repo; `httpd_file_handler` serves from a *consumer* app's `priv`. + +## Testing + +- `erlc_options(:test)` injects `{d, TEST}`, enabling `-ifdef(TEST)` internal exports in `httpd.erl`. +- `test/support/*.ex` (e.g. `TestEchoHandler`) compile only in `:test` (`elixirc_paths`). +- Integration tests drive the server over raw `:gen_tcp`; per-test handler config via the + `handler_config` setup-context key. +- `TestEchoHandler` replies with its configured `:reply_body` (default `"ok"`) — it does NOT + echo the request body. + +## Debugging & tracing + +Per-module tracing: uncomment the define *before* the include, then use `?TRACE`. + +```erlang +-define(TRACE_ENABLED, true). +-include_lib("atomvm_httpd/include/trace.hrl"). +``` -Handlers implement callbacks for HTTP request processing. Check `httpd_handler.erl` for the behavior definition. +Disabled traces compile to `ok`. Keep default log output quiet — gate noise behind `?TRACE`, +not `io:format`. -### TCP Server +## AtomVM constraints -`gen_tcp_server.erl` provides the underlying TCP socket management. +- Subset of OTP (no `handle_continue`); no hot code loading (full redeploy required). +- Memory-sensitive: prefer binaries, stream large payloads, avoid the process dictionary. +- Platform modules: `atomvm:platform/0`, `esp:*`, `atomvm:read_priv/2`. -## References +## Conventions -- [AtomVM GitHub](https://github.com/atomvm/AtomVM) -- [AtomVM Docs](https://www.atomvm.net/doc/master/) -- [ESP32 Platform Notes](https://www.atomvm.net/doc/master/build-instructions.html#esp32) -- [Erlang Compatibility](https://www.atomvm.net/doc/master/programmers-guide.html#erlang-compatibility) +- Active development branch is `improvements` (not `main`); README dep examples pin `main`. +- Erlang `src/` is the source of truth; `lib/atomvm_httpd.ex` is only a convenience wrapper. From f710e207f0ada2ef2d87703abfec26270dd54fa0 Mon Sep 17 00:00:00 2001 From: harmon25 Date: Thu, 18 Jun 2026 21:34:56 -0400 Subject: [PATCH 6/8] Fix maps:without/2 not available in AtomVM; use maps:remove/2 instead --- src/gen_tcp_server.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gen_tcp_server.erl b/src/gen_tcp_server.erl index acd3f3c..6996c14 100644 --- a/src/gen_tcp_server.erl +++ b/src/gen_tcp_server.erl @@ -95,7 +95,7 @@ init({BindOptions, SocketOptions, Handler, Args}) -> ChunkSize = maps:get(chunk_size, SocketOptions, ?DEFAULT_SEND_CHUNK), %% Strip application-level keys before passing to set_socket_options/2 so %% that socket:setopt/3 is never called with an unknown option key. - CleanSocketOptions = maps:without([max_connections, chunk_size], SocketOptions), + CleanSocketOptions = maps:remove(chunk_size, maps:remove(max_connections, SocketOptions)), case socket:open(inet, stream, tcp) of {ok, Socket} -> ok = set_socket_options(Socket, CleanSocketOptions), From 1c5706095dc0d7f566eee994779e98a0d3a0493d Mon Sep 17 00:00:00 2001 From: harmon25 Date: Sun, 21 Jun 2026 11:13:29 -0400 Subject: [PATCH 7/8] Remove commented-out dependency placeholders from mix.exs --- TODO.md | 100 -------------------------------------------------------- mix.exs | 8 +---- 2 files changed, 1 insertion(+), 107 deletions(-) delete mode 100644 TODO.md diff --git a/TODO.md b/TODO.md deleted file mode 100644 index 805fe97..0000000 --- a/TODO.md +++ /dev/null @@ -1,100 +0,0 @@ -# AtomVM HTTPD - Stability & Memory Leak Fixes - -Tracking document for memory leak and VM stability issues identified during code review. - -## Critical Issues - -### 1. ✅ Fix accept loop crash recovery -**File:** `src/gen_tcp_server.erl` lines 271-283 - -**Problem:** When `socket:accept` fails (e.g., `emfile` - too many open files), the accept loop silently exits without spawning a replacement. The server stops accepting new connections permanently but the gen_server keeps running. - -**Solution:** Handle errors by retrying with a backoff, or notify the controlling process. - ---- - -### 2. ❌ Add pending request timeouts -**File:** `src/httpd.erl` - `pending_request_map` and `pending_buffer_map` - -**Problem:** Clients can open connections and send partial data, leaving entries in the maps indefinitely. These are only cleaned up when the socket closes or request completes. A malicious or buggy client could cause unbounded memory growth. - -**Solution:** Implement timeouts for incomplete requests: -- Periodic cleanup timer that removes stale entries -- Maximum buffer size per connection -- Maximum number of pending connections - ---- - -### 3. ❌ Fix WebSocket process leak on error -**File:** `src/httpd.erl` lines 172-193 - -**Problem:** If `get_reply_token/1` throws after `WsHandler:start/3` succeeds (e.g., missing `Sec-WebSocket-Key` header), the WebSocket process is started but never added to `ws_socket_map` and never stopped - becoming an orphaned process. - -**Solution:** Validate headers before starting WebSocket process, or wrap in try/catch with cleanup. - ---- - -## Medium Issues - -### 4. ❌ Link loop processes to controller -**File:** `src/gen_tcp_server.erl` lines 285-296 - -**Problem:** The `loop/2` process is spawned but not linked to anything. If the controlling gen_server crashes, loop processes continue running, try to send messages to dead processes, and sockets may not be properly closed. - -**Solution:** Use `spawn_link` or monitors, and handle exit signals appropriately. - ---- - -### 5. ✅ Fix update_state map operator -**File:** `src/httpd.erl` line 309 - -**Problem:** Uses `:=` (update existing key) instead of `=>` (insert/update). Will crash if the socket isn't already in the map, which can happen if a handler returns `{noreply, ...}` on the first call. - -**Solution:** Change `:=` to `=>` for robustness. - ---- - -### 6. ❌ Improve WebSocket unmask memory usage -**File:** `src/httpd_ws_handler.erl` lines 208-214 - -**Problem:** The `unmask/4` function builds a list in reverse then converts to binary. For very large WebSocket payloads, this could consume excessive memory and block the scheduler. - -**Solution:** Process in chunks or use binary comprehensions for better memory behavior. - ---- - -## Minor Issues - -### 7. ❌ Fix atom leak in query params -**File:** `src/httpd.erl` line 454 - -**Problem:** Uses `list_to_atom/1` on user-supplied query parameter keys. Could exhaust atom table resources with many unique query parameters. - -**Solution:** Use `list_to_binary/1` instead for query param keys. - ---- - -### 8. ❌ Add connection limit -**File:** `src/gen_tcp_server.erl` accept loop - -**Problem:** Every incoming connection spawns a new process. Under load or attack, this could exhaust process limit, file descriptor limit, or memory. - -**Solution:** Add a configurable maximum connection count. - ---- - -## Progress Log - -| Date | Issue # | Status | Notes | -|------|---------|--------|-------| -| 2024-12-07 | - | - | Initial review and documentation | -| 2024-12-07 | 5 | ✅ | Fixed map update operator in httpd.erl | -| 2024-12-07 | 1 | ✅ | Fixed accept loop crash recovery in gen_tcp_server.erl | - ---- - -## Legend -- ❌ Not started -- 🔄 In progress -- ✅ Complete -- ⏸️ Blocked diff --git a/mix.exs b/mix.exs index f2023c9..0b62f50 100644 --- a/mix.exs +++ b/mix.exs @@ -22,13 +22,7 @@ defmodule AtomvmHttpd.MixProject do ] end - # Run "mix help deps" to learn about dependencies. - defp deps do - [ - # {:dep_from_hexpm, "~> 0.3.0"}, - # {:dep_from_git, git: "https://github.com/elixir-lang/my_dep.git", tag: "0.1.0"} - ] - end + defp deps, do: [] defp erlc_options(:test), do: [:debug_info, {:d, :TEST}] defp erlc_options(_env), do: [:debug_info] From 3736d155a1d9e18da087840c6ecb6aae8d6f7f2f Mon Sep 17 00:00:00 2001 From: harmon25 Date: Sun, 21 Jun 2026 13:34:37 -0400 Subject: [PATCH 8/8] Implement HTTPD Debug Example for AtomVM on ESP32 - Added a complete debug/test application for iterating on ESP32 hardware with real-world HTTP loads. - Introduced WiFi STA connectivity with compile-time credential injection. - Created debug API endpoints for stress-testing various request/response sizes. - Implemented built-in stats handlers for system info and memory monitoring. - Developed a browser test dashboard with interactive controls. - Included an automated test suite with curl-based size sweeps. - Added serial monitoring with persistent logs for debugging. - Updated existing HTTPD and API handler modules to support new features and error handling. - Introduced new utility functions for binary conversion and heap information retrieval. - Created scripts for building, flashing, and testing the application. --- .gitignore | 4 +- AGENTS.md | 68 +++++ examples/httpd_debug/.gitignore | 4 + examples/httpd_debug/README.md | 227 +++++++++++++++ examples/httpd_debug/lib/httpd_debug.ex | 94 ++++++ .../lib/httpd_debug/debug_api_handler.ex | 137 +++++++++ examples/httpd_debug/lib/httpd_debug/wifi.ex | 86 ++++++ examples/httpd_debug/mix.exs | 30 ++ examples/httpd_debug/mix.lock | 4 + examples/httpd_debug/priv/index.html | 274 ++++++++++++++++++ examples/httpd_debug/scripts/flash.sh | 37 +++ examples/httpd_debug/scripts/monitor.sh | 14 + examples/httpd_debug/scripts/test.sh | 134 +++++++++ src/gen_tcp_server.erl | 76 ++--- src/httpd.erl | 28 +- src/httpd_api_handler.erl | 6 +- src/httpd_stats_api_handler.erl | 19 +- 17 files changed, 1180 insertions(+), 62 deletions(-) create mode 100644 examples/httpd_debug/.gitignore create mode 100644 examples/httpd_debug/README.md create mode 100644 examples/httpd_debug/lib/httpd_debug.ex create mode 100644 examples/httpd_debug/lib/httpd_debug/debug_api_handler.ex create mode 100644 examples/httpd_debug/lib/httpd_debug/wifi.ex create mode 100644 examples/httpd_debug/mix.exs create mode 100644 examples/httpd_debug/mix.lock create mode 100644 examples/httpd_debug/priv/index.html create mode 100755 examples/httpd_debug/scripts/flash.sh create mode 100755 examples/httpd_debug/scripts/monitor.sh create mode 100755 examples/httpd_debug/scripts/test.sh diff --git a/.gitignore b/.gitignore index 35d485b..602e4e5 100644 --- a/.gitignore +++ b/.gitignore @@ -22,4 +22,6 @@ atomvm_httpd-*.tar # Temporary files, for example, from tests. /tmp/ -.elixir_ls/ \ No newline at end of file +.elixir_ls/ + +mise.local.toml \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index 70d17c3..551e2ff 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,3 +112,71 @@ not `io:format`. - Active development branch is `improvements` (not `main`); README dep examples pin `main`. - Erlang `src/` is the source of truth; `lib/atomvm_httpd.ex` is only a convenience wrapper. + +## ESP32 Debug Loop (examples/httpd_debug/) + +A complete debug/test application for iterating on ESP32 hardware with real-world HTTP loads. + +### Setup (one-time) + +1. **WiFi credentials** (compile-time): + ```bash + export ATOMVM_WIFI_SSID="your-ssid" + export ATOMVM_WIFI_PSK="your-password" + ``` + +2. **ESP32 connection**: Connect ESP32-S3 to `/dev/ttyACM0` (or update scripts if different port). + +3. **ESP-IDF environment**: Already available via `get_idf` alias (sources `$HOME/.espressif/v5.5.4/esp-idf/export.sh`). Flash script sources this automatically. + +### Iteration cycle + +```bash +cd examples/httpd_debug + +# 1. Edit code in src/ (library) or lib/ (test app) + +# 2. Build and flash (kills existing monitor, builds, flashes to ESP32) +./scripts/flash.sh + +# 3. Monitor serial output (logs to /tmp/atomvm_serial.log) +./scripts/monitor.sh +# Watch for: "HTTPD ready at http://X.X.X.X:80" + +# 4. Test via browser or automated suite +open http://X.X.X.X/ # Browser dashboard +./scripts/test.sh X.X.X.X # Automated curl tests + +# 5. Check serial log for crashes/errors +grep -i "error\|crash\|abort" /tmp/atomvm_serial.log + +# 6. Fix and repeat +``` + +### What's included + +- **Debug API endpoints** (`/api/ping`, `/api/echo`, `/api/generate?size=N`, `/api/memory`): + Stress-test request/response sizes up to 1MB. Every request logs heap state to serial. +- **Built-in stats** (`/api/stats/system`, `/api/stats/memory`): Platform info + ESP32 heap. +- **Command API** (`/api/cmd/restart`): Restart ESP32 over HTTP. +- **Browser dashboard** (`/`): Interactive UI for triggering tests, viewing results, monitoring memory. +- **Automated test suite** (`scripts/test.sh`): Sweeps response sizes (100B → 64KB) and upload sizes (100B → 16KB), reports pass/fail + timing. + +### Tuning parameters + +- **`chunk_size`** (default 4096): Set in `lib/httpd_debug.ex` line 13. AtomVM lwIP default send buffer is 8KB; values up to 8192 are safe. +- **Request timeout** (default 30s): Set via `:httpd.start_link/5` options map (not currently exposed in debug app, but easy to add). + +### AI agent workflow + +When debugging performance issues or crashes on hardware: + +1. **Flash**: `bash examples/httpd_debug/scripts/flash.sh` (Read tool to check output) +2. **Monitor**: `bash examples/httpd_debug/scripts/monitor.sh` in background, or Read `/tmp/atomvm_serial.log` +3. **Extract IP**: Grep serial log for `"HTTPD ready at http://"` +4. **Test**: `bash examples/httpd_debug/scripts/test.sh ` or individual curl commands +5. **Analyze**: Read serial log for crash traces (Guru Meditation, stack dumps), parse test failures +6. **Edit**: Make fixes in `src/` (library code) or `examples/httpd_debug/lib/` (test app) +7. **Iterate**: Return to step 1 + +Serial log persists at `/tmp/atomvm_serial.log` across monitor restarts for post-mortem analysis. diff --git a/examples/httpd_debug/.gitignore b/examples/httpd_debug/.gitignore new file mode 100644 index 0000000..8ce610c --- /dev/null +++ b/examples/httpd_debug/.gitignore @@ -0,0 +1,4 @@ +/_build +/deps +/*.avm +/.elixir_ls diff --git a/examples/httpd_debug/README.md b/examples/httpd_debug/README.md new file mode 100644 index 0000000..f852811 --- /dev/null +++ b/examples/httpd_debug/README.md @@ -0,0 +1,227 @@ +# AtomVM HTTPD Debug Example + +Complete debug/test application for iterating on ESP32 hardware with real-world HTTP loads. + +## Features + +- **WiFi STA connectivity** with compile-time credential injection +- **Debug API endpoints** for stress-testing various request/response sizes +- **Built-in stats handlers** for system info and memory monitoring +- **Browser test dashboard** with interactive controls +- **Automated test suite** with curl-based size sweeps +- **Serial monitoring** with persistent logs for debugging + +## Prerequisites + +1. **ESP32 with AtomVM base firmware flashed** +2. **ESP-IDF environment** (available via `get_idf` alias) +3. **WiFi network** (2.4 GHz for ESP32 compatibility) + +## Quick Start + +### 1. Set WiFi credentials + +```bash +export ATOMVM_WIFI_SSID="your-ssid" +export ATOMVM_WIFI_PSK="your-password" +``` + +### 2. Build and flash + +```bash +cd examples/httpd_debug +./scripts/flash.sh +``` + +The script will: +- Source ESP-IDF environment (for esptool.py) +- Kill any existing serial monitor +- Build the application with ExAtomVM +- Flash to `/dev/ttyACM0` at 921600 baud + +### 3. Monitor serial output + +```bash +./scripts/monitor.sh +``` + +Watch for the line: +``` +HTTPD ready at http://192.168.1.XXX:80 +``` + +Press `Ctrl+A` then `Ctrl+X` to exit picocom. + +Serial output is also logged to `/tmp/atomvm_serial.log` for later analysis. + +### 4. Test the server + +**Option A: Browser dashboard** +```bash +open http://192.168.1.XXX/ +``` + +The dashboard provides: +- Response size test (100B to 64KB) +- Upload test (100B to 16KB) +- Memory monitor +- System info display +- Live console log + +**Option B: Automated test suite** +```bash +./scripts/test.sh 192.168.1.XXX +``` + +Runs a comprehensive test suite: +- Connectivity (ping) +- Response generation (100B → 64KB) +- Upload echo (100B → 16KB) +- Stats endpoints validation + +## API Endpoints + +### Debug Endpoints + +| Endpoint | Method | Description | +|----------|--------|-------------| +| `/api/ping` | GET | Minimal response, baseline test | +| `/api/echo` | POST | Echoes back request body size + preview | +| `/api/generate?size=N` | GET | Generates N bytes of data (up to 1MB) | +| `/api/memory` | GET | Quick memory snapshot | + +### Built-in Endpoints + +| Endpoint | Method | Description | +|----------|--------|-------------| +| `/api/stats/system` | GET | Platform, AtomVM version, chip info | +| `/api/stats/memory` | GET | ESP32 heap stats (free, largest block, min free) | +| `/api/cmd/restart` | POST | Restart ESP32 (3 second delay) | +| `/` | GET | Browser test dashboard | + +## Development Workflow + +### Typical iteration cycle + +```bash +# 1. Edit code in ../../src/ (library) or ./lib/ (test app) +vim ../../src/httpd.erl + +# 2. Flash changes +./scripts/flash.sh + +# 3. Monitor in background or separate terminal +./scripts/monitor.sh & + +# 4. Wait for IP, then test +./scripts/test.sh 192.168.1.XXX + +# 5. Check for errors in serial log +grep -i "error\|crash" /tmp/atomvm_serial.log +``` + +### Debugging crashes + +If the ESP32 crashes (Guru Meditation), the stack trace is in `/tmp/atomvm_serial.log`: + +```bash +# Show crash details +grep -A 20 "Guru Meditation" /tmp/atomvm_serial.log + +# Show all errors +grep -i "error\|abort\|panic" /tmp/atomvm_serial.log +``` + +## Configuration + +### WiFi + +WiFi credentials are **compile-time** values (baked into the BEAM bytecode). + +To change WiFi: +```bash +export ATOMVM_WIFI_SSID="new-ssid" +export ATOMVM_WIFI_PSK="new-password" +./scripts/flash.sh +``` + +### Tuning + +**chunk_size** (default 4096): Edit `lib/httpd_debug.ex` line 13: +```elixir +@chunk_size 8192 # or another value +``` + +AtomVM lwIP default send buffer is 8KB, so values up to 8192 are safe. + +**request_timeout** (default 30s): Modify `start_httpd/0` to use `:httpd.start_link/5`: +```elixir +options = %{request_timeout: 60_000} # 60 seconds +:httpd.start_link(:any, port, socket_options, options, config) +``` + +## Serial Port + +Default: `/dev/ttyACM0` (ESP32-S3 native USB) + +If your ESP32 uses a different port (e.g., `/dev/ttyUSB0` for CP2102/CH340): +1. Edit `scripts/flash.sh` line 29: `--port /dev/ttyUSB0` +2. Edit `scripts/monitor.sh` line 11: `/dev/ttyUSB0` + +## Troubleshooting + +### Flash script fails with "esptool.py: command not found" + +The `get_idf` alias may not be configured correctly. Check: +```bash +type get_idf +# Should show: get_idf is aliased to `. $HOME/.espressif/v5.5.4/esp-idf/export.sh' +``` + +If missing, add to `~/.bashrc`: +```bash +alias get_idf='. $HOME/.espressif/v5.5.4/esp-idf/export.sh' +``` + +### "ATOMVM_WIFI_SSID not set" error + +You must export WiFi credentials **before** running `flash.sh`. They are compiled into the firmware. + +### Monitor shows garbage characters + +Wrong baud rate. The monitor uses 115200; if AtomVM is configured differently, update `scripts/monitor.sh` line 11. + +### Tests fail with "Connection refused" + +1. Check serial log for the actual IP address +2. Verify ESP32 and your computer are on the same network +3. Try pinging the ESP32: `ping 192.168.1.XXX` + +### Large response tests fail + +This is expected — it's why this debug app exists! Check: +1. Serial log for crash traces or OOM errors +2. Memory stats before/after via `/api/memory` +3. Try reducing `chunk_size` or increasing timeout + +## Files + +``` +examples/httpd_debug/ +├── mix.exs # ExAtomVM project config +├── lib/ +│ ├── httpd_debug.ex # Entrypoint, starts WiFi + HTTPD +│ └── httpd_debug/ +│ ├── wifi.ex # WiFi STA with callbacks +│ └── debug_api_handler.ex # Debug/test API endpoints +├── priv/ +│ └── index.html # Browser test dashboard +└── scripts/ + ├── flash.sh # Build and flash to ESP32 + ├── monitor.sh # Serial monitor with logging + └── test.sh # Automated curl test suite +``` + +## License + +Same as atomvm_httpd (Apache-2.0 OR LGPL-2.1-or-later). diff --git a/examples/httpd_debug/lib/httpd_debug.ex b/examples/httpd_debug/lib/httpd_debug.ex new file mode 100644 index 0000000..fbb252a --- /dev/null +++ b/examples/httpd_debug/lib/httpd_debug.ex @@ -0,0 +1,94 @@ +defmodule HttpdDebug do + @moduledoc """ + Debug/test application for atomvm_httpd on ESP32. + + This application demonstrates and stress-tests the HTTP server with: + - WiFi STA connectivity with DHCP + - Built-in stats and command handlers + - Debug API endpoints for testing large requests/responses + - Static file serving (test dashboard) + + Set WiFi credentials via environment variables before building: + export ATOMVM_WIFI_SSID="your-ssid" + export ATOMVM_WIFI_PSK="your-password" + """ + + require Logger + + @chunk_size 4096 + + def start do + IO.puts("HttpdDebug starting...") + + # Connect to WiFi and wait for IP + case HttpdDebug.WiFi.connect() do + {:ok, ip_info} -> + ip = format_ip(extract_ip(ip_info)) + IO.puts("HTTPD starting on http://#{ip}:80") + + # Start HTTP server with debug configuration + case start_httpd() do + {:ok, _pid} -> + IO.puts("HTTPD ready at http://#{ip}:80") + IO.puts("Open browser or run: ./scripts/test.sh #{ip}") + Process.sleep(:infinity) + + {:error, reason} -> + IO.puts("ERROR: Failed to start HTTPD: #{inspect(reason)}") + :erlang.halt(1) + end + + {:error, reason} -> + IO.puts("ERROR: WiFi connection failed: #{inspect(reason)}") + :erlang.halt(1) + end + end + + defp start_httpd do + port = 80 + + socket_options = %{ + chunk_size: @chunk_size + } + + config = [ + # Stats API at /api/stats/system and /api/stats/memory + {[<<"api">>, <<"stats">>], + %{ + handler: :httpd_api_handler, + handler_config: %{module: :httpd_stats_api_handler} + }}, + + # Command API at /api/cmd/restart + {[<<"api">>, <<"cmd">>], + %{ + handler: :httpd_api_handler, + handler_config: %{module: :httpd_cmd_api_handler} + }}, + + # Debug test endpoints at /api/* + {[<<"api">>], + %{ + handler: :httpd_api_handler, + handler_config: %{module: HttpdDebug.DebugApiHandler} + }}, + + # Static file handler for test dashboard (catch-all) + {[], + %{ + handler: :httpd_file_handler, + handler_config: %{app: :httpd_debug} + }} + ] + + :httpd.start_link(:any, port, socket_options, config) + end + + # AtomVM network driver returns {IP, Netmask, Gateway} tuple + defp extract_ip({ip, _netmask, _gateway}), do: ip + defp extract_ip(ip), do: ip + + defp format_ip({a, b, c, d}), do: "#{a}.#{b}.#{c}.#{d}" + defp format_ip(ip) when is_list(ip), do: to_string(ip) + defp format_ip(ip), do: inspect(ip) +end diff --git a/examples/httpd_debug/lib/httpd_debug/debug_api_handler.ex b/examples/httpd_debug/lib/httpd_debug/debug_api_handler.ex new file mode 100644 index 0000000..00e07e2 --- /dev/null +++ b/examples/httpd_debug/lib/httpd_debug/debug_api_handler.ex @@ -0,0 +1,137 @@ +defmodule HttpdDebug.DebugApiHandler do + @moduledoc """ + Debug/test API endpoints for stress-testing atomvm_httpd. + + Provides endpoints to test various request/response sizes and patterns. + """ + + @behaviour :httpd_api_handler + + def handle_api_request(:get, [<<"ping">>], _http_request, _args) do + heap_before = get_heap_info() + log_request(:get, "/api/ping", 0, heap_before) + + {:ok, %{status: "ok", heap: heap_before}} + end + + def handle_api_request(:post, [<<"echo">>], http_request, _args) do + body = Map.get(http_request, :body, <<>>) + body_size = byte_size(body) + heap_before = get_heap_info() + + log_request(:post, "/api/echo", body_size, heap_before) + + # Return size info without echoing full body (could be huge) + # Use binary pattern match instead of String.slice (not available on AtomVM) + preview = body_preview(body, body_size) + + {:ok, + %{ + status: "ok", + received_bytes: body_size, + body_preview: preview, + heap: heap_before + }} + end + + def handle_api_request(:get, [<<"generate">>], http_request, _args) do + query_params = Map.get(http_request, :query_params, %{}) + size = parse_size(query_params) + heap_before = get_heap_info() + + log_request(:get, "/api/generate?size=#{size}", 0, heap_before) + + # Generate N bytes of data using binary:copy (available on AtomVM) + data = :binary.copy(<<"A">>, size) + + {:ok, + %{ + status: "ok", + size: size, + data: data, + heap: heap_before + }} + end + + def handle_api_request(:get, [<<"memory">>], _http_request, _args) do + heap_info = get_heap_info() + log_request(:get, "/api/memory", 0, heap_info) + + {:ok, + %{ + free_heap: heap_info.free_heap, + largest_block: heap_info.largest_block, + min_free: heap_info.min_free + }} + end + + def handle_api_request(method, path, _http_request, _args) do + IO.puts("DebugApiHandler: Unsupported #{method} #{inspect(path)}") + :not_found + end + + defp parse_size(query_params) do + case Map.get(query_params, <<"size">>) do + nil -> + 1024 + + size_str when is_binary(size_str) -> + try do + size = :erlang.binary_to_integer(size_str) + if size > 0, do: min(size, 1_048_576), else: 1024 + rescue + _ -> 1024 + end + + size when is_integer(size) -> + min(size, 1_048_576) + + _ -> + 1024 + end + end + + # Return a safe JSON-encodable preview of the body. + # Binary data may contain non-UTF-8 bytes which json:encode rejects, + # so we return a hex-encoded version for non-text content. + defp body_preview(body, size) do + preview = if size <= 100 do + body + else + <> = body + p + end + # Check if all bytes are printable ASCII (safe for JSON) + if printable_ascii?(preview, 0, byte_size(preview)) do + preview + else + # Return byte count only — avoid encoding raw bytes as JSON string + size_str = :erlang.integer_to_binary(size) + <<"(binary: ", size_str::binary, " bytes)">> + end + end + + defp printable_ascii?(_bin, pos, len) when pos >= len, do: true + defp printable_ascii?(bin, pos, len) do + <<_::binary-size(pos), byte, _::binary>> = bin + if byte >= 32 and byte <= 126 do + printable_ascii?(bin, pos + 1, len) + else + false + end + end + + defp get_heap_info do + %{ + free_heap: :erlang.system_info(:esp32_free_heap_size), + largest_block: :erlang.system_info(:esp32_largest_free_block), + min_free: :erlang.system_info(:esp32_minimum_free_size) + } + end + + defp log_request(method, path, body_size, heap) do + IO.puts( + "DebugApi: #{method} #{path} | body=#{body_size}B | heap=#{heap.free_heap}B free" + ) + end +end diff --git a/examples/httpd_debug/lib/httpd_debug/wifi.ex b/examples/httpd_debug/lib/httpd_debug/wifi.ex new file mode 100644 index 0000000..eae6c12 --- /dev/null +++ b/examples/httpd_debug/lib/httpd_debug/wifi.ex @@ -0,0 +1,86 @@ +defmodule HttpdDebug.WiFi do + @moduledoc """ + WiFi STA connectivity for HttpdDebug. + + Reads WiFi credentials from compile-time environment variables + and establishes a connection to the configured access point. + """ + + @compile {:no_warn_undefined, :network} + + @wifi_ssid System.get_env("ATOMVM_WIFI_SSID") + @wifi_psk System.get_env("ATOMVM_WIFI_PSK") + @connect_timeout 15_000 + + def connect do + unless @wifi_ssid do + IO.puts("ERROR: ATOMVM_WIFI_SSID environment variable not set") + IO.puts("Set it before building: export ATOMVM_WIFI_SSID=\"your-ssid\"") + {:error, :missing_ssid} + else + IO.puts("WiFi: Connecting to #{@wifi_ssid}...") + start_network() + end + end + + defp start_network do + parent = self() + + sta_config = + [ + ssid: @wifi_ssid, + connected: fn -> handle_connected(parent) end, + disconnected: fn -> handle_disconnected(parent) end, + got_ip: fn ip_info -> handle_got_ip(parent, ip_info) end + ] + |> maybe_add_psk(@wifi_psk) + + network_config = [sta: sta_config] + + case :network.start(network_config) do + {:ok, _pid} -> + IO.puts("WiFi: Network driver started") + wait_for_ip() + + {:error, reason} -> + IO.puts("WiFi: Failed to start network driver: #{inspect(reason)}") + {:error, reason} + end + end + + defp maybe_add_psk(config, nil), do: config + defp maybe_add_psk(config, ""), do: config + defp maybe_add_psk(config, psk), do: Keyword.put(config, :psk, psk) + + defp wait_for_ip do + receive do + {:wifi_connected} -> + IO.puts("WiFi: Connected to AP") + wait_for_ip() + + {:wifi_disconnected} -> + IO.puts("WiFi: Disconnected from AP") + wait_for_ip() + + {:wifi_got_ip, ip_info} -> + IO.puts("WiFi: Got IP #{inspect(ip_info)}") + {:ok, ip_info} + after + @connect_timeout -> + IO.puts("WiFi: Timeout waiting for IP address") + {:error, :timeout} + end + end + + defp handle_connected(parent) do + send(parent, {:wifi_connected}) + end + + defp handle_disconnected(parent) do + send(parent, {:wifi_disconnected}) + end + + defp handle_got_ip(parent, ip_info) do + send(parent, {:wifi_got_ip, ip_info}) + end +end diff --git a/examples/httpd_debug/mix.exs b/examples/httpd_debug/mix.exs new file mode 100644 index 0000000..e07909e --- /dev/null +++ b/examples/httpd_debug/mix.exs @@ -0,0 +1,30 @@ +defmodule HttpdDebug.MixProject do + use Mix.Project + + def project do + [ + app: :httpd_debug, + version: "0.1.0", + elixir: "~> 1.18", + start_permanent: Mix.env() == :prod, + deps: deps(), + atomvm: [ + start: HttpdDebug, + flash_offset: 0x250000 + ] + ] + end + + def application do + [ + extra_applications: [:logger] + ] + end + + defp deps do + [ + {:atomvm_httpd, path: "../.."}, + {:exatomvm, github: "atomvm/exatomvm", runtime: false} + ] + end +end diff --git a/examples/httpd_debug/mix.lock b/examples/httpd_debug/mix.lock new file mode 100644 index 0000000..f56b5dd --- /dev/null +++ b/examples/httpd_debug/mix.lock @@ -0,0 +1,4 @@ +%{ + "exatomvm": {:git, "https://github.com/atomvm/exatomvm.git", "ff7daf7e83a4e86fbf078730b6c49045a99de9f8", []}, + "uf2tool": {:hex, :uf2tool, "1.1.0", "7091931234ca5a256b66ea691983867d51229798622d083f85c1ad779798a734", [:rebar3], [], "hexpm", "1a7e5ca7ef3d19c7a0b0acf3db804b3188e0980884acffa13fd57d733507b73d"}, +} diff --git a/examples/httpd_debug/priv/index.html b/examples/httpd_debug/priv/index.html new file mode 100644 index 0000000..8cb1eef --- /dev/null +++ b/examples/httpd_debug/priv/index.html @@ -0,0 +1,274 @@ + + + + + + AtomVM HTTPD Debug Dashboard + + + +
+

AtomVM HTTPD Debug Dashboard

+

Test and debug atomvm_httpd on ESP32

+ +
+
+

Response Size Test

+
+ + +
+ + + + + + + +
+
+ + +
+ +
+

Upload Test

+
+ + +
+ + + + + +
+
+ + +
+ +
+

Memory Monitor

+ + +
+ +
+

System Info

+ + +
+
+ +
+

Console Log

+
+
+
+ + + + diff --git a/examples/httpd_debug/scripts/flash.sh b/examples/httpd_debug/scripts/flash.sh new file mode 100755 index 0000000..6f958a0 --- /dev/null +++ b/examples/httpd_debug/scripts/flash.sh @@ -0,0 +1,37 @@ +#!/bin/bash +set -e + +echo "=== AtomVM HTTPD Debug - Flash Script ===" + +# Kill any existing serial monitor +echo "Stopping any existing serial monitor..." +pkill -f "picocom.*ttyACM0" 2>/dev/null || true +sleep 1 + +# Source ESP-IDF environment to get esptool.py +echo "Loading ESP-IDF environment..." +source "$HOME/.espressif/v5.5.4/esp-idf/export.sh" >/dev/null 2>&1 + +# Navigate to project directory +cd "$(dirname "$0")/.." + +# Check for WiFi credentials +if [ -z "$ATOMVM_WIFI_SSID" ]; then + echo "" + echo "ERROR: ATOMVM_WIFI_SSID not set" + echo "Set WiFi credentials before building:" + echo " export ATOMVM_WIFI_SSID=\"your-ssid\"" + echo " export ATOMVM_WIFI_PSK=\"your-password\"" + echo "" + exit 1 +fi + +echo "WiFi SSID: $ATOMVM_WIFI_SSID" + +# Build and flash +echo "Building and flashing to /dev/ttyACM0..." +mix atomvm.esp32.flash --port /dev/ttyACM0 --baud 921600 + +echo "" +echo "=== Flash complete ===" +echo "Run './scripts/monitor.sh' to view serial output" diff --git a/examples/httpd_debug/scripts/monitor.sh b/examples/httpd_debug/scripts/monitor.sh new file mode 100755 index 0000000..00501bf --- /dev/null +++ b/examples/httpd_debug/scripts/monitor.sh @@ -0,0 +1,14 @@ +#!/bin/bash + +echo "=== AtomVM HTTPD Debug - Serial Monitor ===" +echo "Monitoring /dev/ttyACM0 at 115200 baud" +echo "Log file: /tmp/atomvm_serial.log" +echo "Press Ctrl+A then Ctrl+X to exit picocom" +echo "" + +# Truncate log file +LOG=/tmp/atomvm_serial.log +> "$LOG" + +# Start picocom with tee to log file +picocom -b 115200 /dev/ttyACM0 | tee "$LOG" diff --git a/examples/httpd_debug/scripts/test.sh b/examples/httpd_debug/scripts/test.sh new file mode 100755 index 0000000..84fcafa --- /dev/null +++ b/examples/httpd_debug/scripts/test.sh @@ -0,0 +1,134 @@ +#!/bin/bash +set -e + +if [ -z "$1" ]; then + echo "Usage: $0 " + echo "Example: $0 192.168.1.100" + exit 1 +fi + +HOST="$1" +BASE="http://$HOST" +FAILED=0 +PASSED=0 +# Helper: increment without triggering set -e on zero-to-one transition +inc_passed() { PASSED=$((PASSED + 1)); } +inc_failed() { FAILED=$((FAILED + 1)); } + +echo "=== AtomVM HTTPD Debug - Automated Test Suite ===" +echo "Target: $BASE" +echo "" + +# Helper function to run a test +run_test() { + local name="$1" + local cmd="$2" + + echo -n "Testing: $name ... " + + if eval "$cmd" >/dev/null 2>&1; then + echo "✓ PASS" + inc_passed + return 0 + else + echo "✗ FAIL" + inc_failed + return 1 + fi +} + +# Test 1: Ping endpoint +echo "=== Basic Connectivity ===" +run_test "Ping" "curl -sf -m 5 '$BASE/api/ping'" || true +echo "" + +# Test 2: Memory endpoint +echo "=== Memory Info ===" +if curl -sf -m 5 "$BASE/api/memory" | grep -q '"free_heap"'; then + echo "✓ Memory endpoint working" + curl -sf "$BASE/api/memory" 2>/dev/null | head -5 + inc_passed +else + echo "✗ Memory endpoint failed" + inc_failed +fi +echo "" + +# Test 3: Response generation with increasing sizes +echo "=== Response Size Tests ===" +for SIZE in 100 500 1024 2048 4096 8192 16384 32768 65536; do + echo -n "Generate $SIZE bytes ... " + + START=$(date +%s%N) + # Scale timeout with size: 10s base + 1s per 4KB + TIMEOUT=$(( 10 + SIZE / 4096 )) + if RESPONSE=$(curl -sf -m $TIMEOUT "$BASE/api/generate?size=$SIZE" 2>/dev/null); then + END=$(date +%s%N) + ELAPSED=$(( (END - START) / 1000000 )) + + # Check if response is valid JSON + if echo "$RESPONSE" | grep -q '"data"'; then + ACTUAL_SIZE=$(echo "$RESPONSE" | grep -o '"data":"[^"]*"' | sed 's/"data":"//;s/"$//' | wc -c) + ACTUAL_SIZE=$((ACTUAL_SIZE - 1)) # Subtract newline + echo "✓ PASS (${ELAPSED}ms, ~${ACTUAL_SIZE} bytes)" + inc_passed + else + echo "✗ FAIL (invalid JSON response)" + inc_failed + fi + else + echo "✗ FAIL (request failed)" + inc_failed + fi +done +echo "" + +# Test 4: POST echo with increasing sizes +echo "=== Upload Size Tests ===" +for SIZE in 100 500 1024 2048 4096 8192 16384; do + echo -n "Upload $SIZE bytes ... " + + START=$(date +%s%N) + if RESPONSE=$(dd if=/dev/urandom bs=$SIZE count=1 2>/dev/null | \ + curl -sf -m 10 -X POST \ + -H "Content-Type: application/octet-stream" \ + --data-binary @- \ + "$BASE/api/echo" 2>/dev/null); then + END=$(date +%s%N) + ELAPSED=$(( (END - START) / 1000000 )) + + # Check if server received correct size + if echo "$RESPONSE" | grep -q '"received_bytes":'$SIZE; then + echo "✓ PASS (${ELAPSED}ms)" + inc_passed + else + RECEIVED=$(echo "$RESPONSE" | grep -o '"received_bytes":[0-9]*' | grep -o '[0-9]*') + echo "✗ FAIL (server received $RECEIVED bytes instead of $SIZE)" + inc_failed + fi + else + echo "✗ FAIL (request failed)" + inc_failed + fi +done +echo "" + +# Test 5: Stats endpoints +echo "=== Stats Endpoints ===" +run_test "System stats" "curl -sf -m 5 '$BASE/api/stats/system' | grep -q 'platform'" || true +run_test "Memory stats" "curl -sf -m 5 '$BASE/api/stats/memory' | grep -q 'esp32_free_heap_size'" || true +echo "" + +# Summary +echo "=== Test Summary ===" +echo "Passed: $PASSED" +echo "Failed: $FAILED" +echo "" + +if [ $FAILED -eq 0 ]; then + echo "✓ All tests passed!" + exit 0 +else + echo "✗ Some tests failed" + exit 1 +fi diff --git a/src/gen_tcp_server.erl b/src/gen_tcp_server.erl index 6996c14..2a818ec 100644 --- a/src/gen_tcp_server.erl +++ b/src/gen_tcp_server.erl @@ -30,7 +30,7 @@ {ok, State :: term()} | {stop, Reason :: term()}. -callback handle_receive(Socket :: term(), Packet :: binary(), State :: term()) -> - {reply, Packet :: iolist(), NewState :: term()} | {noreply, NewState :: term()} | {close, Packet :: iolist()} | close. + {reply, Packet :: iolist(), NewState :: term()} | {noreply, NewState :: term()} | {close, Packet :: iolist()} | {close, Packet :: iolist(), NewState :: term()} | close. -callback handle_tcp_closed(Socket :: term(), State :: term()) -> NewState :: term(). @@ -222,6 +222,17 @@ handle_tcp_data(Socket, Packet, State) -> try_close(Socket) end, {noreply, State}; + {close, ResponsePacket, ResponseState} -> + ?TRACE("Sending reply to endpoint ~p and closing socket: ~p", [socket:peername(Socket), Socket]), + case try_send(Socket, ResponsePacket, MaxChunk) of + ok -> + try_close(Socket); + {error, closed} -> + ok; + {error, _Reason} -> + try_close(Socket) + end, + {noreply, State#state{handler_state=ResponseState}}; close -> ?TRACE("Closing socket ~p", [Socket]), try_close(Socket), @@ -248,22 +259,13 @@ try_send(Socket, Byte, MaxChunk) when is_integer(Byte) -> ?TRACE("Sending byte ~p as ~p", [Byte, <>]), try_send(Socket, <>, MaxChunk); try_send(Socket, List, MaxChunk) when is_list(List) -> - case is_string(List) of - true -> - try_send(Socket, list_to_binary(List), MaxChunk); - _ -> - try_send_iolist(Socket, List, MaxChunk) - end. - -try_send_iolist(_Socket, [], _MaxChunk) -> - ok; -try_send_iolist(Socket, [H | T], MaxChunk) -> - case try_send(Socket, H, MaxChunk) of - ok -> - try_send_iolist(Socket, T, MaxChunk); - {error, _Reason} = Error -> - Error - end. + %% Flatten the entire iolist to a single binary before sending. + %% This avoids walking deeply nested iolists (e.g. from json:encode) + %% element-by-element, which generates hundreds of tiny socket:send + %% NIF calls. A single flattened binary lets try_send_binary chunk + %% it into MaxChunk slices — far fewer NIF crossings and much better + %% throughput on AtomVM/ESP32. + try_send_binary(Socket, erlang:iolist_to_binary(List), MaxChunk). try_send_binary(_Socket, <<>>, _MaxChunk) -> ok; @@ -273,35 +275,35 @@ try_send_binary(Socket, Packet, MaxChunk) when is_binary(Packet) -> <> = Packet, case socket:send(Socket, Chunk) of ok -> - %% Give the scheduler a chance to run and let TCP drain - maybe_yield(Rest), + %% Entire chunk accepted. Yield then continue with rest. + case byte_size(Rest) > 0 of + true -> receive after 0 -> ok end; + false -> ok + end, try_send_binary(Socket, Rest, MaxChunk); - {ok, Remaining} -> - %% Partial send - combine remaining with rest and retry - try_send_binary(Socket, <>, MaxChunk); + {ok, Unsent} -> + %% Partial send — on AtomVM/ESP32, socket:send only writes + %% TCP_MSS (1440) bytes per tcp_write call, so partial sends + %% are the norm, not the exception. When the TCP send buffer + %% is full (ERR_MEM), zero bytes are written and the full + %% chunk is returned. + %% + %% In all cases we yield for 10 ms to let the lwIP stack + %% transmit queued data and process incoming ACKs. Without + %% this delay the send loop can starve the TCP stack and + %% cause the connection to stall. + receive after 10 -> ok end, + try_send_binary(Socket, <>, MaxChunk); {error, closed} -> - %% Normal client behaviour (tab close, image swap, parallel request racing ahead). - %% Log at TRACE only — this fires dozens of times per minute on camera streams. ?TRACE("Connection closed mid-transfer (~p/~p bytes sent)", [ChunkSize, TotalSize]), {error, closed}; {error, Reason} -> - io:format("Send error: ~p (chunk: ~p, total: ~p)~n", + io:format("Send error: ~p (chunk: ~p, total: ~p)~n", [Reason, ChunkSize, TotalSize]), {error, Reason} end. -%% Lightweight yield using receive timeout - works in AtomVM -maybe_yield(<<>>) -> - ok; -maybe_yield(_) -> - receive after 0 -> ok end. - -is_string([]) -> - true; -is_string([H | T]) when is_integer(H) -> - is_string(T); -is_string(_) -> - false. + %% @private try_close(Socket) -> diff --git a/src/httpd.erl b/src/httpd.erl index 7b227d4..ecd5f49 100644 --- a/src/httpd.erl +++ b/src/httpd.erl @@ -142,13 +142,13 @@ handle_receive(Socket, Packet, State) -> ok -> {noreply, State}; Error -> - {close, create_error(?INTERNAL_SERVER_ERROR, Error)} + {close, create_error(?INTERNAL_SERVER_ERROR, Error), State} end end catch A:E:S -> io:format("Caught error: ~p:~p:~p~n", [A, E, S]), - {close, create_error(?BAD_REQUEST, E)} + {close, create_error(?BAD_REQUEST, E), State} end. %% @private @@ -173,7 +173,7 @@ handle_http_request(Socket, Packet, State) -> } = HttpRequest, case Method of undefined -> - {close, create_error(?NOT_ALLOWED, method_not_allowed)}; + {close, create_error(?NOT_ALLOWED, method_not_allowed), CleanState}; _ -> case get_protocol(Method, Headers) of http -> @@ -188,7 +188,7 @@ handle_http_request(Socket, Packet, State) -> }, handle_request_state(Socket, NewHttpRequest, CleanState); Error -> - {close, create_error(?INTERNAL_SERVER_ERROR, Error)} + {close, create_error(?INTERNAL_SERVER_ERROR, Error), CleanState} end; ws -> ?TRACE("Protocol is ws", []), @@ -214,18 +214,18 @@ handle_http_request(Socket, Packet, State) -> {reply, Reply, NewState}; Error -> ?TRACE("Web socket error: ~p", [Error]), - {close, create_error(?INTERNAL_SERVER_ERROR, {web_socket_error, Error})} + {close, create_error(?INTERNAL_SERVER_ERROR, {web_socket_error, Error}), CleanState} end; Error -> - {close, create_error(?INTERNAL_SERVER_ERROR, {web_socket_error, Error})} + {close, create_error(?INTERNAL_SERVER_ERROR, {web_socket_error, Error}), CleanState} end; error -> - {close, create_error(?BAD_REQUEST, missing_websocket_key)} + {close, create_error(?BAD_REQUEST, missing_websocket_key), CleanState} end end end; {error, Reason} -> - {close, create_error(?BAD_REQUEST, Reason)} + {close, create_error(?BAD_REQUEST, Reason), State} end; PendingHttpRequest -> ?TRACE("Packetlen: ~p", [erlang:byte_size(Packet)]), @@ -332,19 +332,19 @@ call_http_req_handler(Socket, HttpRequest, State) -> {close, State}; {close, Reply} -> ReplyPacket = create_reply(?OK, #{"Content-Type" => "application/octet-stream"}, Reply), - {close, ReplyPacket}; + {close, ReplyPacket, State}; {close, ReplyHeaders, Reply} -> ReplyPacket = create_reply(?OK, ReplyHeaders, Reply), - {close, ReplyPacket}; + {close, ReplyPacket, State}; %% errors {error, not_found} -> - {close, create_error(?NOT_FOUND, not_found)}; + {close, create_error(?NOT_FOUND, not_found), State}; {error, bad_request} -> - {close, create_error(?BAD_REQUEST, bad_request)}; + {close, create_error(?BAD_REQUEST, bad_request), State}; {error, internal_server_error} -> - {close, create_error(?INTERNAL_SERVER_ERROR, internal_server_error)}; + {close, create_error(?INTERNAL_SERVER_ERROR, internal_server_error), State}; HandlerError -> - {close, create_error(?INTERNAL_SERVER_ERROR, HandlerError)} + {close, create_error(?INTERNAL_SERVER_ERROR, HandlerError), State} end. %% @private diff --git a/src/httpd_api_handler.erl b/src/httpd_api_handler.erl index 4e36567..96739fc 100644 --- a/src/httpd_api_handler.erl +++ b/src/httpd_api_handler.erl @@ -65,12 +65,12 @@ handle_http_req(HttpRequest, State) -> {close, #{"Content-Type" => "text/plain"}, Reply}; {ok, Reply} when is_map(Reply) -> ?TRACE("Encoding reply ~p", [Reply]), - Body = json_encoder:encode(Reply), + Body = json:encode(Reply), ?TRACE("JSON Body: ~p", [Body]), {close, #{"Content-Type" => "application/json"}, Body}; {close, Reply} -> ?TRACE("Encoding reply ~p", [Reply]), - Body = json_encoder:encode(Reply), + Body = json:encode(Reply), ?TRACE("JSON Body: ~p", [Body]), {close, #{"Content-Type" => "application/json"}, Body}; Error -> @@ -80,7 +80,7 @@ handle_http_req(HttpRequest, State) -> case Fun(Method, PathSuffix, HttpRequest, Args) of {ok, Reply, State} -> ?TRACE("Encoding reply ~p", [Reply]), - Body = json_encoder:encode(Reply), + Body = json:encode(Reply), {close, #{"Content-Type" => "application/json"}, Body}; Error -> Error diff --git a/src/httpd_stats_api_handler.erl b/src/httpd_stats_api_handler.erl index 9d77ff7..398e702 100644 --- a/src/httpd_stats_api_handler.erl +++ b/src/httpd_stats_api_handler.erl @@ -48,23 +48,28 @@ get_system_info() -> #{ platform => atomvm:platform(), word_size => erlang:system_info(wordsize), - system_architecture => erlang:system_info(system_architecture), - atomvm_version => erlang:system_info(atomvm_version), + system_architecture => to_binary(erlang:system_info(system_architecture)), + atomvm_version => to_binary(erlang:system_info(atomvm_version)), esp32_chip_info => get_esp32_chip_info(), - esp_idf_version => list_to_binary(erlang:system_info(esp_idf_version)) + esp_idf_version => to_binary(erlang:system_info(esp_idf_version)) }. +to_binary(Value) when is_binary(Value) -> Value; +to_binary(Value) when is_list(Value) -> list_to_binary(Value); +to_binary(Value) when is_atom(Value) -> atom_to_binary(Value, utf8); +to_binary(_) -> <<"unknown">>. + get_esp32_chip_info() -> case erlang:system_info(esp32_chip_info) of undefined -> - undefined; + null; %% TODO remove old API {esp32, Features, Cores, Revision} -> - [{features, Features}, {cores, Cores}, {revision, Revision}, {model, undefined}]; + #{features => Features, cores => Cores, revision => Revision, model => null}; Info when is_map(Info) -> - maps:to_list(Info); + Info; _ -> - unknown + null end. get_memory() ->