Skip to content

Commit

Permalink
Fix issues in http_server
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
pguyot committed Nov 22, 2024
1 parent 11b9c1b commit a6161b4
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 11 additions & 5 deletions libs/eavmlib/src/http_server.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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) ->
Expand Down Expand Up @@ -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">>;
Expand Down
2 changes: 1 addition & 1 deletion src/platforms/generic_unix/lib/socket_driver.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions tests/libs/eavmlib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ set(ERLANG_MODULES
test_dir
test_file
test_ahttp_client
test_http_server
test_port
test_timer_manager
)
Expand Down
60 changes: 60 additions & 0 deletions tests/libs/eavmlib/test_http_server.erl
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions tests/libs/eavmlib/tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ start() ->
etest:test([
test_dir,
test_file,
test_http_server,
test_port,
test_timer_manager,
test_ahttp_client
Expand Down

0 comments on commit a6161b4

Please sign in to comment.