Merge pull request #1370 from pguyot/w47/http-server-fixes

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

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

Author: bettio

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
 

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">>;

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;

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
 )

tests/libs/eavmlib/test_http_server.erl

@@ -0,0 +1,60 @@
+%
+% This file is part of AtomVM.
+%
+% Copyright 2024 Paul Guyot <[email protected]>
+%
+% 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.

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