From a6161b4ceb8dc26743661152ff8da94dc6b73c6d Mon Sep 17 00:00:00 2001 From: Paul Guyot Date: Tue, 19 Nov 2024 22:42:44 +0100 Subject: [PATCH] Fix issues in http_server Fix a bug where http_server would fail to send a reply with integers. Fix another bug where http_server crashed with badmatch if connection was closed, including because of previous bug. Also fix socket_driver to return `{gen_tcp, closed}` when socket is closed on Linux instead of `{gen_tcp, {recv, 104}}` Fixes #1366 Signed-off-by: Paul Guyot --- CHANGELOG.md | 2 + libs/eavmlib/src/http_server.erl | 16 +++-- .../generic_unix/lib/socket_driver.c | 2 +- tests/libs/eavmlib/CMakeLists.txt | 1 + tests/libs/eavmlib/test_http_server.erl | 60 +++++++++++++++++++ tests/libs/eavmlib/tests.erl | 1 + 6 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 tests/libs/eavmlib/test_http_server.erl diff --git a/CHANGELOG.md b/CHANGELOG.md index ff89adcb4..83a75e5f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - ESP32: fix `gpio:init/1` on GPIO >= 32 - Adding missing check, passing a non numeric argument to a function expecting a floating point might lead to a crash in certain situations. +- Fixed several bugs in `http_server` (#1366) +- Fixed generic\_unix `socket_driver` to return `{gen_tcp, closed}` when socket is closed on Linux instead of `{gen_tcp, {recv, 104}}` ## [0.6.5] - 2024-10-15 diff --git a/libs/eavmlib/src/http_server.erl b/libs/eavmlib/src/http_server.erl index 1d95e7c86..2489a7e1c 100644 --- a/libs/eavmlib/src/http_server.erl +++ b/libs/eavmlib/src/http_server.erl @@ -73,12 +73,16 @@ find_route(Method, Path, [{Target, Mod, _Opts} | T]) -> end. reply(StatusCode, ReplyBody, Conn) -> - {ok, Conn} = reply( + {ok, NewConn} = reply( StatusCode, ReplyBody, [<<"Content-Type: text/html\r\nConnection: close\r\n">>], Conn ), - Socket = proplists:get_value(socket, Conn), + Socket = proplists:get_value(socket, NewConn), gen_tcp:close(Socket), - ClosedConn = [{closed, true} | Conn], + ClosedConn = + case proplists:get_value(closed, NewConn) of + undefined -> [{closed, true} | NewConn]; + true -> NewConn + end, {ok, ClosedConn}. reply(StatusCode, ReplyBody, ReplyHeaders, Conn) -> @@ -107,13 +111,15 @@ reply(StatusCode, ReplyBody, ReplyHeaders, Conn) -> {ok, ClosedConn} end. -send_reply(Socket, [Chunk | Tail]) -> +send_reply(Socket, [Chunk | Tail]) when is_list(Chunk) orelse is_binary(Chunk) -> case gen_tcp:send(Socket, Chunk) of ok -> send_reply(Socket, Tail); {error, _} = ErrorTuple -> ErrorTuple end; send_reply(_Socket, []) -> - ok. + ok; +send_reply(Socket, IOData) -> + gen_tcp:send(Socket, IOData). code_to_status_string(200) -> <<"200 OK">>; diff --git a/src/platforms/generic_unix/lib/socket_driver.c b/src/platforms/generic_unix/lib/socket_driver.c index 3e1792fd0..ebf52e83d 100644 --- a/src/platforms/generic_unix/lib/socket_driver.c +++ b/src/platforms/generic_unix/lib/socket_driver.c @@ -790,7 +790,7 @@ static EventListener *passive_recv_callback(GlobalContext *glb, EventListener *b return NULL; } SocketDriverData *socket_data = (SocketDriverData *) ctx->platform_data; - if (len == 0) { + if (len == 0 || (len < 0 && errno == ECONNRESET)) { // {Ref, {error, closed}} BEGIN_WITH_STACK_HEAP(12, heap); term pid = listener->pid; diff --git a/tests/libs/eavmlib/CMakeLists.txt b/tests/libs/eavmlib/CMakeLists.txt index 758764111..71be02036 100644 --- a/tests/libs/eavmlib/CMakeLists.txt +++ b/tests/libs/eavmlib/CMakeLists.txt @@ -26,6 +26,7 @@ set(ERLANG_MODULES test_dir test_file test_ahttp_client + test_http_server test_port test_timer_manager ) diff --git a/tests/libs/eavmlib/test_http_server.erl b/tests/libs/eavmlib/test_http_server.erl new file mode 100644 index 000000000..b82a487e7 --- /dev/null +++ b/tests/libs/eavmlib/test_http_server.erl @@ -0,0 +1,60 @@ +% +% This file is part of AtomVM. +% +% Copyright 2024 Paul Guyot +% +% Licensed under the Apache License, Version 2.0 (the "License"); +% you may not use this file except in compliance with the License. +% You may obtain a copy of the License at +% +% http://www.apache.org/licenses/LICENSE-2.0 +% +% Unless required by applicable law or agreed to in writing, software +% distributed under the License is distributed on an "AS IS" BASIS, +% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +% See the License for the specific language governing permissions and +% limitations under the License. +% +% SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later +% + +-module(test_http_server). +-export([test/0, handle_req/3]). + +test() -> + ok = test_chunk(). + +test_chunk() -> + Router = [ + {"*", ?MODULE, []} + ], + Pid = http_server:start_server(8080, Router), + {ok, Conn} = connect_client(5), + {ok, Conn2, _Ref} = ahttp_client:request(Conn, <<"GET">>, <<"/">>, [], undefined), + ok = loop_passive(Conn2, []), + exit(Pid, kill), + ok. + +connect_client(Retries) when Retries > 0 -> + ConnectResult = ahttp_client:connect(http, "localhost", 8080, [{active, false}]), + case ConnectResult of + {ok, Conn} -> + {ok, Conn}; + {error, _} = ConnectError -> + io:format("Request failed: ~p~n", [ConnectError]), + connect_client(Retries - 1) + end. + +handle_req("GET", [], Conn) -> + Body = [34, <<"hello">>], + http_server:reply(200, Body, Conn). + +% http_server doesn't send Content-Length, so ahttp_client doesn't know when it's done and reports closed connection +loop_passive(Conn, AccResp) -> + case ahttp_client:recv(Conn, 0) of + {ok, UpdatedConn, Responses} -> + loop_passive(UpdatedConn, lists:reverse(Responses, AccResp)); + {error, {gen_tcp, closed}} -> + [{data, _DataRef, <<"\"hello">>}, {status, _StatusRef, 200}] = AccResp, + ok + end. diff --git a/tests/libs/eavmlib/tests.erl b/tests/libs/eavmlib/tests.erl index c8ceac567..870b48c07 100644 --- a/tests/libs/eavmlib/tests.erl +++ b/tests/libs/eavmlib/tests.erl @@ -26,6 +26,7 @@ start() -> etest:test([ test_dir, test_file, + test_http_server, test_port, test_timer_manager, test_ahttp_client