From 87415c48541cd3b96ff21dd4dbdb42c9c81b5cea Mon Sep 17 00:00:00 2001 From: Jordan Bracco Date: Sun, 14 Jun 2020 18:21:17 +0200 Subject: [PATCH 1/7] Safeguard against ei_* failures --- lib/gen_magic/server.ex | 2 + src/apprentice.c | 78 ++++++++++++++++-------------- test/gen_magic/apprentice_test.exs | 68 ++++++++++++++------------ 3 files changed, 80 insertions(+), 68 deletions(-) diff --git a/lib/gen_magic/server.ex b/lib/gen_magic/server.ex index f057dda..b9a3d22 100644 --- a/lib/gen_magic/server.ex +++ b/lib/gen_magic/server.ex @@ -198,6 +198,8 @@ defmodule GenMagic.Server do 1 -> :no_database 2 -> :no_argument 3 -> :missing_database + 4 -> :ei_alloc_failed + 5 -> :ei_bad_term code -> {:unexpected_error, code} end diff --git a/src/apprentice.c b/src/apprentice.c index 9fb76be..806a288 100644 --- a/src/apprentice.c +++ b/src/apprentice.c @@ -58,8 +58,8 @@ #define ERROR_NO_DATABASE 1 #define ERROR_NO_ARGUMENT 2 #define ERROR_MISSING_DATABASE 3 -#define ERROR_BAD_TERM 4 -#define ERROR_EI 5 +#define ERROR_EI 4 +#define ERROR_BAD_TERM 5 // We use a bigger than possible valid command length (around 4111 bytes) to // allow more precise errors when using too long paths. @@ -69,6 +69,13 @@ #define MAGIC_FLAGS_COMMON (MAGIC_CHECK | MAGIC_ERROR) magic_t magic_setup(int flags); +#define EI_ENSURE(result) \ + do { \ + if (result != 0) { \ + exit(ERROR_EI); \ + } \ + } while (0); + typedef char byte; void setup_environment(); @@ -103,11 +110,10 @@ int main(int argc, char **argv) { setup_system(); ei_x_buff ok_buf; - if (ei_x_new_with_version(&ok_buf) || ei_x_encode_atom(&ok_buf, "ready")) - exit(ERROR_EI); + EI_ENSURE(ei_x_new_with_version(&ok_buf)); + EI_ENSURE(ei_x_encode_atom(&ok_buf, "ready")); write_cmd(ok_buf.buff, ok_buf.index); - if (ei_x_free(&ok_buf) != 0) - exit(ERROR_EI); + EI_ENSURE(ei_x_free(&ok_buf)); byte buf[COMMAND_BUFFER_SIZE]; uint16_t len; @@ -125,9 +131,8 @@ int process_command(uint16_t len, byte *buf) { index = 0; // Initialize result - if (ei_x_new_with_version(&result) || ei_x_encode_tuple_header(&result, 2)) { - exit(ERROR_EI); - } + EI_ENSURE(ei_x_new_with_version(&result)); + EI_ENSURE(ei_x_encode_tuple_header(&result, 2)); if (len >= COMMAND_LEN) { error(&result, "badarg"); @@ -160,7 +165,7 @@ int process_command(uint16_t len, byte *buf) { if (termtype == ERL_BINARY_EXT) { if (termsize < 4096) { long bin_length; - ei_decode_binary(buf, &index, path, &bin_length); + EI_ENSURE(ei_decode_binary(buf, &index, path, &bin_length)); path[termsize] = '\0'; process_file(path, &result); } else { @@ -175,11 +180,11 @@ int process_command(uint16_t len, byte *buf) { int termtype; int termsize; char bytes[51]; - ei_get_type(buf, &index, &termtype, &termsize); + EI_ENSURE(ei_get_type(buf, &index, &termtype, &termsize)); if (termtype == ERL_BINARY_EXT && termsize < 50) { long bin_length; - ei_decode_binary(buf, &index, bytes, &bin_length); + EI_ENSURE(ei_decode_binary(buf, &index, bytes, &bin_length)); bytes[termsize] = '\0'; process_bytes(bytes, termsize, &result); } else { @@ -195,9 +200,7 @@ int process_command(uint16_t len, byte *buf) { write_cmd(result.buff, result.index); - if (ei_x_free(&result) != 0) { - exit(ERROR_EI); - } + EI_ENSURE(ei_x_free(&result)); return 0; } @@ -254,7 +257,6 @@ void setup_options_file(char *optarg) { } void setup_options_default() { - struct magic_file *next = malloc(sizeof(struct magic_file)); next->path = NULL; next->prev = magic_database; @@ -314,22 +316,24 @@ void process_bytes(char *path, int size, ei_x_buff *result) { return; } - ei_x_encode_atom(result, "ok"); - ei_x_encode_tuple_header(result, 3); - ei_x_encode_binary(result, mime_type_result, strlen(mime_type_result)); - ei_x_encode_binary(result, mime_encoding_result, - strlen(mime_encoding_result)); - ei_x_encode_binary(result, type_name_result, strlen(type_name_result)); + EI_ENSURE(ei_x_encode_atom(result, "ok")); + EI_ENSURE(ei_x_encode_tuple_header(result, 3)); + EI_ENSURE( + ei_x_encode_binary(result, mime_type_result, strlen(mime_type_result))); + EI_ENSURE(ei_x_encode_binary(result, mime_encoding_result, + strlen(mime_encoding_result))); + EI_ENSURE( + ei_x_encode_binary(result, type_name_result, strlen(type_name_result))); return; } void handle_magic_error(magic_t handle, int errn, ei_x_buff *result) { const char *error = magic_error(handle); - ei_x_encode_atom(result, "error"); - ei_x_encode_tuple_header(result, 2); + EI_ENSURE(ei_x_encode_atom(result, "error")); + EI_ENSURE(ei_x_encode_tuple_header(result, 2)); long errlon = (long)errn; - ei_x_encode_long(result, errlon); - ei_x_encode_binary(result, error, strlen(error)); + EI_ENSURE(ei_x_encode_long(result, errlon)); + EI_ENSURE(ei_x_encode_binary(result, error, strlen(error))); return; } @@ -358,12 +362,14 @@ void process_file(char *path, ei_x_buff *result) { return; } - ei_x_encode_atom(result, "ok"); - ei_x_encode_tuple_header(result, 3); - ei_x_encode_binary(result, mime_type_result, strlen(mime_type_result)); - ei_x_encode_binary(result, mime_encoding_result, - strlen(mime_encoding_result)); - ei_x_encode_binary(result, type_name_result, strlen(type_name_result)); + EI_ENSURE(ei_x_encode_atom(result, "ok")); + EI_ENSURE(ei_x_encode_tuple_header(result, 3)); + EI_ENSURE( + ei_x_encode_binary(result, mime_type_result, strlen(mime_type_result))); + EI_ENSURE(ei_x_encode_binary(result, mime_encoding_result, + strlen(mime_encoding_result))); + EI_ENSURE( + ei_x_encode_binary(result, type_name_result, strlen(type_name_result))); return; } @@ -428,12 +434,10 @@ size_t write_cmd(byte *buf, size_t len) { } void error(ei_x_buff *result, const char *error) { - ei_x_encode_atom(result, "error"); - ei_x_encode_atom(result, error); + EI_ENSURE(ei_x_encode_atom(result, "error")); + EI_ENSURE(ei_x_encode_atom(result, error)); write_cmd(result->buff, result->index); - - if (ei_x_free(result) != 0) - exit(ERROR_EI); + EI_ENSURE(ei_x_free(result)); } void fdseek(uint16_t count) { diff --git a/test/gen_magic/apprentice_test.exs b/test/gen_magic/apprentice_test.exs index 7c6b4b5..8f50ea8 100644 --- a/test/gen_magic/apprentice_test.exs +++ b/test/gen_magic/apprentice_test.exs @@ -2,6 +2,7 @@ defmodule GenMagic.ApprenticeTest do use GenMagic.MagicCase @tmp_path "/tmp/testgenmagicx" + require Logger test "sends ready" do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) @@ -43,7 +44,7 @@ defmodule GenMagic.ApprenticeTest do test "exits with badly formatted erlang terms", %{port: port} do send(port, {self(), {:command, "i forgot to term_to_binary!!"}}) - assert_receive {^port, {:exit_status, 4}} + assert_receive {^port, {:exit_status, 5}} end test "errors with wrong command", %{port: port} do @@ -89,37 +90,42 @@ defmodule GenMagic.ApprenticeTest do test "works with big file path", %{port: port} do # Test with longest valid path. {dir, bigfile} = too_big(@tmp_path, "/a") - File.mkdir_p!(dir) - File.touch!(bigfile) - on_exit(fn -> File.rm_rf!(@tmp_path) end) - send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}}) - assert_receive {^port, {:data, data}} - assert {:ok, _} = :erlang.binary_to_term(data) - refute_receive _ + case File.mkdir_p(dir) do + :ok -> + File.touch!(bigfile) + on_exit(fn -> File.rm_rf!(@tmp_path) end) + send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}}) + assert_receive {^port, {:data, data}} + assert {:ok, _} = :erlang.binary_to_term(data) + refute_receive _ - # This path should be long enough for buffers, but larger than a valid path name. Magic will return an errno 36. - file = @tmp_path <> String.duplicate("a", 256) - send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) - assert_receive {^port, {:data, data}} - assert {:error, {36, _}} = :erlang.binary_to_term(data) - refute_receive _ - # Theses filename should be too big for the path buffer. - file = bigfile <> "aaaaaaaaaa" - send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) - assert_receive {^port, {:data, data}} - assert {:error, :enametoolong} = :erlang.binary_to_term(data) - refute_receive _ - # This call should be larger than the COMMAND_BUFFER_SIZE. Ensure nothing bad happens! - file = String.duplicate(bigfile, 4) - send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) - assert_receive {^port, {:data, data}} - assert {:error, :badarg} = :erlang.binary_to_term(data) - refute_receive _ - # We re-run a valid call to ensure the buffer/... haven't been corrupted in port land. - send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}}) - assert_receive {^port, {:data, data}} - assert {:ok, _} = :erlang.binary_to_term(data) - refute_receive _ + # This path should be long enough for buffers, but larger than a valid path name. Magic will return an errno 36. + file = @tmp_path <> String.duplicate("a", 256) + send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) + assert_receive {^port, {:data, data}} + assert {:error, {36, _}} = :erlang.binary_to_term(data) + refute_receive _ + # Theses filename should be too big for the path buffer. + file = bigfile <> "aaaaaaaaaa" + send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) + assert_receive {^port, {:data, data}} + assert {:error, :enametoolong} = :erlang.binary_to_term(data) + refute_receive _ + # This call should be larger than the COMMAND_BUFFER_SIZE. Ensure nothing bad happens! + file = String.duplicate(bigfile, 4) + send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) + assert_receive {^port, {:data, data}} + assert {:error, :badarg} = :erlang.binary_to_term(data) + refute_receive _ + # We re-run a valid call to ensure the buffer/... haven't been corrupted in port land. + send(port, {self(), {:command, :erlang.term_to_binary({:file, bigfile})}}) + assert_receive {^port, {:data, data}} + assert {:ok, _} = :erlang.binary_to_term(data) + refute_receive _ + {:error, :enametoolong} -> + Logger.info("Skipping test, operating system does not support max POSIX length for directories") + :ignore + end end end From 22024701a5b63b7908b05a238cd01495183fd237 Mon Sep 17 00:00:00 2001 From: Jordan Bracco Date: Sun, 14 Jun 2020 18:39:46 +0200 Subject: [PATCH 2/7] Build custom database in test --- Makefile | 12 ++---------- test/gen_magic/gen_magic_test.exs | 4 +++- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index cb2e93d..cbd6fec 100644 --- a/Makefile +++ b/Makefile @@ -6,24 +6,16 @@ CFLAGS = -std=c99 -g -Wall -Werror CPPFLAGS = -I$(ERL_EI_INCLUDE) LDFLAGS = -L$(ERL_EI_LIB) LDLIBS = -lpthread -lei -lm -lmagic -BEAM_FILES = _build/ PRIV = priv/ RM = rm -Rf -# Unit test custom magic file - -MAGIC = file - -all: priv/apprentice test/elixir.mgc - -test/%.mgc: test/% - cd test; file -C -m ../$^ +all: priv/apprentice priv/apprentice: src/apprentice.c mkdir -p priv $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) $^ $(LDLIBS) -o $@ clean: - $(RM) $(PRIV) $(BEAM_FILES) test/*.mgc + $(RM) $(PRIV) .PHONY: clean diff --git a/test/gen_magic/gen_magic_test.exs b/test/gen_magic/gen_magic_test.exs index dbd287c..9699177 100644 --- a/test/gen_magic/gen_magic_test.exs +++ b/test/gen_magic/gen_magic_test.exs @@ -42,7 +42,9 @@ defmodule GenMagicTest do end test "Custom database file recognises Elixir files" do - database = absolute_path("test/elixir.mgc") + database = absolute_path("elixir.mgc") + on_exit(fn() -> File.rm(database) end) + {_, 0} = System.cmd("file", ["-C", "-m", absolute_path("test/elixir")]) {:ok, pid} = GenMagic.Server.start_link(database_patterns: [database]) path = absolute_path("mix.exs") assert {:ok, %Result{} = result} = GenMagic.Server.perform(pid, path) From d358007c2661e13cb6212b2bee6b70d9318301b8 Mon Sep 17 00:00:00 2001 From: Jordan Bracco Date: Sun, 14 Jun 2020 18:54:04 +0200 Subject: [PATCH 3/7] Ensure we always close the port --- lib/gen_magic/server.ex | 10 ++++++++++ test/gen_magic/apprentice_test.exs | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/lib/gen_magic/server.ex b/lib/gen_magic/server.ex index b9a3d22..fc686b2 100644 --- a/lib/gen_magic/server.ex +++ b/lib/gen_magic/server.ex @@ -273,6 +273,16 @@ defmodule GenMagic.Server do {:next_state, :starting, %{data | port: nil, cycles: 0}} end + @doc false + @impl :gen_statem + def terminate(_, _, %{port: port}) do + Kernel.send(port, {self(), :close}) + end + + def terminate(_, _, _) do + :ok + end + defp send(port, command) do Kernel.send(port, {self(), {:command, :erlang.term_to_binary(command)}}) end diff --git a/test/gen_magic/apprentice_test.exs b/test/gen_magic/apprentice_test.exs index 8f50ea8..bffd3c7 100644 --- a/test/gen_magic/apprentice_test.exs +++ b/test/gen_magic/apprentice_test.exs @@ -6,11 +6,13 @@ defmodule GenMagic.ApprenticeTest do test "sends ready" do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) + on_exit(fn() -> send(port, {self(), :close}) end) assert_ready(port) end test "stops" do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) + on_exit(fn() -> send(port, {self(), :close}) end) assert_ready(port) send(port, {self(), {:command, :erlang.term_to_binary({:stop, :stop})}}) assert_receive {^port, {:exit_status, 0}} @@ -19,6 +21,7 @@ defmodule GenMagic.ApprenticeTest do test "exits with no database" do opts = [:use_stdio, :binary, :exit_status, {:packet, 2}, {:args, []}] port = Port.open(GenMagic.Config.get_port_name(), opts) + on_exit(fn() -> send(port, {self(), :close}) end) assert_receive {^port, {:exit_status, 1}} end @@ -32,12 +35,14 @@ defmodule GenMagic.ApprenticeTest do ] port = Port.open(GenMagic.Config.get_port_name(), opts) + on_exit(fn() -> send(port, {self(), :close}) end) assert_receive {^port, {:exit_status, 3}} end describe "port" do setup do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) + on_exit(fn() -> send(port, {self(), :close}) end) assert_ready(port) %{port: port} end From 08ebe0575b9943614a67dc90f2e1551f91a36c62 Mon Sep 17 00:00:00 2001 From: Jordan Bracco Date: Sun, 14 Jun 2020 19:10:41 +0200 Subject: [PATCH 4/7] Shorten a comment to make Credo happy --- test/gen_magic/apprentice_test.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/gen_magic/apprentice_test.exs b/test/gen_magic/apprentice_test.exs index bffd3c7..f7f0bb1 100644 --- a/test/gen_magic/apprentice_test.exs +++ b/test/gen_magic/apprentice_test.exs @@ -104,7 +104,8 @@ defmodule GenMagic.ApprenticeTest do assert {:ok, _} = :erlang.binary_to_term(data) refute_receive _ - # This path should be long enough for buffers, but larger than a valid path name. Magic will return an errno 36. + # This path should be long enough for buffers, but larger than a valid path name. + # Magic will return an errno 36. file = @tmp_path <> String.duplicate("a", 256) send(port, {self(), {:command, :erlang.term_to_binary({:file, file})}}) assert_receive {^port, {:data, data}} From 011d1150e3eff6e95e82ed9ae92db5aa91367d54 Mon Sep 17 00:00:00 2001 From: Jordan Bracco Date: Mon, 15 Jun 2020 17:57:40 +0200 Subject: [PATCH 5/7] Remove use of arguments and load DB from messages Also: - Adds `GenMagic.Server.recycle` and `GenMagic.Server.reload` which allows to restart/reload the apprentice server with a new set of databases, - Fix compilation on musl based distributions (was caused by the args code), - Handle timeouts better in `GenMagic.Server` - Force the port to close in recycling if the graceful stop timeouts --- lib/gen_magic/config.ex | 17 +-- lib/gen_magic/server.ex | 129 ++++++++++++++++-- lib/gen_magic/server/data.ex | 1 + src/apprentice.c | 206 +++++++++++++---------------- test/gen_magic/apprentice_test.exs | 46 ++++--- test/gen_magic/gen_magic_test.exs | 39 ++++-- test/gen_magic/server_test.exs | 11 ++ 7 files changed, 279 insertions(+), 170 deletions(-) diff --git a/lib/gen_magic/config.ex b/lib/gen_magic/config.ex index 0ee2ace..70ae355 100644 --- a/lib/gen_magic/config.ex +++ b/lib/gen_magic/config.ex @@ -5,19 +5,13 @@ defmodule GenMagic.Config do @startup_timeout 1_000 @process_timeout 30_000 @recycle_threshold :infinity - @database_patterns [:default] def get_port_name do {:spawn_executable, to_charlist(get_executable_name())} end - def get_port_options(options) do - arguments = [:use_stdio, :binary, :exit_status, {:packet, 2}] - - case get_executable_arguments(options) do - [] -> arguments - list -> [{:args, list} | arguments] - end + def get_port_options(_options) do + [:use_stdio, :binary, :exit_status, {:packet, 2}] end def get_startup_timeout(options) do @@ -36,13 +30,6 @@ defmodule GenMagic.Config do Path.join(:code.priv_dir(@otp_app), @executable_name) end - defp get_executable_arguments(options) do - Enum.flat_map(List.wrap(get(options, :database_patterns, @database_patterns)), fn - :default -> ["--database-default"] - pattern -> pattern |> Path.wildcard() |> Enum.flat_map(&["--database-file", &1]) - end) - end - defp get(options, key, default) do Keyword.get(options, key, default) end diff --git a/lib/gen_magic/server.ex b/lib/gen_magic/server.ex index fc686b2..545b024 100644 --- a/lib/gen_magic/server.ex +++ b/lib/gen_magic/server.ex @@ -10,6 +10,7 @@ defmodule GenMagic.Server do alias GenMagic.Server.Data alias GenMagic.Server.Status import Kernel, except: [send: 2] + require Logger @typedoc """ Represents the reference to the underlying server, as returned by `:gen_statem`. @@ -48,6 +49,7 @@ defmodule GenMagic.Server do [:default, "path/to/my/magic"] """ + @database_patterns [:default] @type option :: {:name, atom() | :gen_statem.server_name()} | {:startup_timeout, timeout()} @@ -82,7 +84,7 @@ defmodule GenMagic.Server do @spec child_spec([option()]) :: Supervisor.child_spec() @spec start_link([option()]) :: :gen_statem.start_ret() @spec perform(t(), Path.t() | {:bytes, binary()}, timeout()) :: - {:ok, Result.t()} | {:error, term() | String.t()} + {:ok, Result.t()} | {:error, term() | String.t()} @spec status(t(), timeout()) :: {:ok, Status.t()} | {:error, term()} @spec stop(t(), term(), timeout()) :: :ok @@ -128,6 +130,20 @@ defmodule GenMagic.Server do end end + @doc """ + Reloads a Server with a new set of databases. + """ + def reload(server_ref, database_patterns \\ nil, timeout \\ 5000) do + :gen_statem.call(server_ref, {:reload, database_patterns}, timeout) + end + + @doc """ + Same as `reload/2,3` but with a full restart of the underlying C port. + """ + def recycle(server_ref, database_patterns \\ nil, timeout \\ 5000) do + :gen_statem.call(server_ref, {:recycle, database_patterns}, timeout) + end + @doc """ Returns status of the Server. """ @@ -155,6 +171,7 @@ defmodule GenMagic.Server do data = %Data{ port_name: get_port_name(), + database_patterns: Keyword.get(options, :database_patterns, []), port_options: get_port_options(options), startup_timeout: get_startup_timeout(options), process_timeout: get_process_timeout(options), @@ -170,11 +187,16 @@ defmodule GenMagic.Server do end @doc false - def starting(:enter, _, %{request: nil, port: nil} = data) do + def starting(:enter, _, %{port: nil} = data) do port = Port.open(data.port_name, data.port_options) {:keep_state, %{data | port: port}, data.startup_timeout} end + @doc false + def starting(:enter, _, data) do + {:keep_state_and_data, data.startup_timeout} + end + @doc false def starting({:call, from}, :status, data) do handle_status_call(from, :starting, data) @@ -188,25 +210,79 @@ defmodule GenMagic.Server do @doc false def starting(:info, {port, {:data, ready}}, %{port: port} = data) do case :erlang.binary_to_term(ready) do - :ready -> {:next_state, :available, data} + :ready -> {:next_state, :loading, data} end end def starting(:info, {port, {:exit_status, code}}, %{port: port} = data) do error = case code do - 1 -> :no_database - 2 -> :no_argument - 3 -> :missing_database - 4 -> :ei_alloc_failed - 5 -> :ei_bad_term + 1 -> :bad_db + 2 -> :ei_error + 3 -> :ei_bad_term code -> {:unexpected_error, code} end {:stop, {:error, error}, data} end + def loading(:enter, _old_state, data) do + databases = Enum.flat_map(List.wrap(data.database_patterns || @database_patterns), fn + :default -> [:default] + pattern -> Path.wildcard(pattern) + end) + databases = if databases == [] do + [:default] + else + databases + end + {:keep_state, {databases, data}, {:state_timeout, 0, :load}} + end + + def loading(:state_timeout, :load_timeout, {[database | _], data}) do + {:stop, {:error, {:database_loading_timeout, database}}, data} + end + + def loading(:state_timeout, :load, {[], data}) do + {:next_state, :available, data} + end + + def loading(:state_timeout, :load, {[database | databases], data} = state) do + command = case database do + :default -> {:add_default_database, nil} + path -> {:add_database, database} + end + send(data.port, command) + {:keep_state, state, {:state_timeout, data.startup_timeout, :load_timeout}} + end + + def loading(:info, {port, {:data, response}}, {[database | databases], %{port: port} = data}) do + case :erlang.binary_to_term(response) do + {:ok, :loaded} -> + {:keep_state, {databases, data}, {:state_timeout, 0, :load}} + end + end + + def loading(:info, {port, {:exit_status, 1}}, {[database | _], %{port: port} = data}) do + {:stop, {:error, {:database_not_found, database}}, data} + end + @doc false + def loading({:call, from}, :status, {[database | _], data}) do + handle_status_call(from, :loading, data) + end + + @doc false + def loading({:call, _from}, {:perform, _path}, _data) do + {:keep_state_and_data, :postpone} + end + + @doc false + def available(:enter, _old_state, %{request: {:reload, from, _}}) do + response = {:reply, from, :ok} + {:keep_state_and_data, response} + end + def available(:enter, _old_state, %{request: nil}) do :keep_state_and_data end @@ -225,6 +301,17 @@ defmodule GenMagic.Server do {:next_state, :processing, data} end + def available({:call, from}, {:reload, databases}, data) do + send(data.port, {:reload, :reload}) + {:next_state, :starting, + %{data | database_patterns: databases || data.database_patterns, request: {:reload, from, :reload}}} + end + + def available({:call, from}, {:recycle, databases}, data) do + {:next_state, :recycling, + %{data | database_patterns: databases || data.database_patterns, request: {:reload, from, :recycle}}} + end + @doc false def available({:call, from}, :status, data) do handle_status_call(from, :available, data) @@ -245,6 +332,12 @@ defmodule GenMagic.Server do handle_status_call(from, :processing, data) end + @doc false + def processing(:state_timeout, _, %{port: port, request: {_, from, _}} = data) do + response = {:reply, from, {:error, :timeout}} + {:next_state, :recycling, %{data | request: nil}, [response, :hibernate]} + end + @doc false def processing(:info, {port, {:data, response}}, %{port: port, request: {_, from, _}} = data) do response = {:reply, from, handle_response(response)} @@ -253,9 +346,9 @@ defmodule GenMagic.Server do end @doc false - def recycling(:enter, _, %{request: nil, port: port} = data) when is_port(port) do + def recycling(:enter, _, %{port: port} = data) when is_port(port) do send(data.port, {:stop, :recycle}) - {:keep_state_and_data, data.startup_timeout} + {:keep_state_and_data, {:state_timeout, data.startup_timeout, :stop}} end @doc false @@ -268,8 +361,22 @@ defmodule GenMagic.Server do handle_status_call(from, :recycling, data) end + # In case of timeout, force close. + def recycling(:state_timeout, :stop, data) do + Kernel.send(data.port, {self(), :close}) + {:keep_state_and_data, {:state_timeout, data.startup_timeout, :close}} + end + + def recycling(:state_timeout, :close, data) do + {:stop, {:error, :port_close_failed}} + end + + def recycling(:info, {port, :closed}, %{port: port} = data) do + {:next_state, :starting, %{data | port: nil, cycles: 0}} + end + @doc false - def recycling(:info, {port, {:exit_status, 0}}, %{port: port} = data) do + def recycling(:info, {port, {:exit_status, _}}, %{port: port} = data) do {:next_state, :starting, %{data | port: nil, cycles: 0}} end diff --git a/lib/gen_magic/server/data.ex b/lib/gen_magic/server/data.ex index 6836327..25d23e9 100644 --- a/lib/gen_magic/server/data.ex +++ b/lib/gen_magic/server/data.ex @@ -21,5 +21,6 @@ defmodule GenMagic.Server.Data do process_timeout: :infinity, recycle_threshold: :infinity, cycles: 0, + database_patterns: nil, request: nil end diff --git a/src/apprentice.c b/src/apprentice.c index 806a288..da8afaf 100644 --- a/src/apprentice.c +++ b/src/apprentice.c @@ -6,35 +6,35 @@ // yum or brew. Refer to the Makefile for further reference. // // This program is designed to run interactively as a backend daemon to the -// GenMagic library, and follows the command line pattern: -// -// $ apprentice --database-file --database-default -// -// Where each argument either refers to a compiled or uncompiled magic database, -// or the default database. They will be loaded in the sequence that they were -// specified. Note that you must specify at least one database. +// GenMagic library. // // Communication is done over STDIN/STDOUT as binary packets of 2 bytes length // plus X bytes payload, where the payload is an erlang term encoded with // :erlang.term_to_binary/1 and decoded with :erlang.binary_to_term/1. // -// Once the program is ready, it sends the `:ready` atom. The startup can fail -// for multiples reasons, and the program will exit accordingly: -// - 1: No database -// - 2: Missing/Bad argument -// - 3: Missing database +// Once the program is ready, it sends the `:ready` atom. +// +// It is then up to the Erlang side to load databases, by sending messages: +// - `{:add_database, path}` +// - `{:add_default_database, _}` +// +// If the requested database have been loaded, an `{:ok, :loaded}` message will +// follow. Otherwise, the process will exit (exit code 1). // // Commands are sent to the program STDIN as an erlang term of `{Operation, // Argument}`, and response of `{:ok | :error, Response}`. // -// Invalid packets will cause the program to exit (exit code 4). This will +// Invalid packets will cause the program to exit (exit code 3). This will // happen if your Erlang Term format doesn't match the version the program has // been compiled with, or if you send a command too huge. // -// The program may exit with error codes 5 or 255 if something went wrong (such -// as error allocating terms, or if stdin is lost). +// The program may exit with exit code 3 if something went wrong with ei_* +// functions. // // Commands: +// {:reload, _} :: :ready +// {:add_database, String.t()} :: {:ok, _} | {:error, _} +// {:add_default_database, _} :: {:ok, _} | {:error, _} // {:file, path :: String.t()} :: {:ok, {type, encoding, name}} | {:error, // :badarg} | {:error, {errno :: integer(), String.t()}} // {:bytes, binary()} :: same as :file @@ -55,11 +55,9 @@ #include #define ERROR_OK 0 -#define ERROR_NO_DATABASE 1 -#define ERROR_NO_ARGUMENT 2 -#define ERROR_MISSING_DATABASE 3 -#define ERROR_EI 4 -#define ERROR_BAD_TERM 5 +#define ERROR_DB 1 +#define ERROR_EI 2 +#define ERROR_BAD_TERM 3 // We use a bigger than possible valid command length (around 4111 bytes) to // allow more precise errors when using too long paths. @@ -72,6 +70,7 @@ magic_t magic_setup(int flags); #define EI_ENSURE(result) \ do { \ if (result != 0) { \ + fprintf(stderr, "EI ERROR, line: %d", __LINE__); \ exit(ERROR_EI); \ } \ } while (0); @@ -79,10 +78,8 @@ magic_t magic_setup(int flags); typedef char byte; void setup_environment(); -void setup_options(int argc, char **argv); -void setup_options_file(char *optarg); -void setup_options_default(); -void setup_system(); +void magic_open_all(); +int magic_load_all(char *path); int process_command(uint16_t len, byte *buf); void process_file(char *path, ei_x_buff *result); void process_bytes(char *bytes, int size, ei_x_buff *result); @@ -92,28 +89,14 @@ void error(ei_x_buff *result, const char *error); void handle_magic_error(magic_t handle, int errn, ei_x_buff *result); void fdseek(uint16_t count); -struct magic_file { - struct magic_file *prev; - struct magic_file *next; - char *path; -}; - -static struct magic_file *magic_database; static magic_t magic_mime_type; // MAGIC_MIME_TYPE static magic_t magic_mime_encoding; // MAGIC_MIME_ENCODING static magic_t magic_type_name; // MAGIC_NONE int main(int argc, char **argv) { - ei_init(); + EI_ENSURE(ei_init()); setup_environment(); - setup_options(argc, argv); - setup_system(); - - ei_x_buff ok_buf; - EI_ENSURE(ei_x_new_with_version(&ok_buf)); - EI_ENSURE(ei_x_encode_atom(&ok_buf, "ready")); - write_cmd(ok_buf.buff, ok_buf.index); - EI_ENSURE(ei_x_free(&ok_buf)); + magic_open_all(); byte buf[COMMAND_BUFFER_SIZE]; uint16_t len; @@ -158,6 +141,7 @@ int process_command(uint16_t len, byte *buf) { return 1; } + // {:file, path} if (strlen(atom) == 4 && strncmp(atom, "file", 4) == 0) { char path[4097]; ei_get_type(buf, &index, &termtype, &termsize); @@ -176,6 +160,7 @@ int process_command(uint16_t len, byte *buf) { error(&result, "badarg"); return 1; } + // {:bytes, bytes} } else if (strlen(atom) == 5 && strncmp(atom, "bytes", 5) == 0) { int termtype; int termsize; @@ -191,8 +176,47 @@ int process_command(uint16_t len, byte *buf) { error(&result, "badarg"); return 1; } + // {:add_database, path} + } else if (strlen(atom) == 12 && strncmp(atom, "add_database", 12) == 0) { + char path[4097]; + ei_get_type(buf, &index, &termtype, &termsize); + + if (termtype == ERL_BINARY_EXT) { + if (termsize < 4096) { + long bin_length; + EI_ENSURE(ei_decode_binary(buf, &index, path, &bin_length)); + path[termsize] = '\0'; + if (magic_load_all(path) == 0) { + EI_ENSURE(ei_x_encode_atom(&result, "ok")); + EI_ENSURE(ei_x_encode_atom(&result, "loaded")); + } else { + exit(ERROR_DB); + } + } else { + error(&result, "enametoolong"); + return 1; + } + } else { + error(&result, "badarg"); + return 1; + } + // {:add_default_database, _} + } else if (strlen(atom) == 20 && + strncmp(atom, "add_default_database", 20) == 0) { + if (magic_load_all(NULL) == 0) { + EI_ENSURE(ei_x_encode_atom(&result, "ok")); + EI_ENSURE(ei_x_encode_atom(&result, "loaded")); + } else { + exit(ERROR_DB); + } + // {:reload, _} + } else if (strlen(atom) == 6 && strncmp(atom, "reload", 6) == 0) { + magic_open_all(); + return 0; + // {:stop, _} } else if (strlen(atom) == 4 && strncmp(atom, "stop", 4) == 0) { exit(ERROR_OK); + // badarg } else { error(&result, "badarg"); return 1; @@ -206,88 +230,40 @@ int process_command(uint16_t len, byte *buf) { void setup_environment() { opterr = 0; } -void setup_options(int argc, char **argv) { - const char *option_string = "f:"; - static struct option long_options[] = { - {"database-file", required_argument, 0, 'f'}, - {"database-default", no_argument, 0, 'd'}, - {0, 0, 0, 0}}; - - int option_character; - while (1) { - int option_index = 0; - option_character = - getopt_long(argc, argv, option_string, long_options, &option_index); - if (-1 == option_character) { - break; - } - switch (option_character) { - case 'f': { - setup_options_file(optarg); - break; - } - case 'd': { - setup_options_default(); - break; - } - case '?': - default: { - exit(ERROR_NO_ARGUMENT); - break; - } - } +void magic_open_all() { + if (magic_mime_encoding) { + magic_close(magic_mime_encoding); } + if (magic_mime_type) { + magic_close(magic_mime_type); + } + if (magic_type_name) { + magic_close(magic_type_name); + } + magic_mime_encoding = magic_open(MAGIC_FLAGS_COMMON | MAGIC_MIME_ENCODING); + magic_mime_type = magic_open(MAGIC_FLAGS_COMMON | MAGIC_MIME_TYPE); + magic_type_name = magic_open(MAGIC_FLAGS_COMMON | MAGIC_NONE); + + ei_x_buff ok_buf; + EI_ENSURE(ei_x_new_with_version(&ok_buf)); + EI_ENSURE(ei_x_encode_atom(&ok_buf, "ready")); + write_cmd(ok_buf.buff, ok_buf.index); + EI_ENSURE(ei_x_free(&ok_buf)); } -void setup_options_file(char *optarg) { - if (0 != access(optarg, R_OK)) { - exit(ERROR_MISSING_DATABASE); - } +int magic_load_all(char *path) { + int res; - struct magic_file *next = malloc(sizeof(struct magic_file)); - size_t path_length = strlen(optarg) + 1; - char *path = malloc(path_length); - memcpy(path, optarg, path_length); - next->path = path; - next->prev = magic_database; - if (magic_database) { - magic_database->next = next; + if ((res = magic_load(magic_mime_encoding, path)) != 0) { + return res; } - magic_database = next; -} - -void setup_options_default() { - struct magic_file *next = malloc(sizeof(struct magic_file)); - next->path = NULL; - next->prev = magic_database; - if (magic_database) { - magic_database->next = next; + if ((res = magic_load(magic_mime_type, path)) != 0) { + return res; } - magic_database = next; -} - -void setup_system() { - magic_mime_encoding = magic_setup(MAGIC_FLAGS_COMMON | MAGIC_MIME_ENCODING); - magic_mime_type = magic_setup(MAGIC_FLAGS_COMMON | MAGIC_MIME_TYPE); - magic_type_name = magic_setup(MAGIC_FLAGS_COMMON | MAGIC_NONE); -} - -magic_t magic_setup(int flags) { - - magic_t magic = magic_open(flags); - struct magic_file *current_database = magic_database; - if (!current_database) { - exit(ERROR_NO_DATABASE); + if ((res = magic_load(magic_type_name, path)) != 0) { + return res; } - - while (current_database->prev) { - current_database = current_database->prev; - } - while (current_database) { - magic_load(magic, current_database->path); - current_database = current_database->next; - } - return magic; + return 0; } void process_bytes(char *path, int size, ei_x_buff *result) { diff --git a/test/gen_magic/apprentice_test.exs b/test/gen_magic/apprentice_test.exs index f7f0bb1..74c2081 100644 --- a/test/gen_magic/apprentice_test.exs +++ b/test/gen_magic/apprentice_test.exs @@ -7,49 +7,51 @@ defmodule GenMagic.ApprenticeTest do test "sends ready" do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) on_exit(fn() -> send(port, {self(), :close}) end) - assert_ready(port) + assert_ready_and_init_default(port) end test "stops" do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) on_exit(fn() -> send(port, {self(), :close}) end) - assert_ready(port) + assert_ready_and_init_default(port) send(port, {self(), {:command, :erlang.term_to_binary({:stop, :stop})}}) assert_receive {^port, {:exit_status, 0}} end - test "exits with no database" do + test "exits with non existent database with an error" do opts = [:use_stdio, :binary, :exit_status, {:packet, 2}, {:args, []}] port = Port.open(GenMagic.Config.get_port_name(), opts) on_exit(fn() -> send(port, {self(), :close}) end) + assert_ready(port) + send(port, {self(), {:command, :erlang.term_to_binary({:add_database, "/somewhere/nowhere"})}}) assert_receive {^port, {:exit_status, 1}} end - test "exits with a non existent database" do - opts = [ - {:args, ["--database-file", "/no/such/database"]}, - :use_stdio, - :binary, - :exit_status, - {:packet, 2} - ] - - port = Port.open(GenMagic.Config.get_port_name(), opts) - on_exit(fn() -> send(port, {self(), :close}) end) - assert_receive {^port, {:exit_status, 3}} - end + #test "exits with a non existent database" do + # opts = [ + # {:args, ["--database-file", "/no/such/database"]}, + # :use_stdio, + # :binary, + # :exit_status, + # {:packet, 2} + # ] + # + # port = Port.open(GenMagic.Config.get_port_name(), opts) + # on_exit(fn() -> send(port, {self(), :close}) end) + # assert_receive {^port, {:exit_status, 3}} + #end describe "port" do setup do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) on_exit(fn() -> send(port, {self(), :close}) end) - assert_ready(port) + assert_ready_and_init_default(port) %{port: port} end test "exits with badly formatted erlang terms", %{port: port} do send(port, {self(), {:command, "i forgot to term_to_binary!!"}}) - assert_receive {^port, {:exit_status, 5}} + assert_receive {^port, {:exit_status, 3}} end test "errors with wrong command", %{port: port} do @@ -140,6 +142,14 @@ defmodule GenMagic.ApprenticeTest do assert :ready == :erlang.binary_to_term(data) end + def assert_ready_and_init_default(port) do + assert_receive {^port, {:data, data}} + assert :ready == :erlang.binary_to_term(data) + send(port, {self(), {:command, :erlang.term_to_binary({:add_default_database, nil})}}) + assert_receive {^port, {:data, data}} + assert {:ok, _} = :erlang.binary_to_term(data) + end + def too_big(path, filename, limit \\ 4095) do last_len = byte_size(filename) path_len = byte_size(path) diff --git a/test/gen_magic/gen_magic_test.exs b/test/gen_magic/gen_magic_test.exs index 9699177..209b832 100644 --- a/test/gen_magic/gen_magic_test.exs +++ b/test/gen_magic/gen_magic_test.exs @@ -24,7 +24,6 @@ defmodule GenMagicTest do end test "Non-existent file" do - Process.flag(:trap_exit, true) {:ok, pid} = GenMagic.Server.start_link([]) path = missing_filename() assert_no_file(GenMagic.Server.perform(pid, path)) @@ -41,15 +40,33 @@ defmodule GenMagicTest do assert "text/x-makefile" = result.mime_type end - test "Custom database file recognises Elixir files" do - database = absolute_path("elixir.mgc") - on_exit(fn() -> File.rm(database) end) - {_, 0} = System.cmd("file", ["-C", "-m", absolute_path("test/elixir")]) - {:ok, pid} = GenMagic.Server.start_link(database_patterns: [database]) - path = absolute_path("mix.exs") - assert {:ok, %Result{} = result} = GenMagic.Server.perform(pid, path) - assert "text/x-elixir" = result.mime_type - assert "us-ascii" = result.encoding - assert "Elixir module source text" = result.content + describe "custom database" do + + setup do + database = absolute_path("elixir.mgc") + on_exit(fn() -> File.rm(database) end) + {_, 0} = System.cmd("file", ["-C", "-m", absolute_path("test/elixir")]) + [database: database] + end + + test "recognises Elixir files", %{database: database} do + {:ok, pid} = GenMagic.Server.start_link(database_patterns: [database]) + path = absolute_path("mix.exs") + assert {:ok, %Result{} = result} = GenMagic.Server.perform(pid, path) + assert "text/x-elixir" = result.mime_type + assert "us-ascii" = result.encoding + assert "Elixir module source text" = result.content + end + + test "recognises Elixir files after a reload", %{database: database} do + {:ok, pid} = GenMagic.Server.start_link([]) + path = absolute_path("mix.exs") + {:ok, %Result{mime_type: mime}} = GenMagic.Server.perform(pid, path) + refute mime == "text/x-elixir" + :ok = GenMagic.Server.reload(pid, [database]) + assert {:ok, %Result{mime_type: "text/x-elixir"}} = GenMagic.Server.perform(pid, path) + end + end + end diff --git a/test/gen_magic/server_test.exs b/test/gen_magic/server_test.exs index 53e5830..d89ea5f 100644 --- a/test/gen_magic/server_test.exs +++ b/test/gen_magic/server_test.exs @@ -31,4 +31,15 @@ defmodule GenMagic.ServerTest do assert {:ok, %{cycles: 0}} = GenMagic.Server.status(pid) end end + + test "recycle" do + {:ok, pid} = GenMagic.Server.start_link([]) + path = absolute_path("Makefile") + assert {:ok, %{cycles: 0}} = GenMagic.Server.status(pid) + assert {:ok, _} = GenMagic.Server.perform(pid, path) + assert {:ok, %{cycles: 1}} = GenMagic.Server.status(pid) + assert :ok = GenMagic.Server.recycle(pid) + assert {:ok, %{cycles: 0}} = GenMagic.Server.status(pid) + end + end From 6186331fe8e7b53476977fa52031eb490a51a2f8 Mon Sep 17 00:00:00 2001 From: Jordan Bracco Date: Mon, 15 Jun 2020 18:03:45 +0200 Subject: [PATCH 6/7] Format & delete .builds/ --- .builds/alpine.yaml | 17 ----------- .builds/archlinux.yaml | 17 ----------- .builds/debian-oldstable.yaml | 20 ------------- .builds/debian-stable.yaml | 20 ------------- .builds/debian-testing.yaml | 20 ------------- .builds/fedora-latest.yaml | 22 -------------- .builds/freebsd.yaml | 17 ----------- lib/gen_magic/server.ex | 47 ++++++++++++++++++++---------- test/gen_magic/apprentice_test.exs | 26 ++++++++++++----- test/gen_magic/gen_magic_test.exs | 5 +--- test/gen_magic/server_test.exs | 1 - 11 files changed, 50 insertions(+), 162 deletions(-) delete mode 100644 .builds/alpine.yaml delete mode 100644 .builds/archlinux.yaml delete mode 100644 .builds/debian-oldstable.yaml delete mode 100644 .builds/debian-stable.yaml delete mode 100644 .builds/debian-testing.yaml delete mode 100644 .builds/fedora-latest.yaml delete mode 100644 .builds/freebsd.yaml diff --git a/.builds/alpine.yaml b/.builds/alpine.yaml deleted file mode 100644 index 579497a..0000000 --- a/.builds/alpine.yaml +++ /dev/null @@ -1,17 +0,0 @@ -image: alpine/latest -packages: - - elixir - - file-dev -sources: - - https://git.sr.ht/~href/gen_magic -tasks: - - setup: | - mix local.hex --force - - build: | - cd gen_magic - mix deps.get - MIX_ENV=test mix compile - - test: | - cd gen_magic - mix test - diff --git a/.builds/archlinux.yaml b/.builds/archlinux.yaml deleted file mode 100644 index 4dc0c26..0000000 --- a/.builds/archlinux.yaml +++ /dev/null @@ -1,17 +0,0 @@ -image: archlinux -packages: - - elixir - - file -sources: - - https://git.sr.ht/~href/gen_magic -tasks: - - setup: | - mix local.hex --force - - build: | - cd gen_magic - mix deps.get - MIX_ENV=test mix compile - - test: | - cd gen_magic - mix test - diff --git a/.builds/debian-oldstable.yaml b/.builds/debian-oldstable.yaml deleted file mode 100644 index 915f1a2..0000000 --- a/.builds/debian-oldstable.yaml +++ /dev/null @@ -1,20 +0,0 @@ -image: debian/oldstable -packages: - - build-essential - - erlang - - erlang-dev - - elixir - - libmagic-dev -sources: - - https://git.sr.ht/~href/gen_magic -tasks: - - setup: | - mix local.hex --force - - build: | - cd gen_magic - mix deps.get - MIX_ENV=test mix compile - - test: | - cd gen_magic - mix test - diff --git a/.builds/debian-stable.yaml b/.builds/debian-stable.yaml deleted file mode 100644 index d6bdbe2..0000000 --- a/.builds/debian-stable.yaml +++ /dev/null @@ -1,20 +0,0 @@ -image: debian/stable -packages: - - build-essential - - erlang - - erlang-dev - - elixir - - libmagic-dev -sources: - - https://git.sr.ht/~href/gen_magic -tasks: - - setup: | - mix local.hex --force - - build: | - cd gen_magic - mix deps.get - MIX_ENV=test mix compile - - test: | - cd gen_magic - mix test - diff --git a/.builds/debian-testing.yaml b/.builds/debian-testing.yaml deleted file mode 100644 index bda46e4..0000000 --- a/.builds/debian-testing.yaml +++ /dev/null @@ -1,20 +0,0 @@ -image: debian/testing -packages: - - build-essential - - erlang - - erlang-dev - - elixir - - libmagic-dev -sources: - - https://git.sr.ht/~hrefhref/gen_magic -tasks: - - setup: | - mix local.hex --force - - build: | - cd gen_magic - mix deps.get - MIX_ENV=test mix compile - - test: | - cd gen_magic - mix test - diff --git a/.builds/fedora-latest.yaml b/.builds/fedora-latest.yaml deleted file mode 100644 index ba87865..0000000 --- a/.builds/fedora-latest.yaml +++ /dev/null @@ -1,22 +0,0 @@ -image: fedora/latest -packages: - - make - - gcc - - kernel-devel - - erlang - - elixir - - file-devel -sources: - - https://git.sr.ht/~href/gen_magic -tasks: - - setup: | - sudo dnf -y group install 'Development Tools' - mix local.hex --force - - build: | - cd gen_magic - mix deps.get - MIX_ENV=test mix compile - - test: | - cd gen_magic - mix test - diff --git a/.builds/freebsd.yaml b/.builds/freebsd.yaml deleted file mode 100644 index 3a78947..0000000 --- a/.builds/freebsd.yaml +++ /dev/null @@ -1,17 +0,0 @@ -image: freebsd/latest -packages: - - elixir - - gmake -sources: - - https://git.sr.ht/~href/gen_magic -tasks: - - setup: | - mix local.hex --force - - build: | - cd gen_magic - mix deps.get - MIX_ENV=test mix compile - - test: | - cd gen_magic - mix test - diff --git a/lib/gen_magic/server.ex b/lib/gen_magic/server.ex index 545b024..6b7f245 100644 --- a/lib/gen_magic/server.ex +++ b/lib/gen_magic/server.ex @@ -84,7 +84,7 @@ defmodule GenMagic.Server do @spec child_spec([option()]) :: Supervisor.child_spec() @spec start_link([option()]) :: :gen_statem.start_ret() @spec perform(t(), Path.t() | {:bytes, binary()}, timeout()) :: - {:ok, Result.t()} | {:error, term() | String.t()} + {:ok, Result.t()} | {:error, term() | String.t()} @spec status(t(), timeout()) :: {:ok, Status.t()} | {:error, term()} @spec stop(t(), term(), timeout()) :: :ok @@ -227,15 +227,19 @@ defmodule GenMagic.Server do end def loading(:enter, _old_state, data) do - databases = Enum.flat_map(List.wrap(data.database_patterns || @database_patterns), fn - :default -> [:default] - pattern -> Path.wildcard(pattern) - end) - databases = if databases == [] do - [:default] - else - databases - end + databases = + Enum.flat_map(List.wrap(data.database_patterns || @database_patterns), fn + :default -> [:default] + pattern -> Path.wildcard(pattern) + end) + + databases = + if databases == [] do + [:default] + else + databases + end + {:keep_state, {databases, data}, {:state_timeout, 0, :load}} end @@ -248,10 +252,12 @@ defmodule GenMagic.Server do end def loading(:state_timeout, :load, {[database | databases], data} = state) do - command = case database do - :default -> {:add_default_database, nil} - path -> {:add_database, database} - end + command = + case database do + :default -> {:add_default_database, nil} + path -> {:add_database, database} + end + send(data.port, command) {:keep_state, state, {:state_timeout, data.startup_timeout, :load_timeout}} end @@ -303,13 +309,22 @@ defmodule GenMagic.Server do def available({:call, from}, {:reload, databases}, data) do send(data.port, {:reload, :reload}) + {:next_state, :starting, - %{data | database_patterns: databases || data.database_patterns, request: {:reload, from, :reload}}} + %{ + data + | database_patterns: databases || data.database_patterns, + request: {:reload, from, :reload} + }} end def available({:call, from}, {:recycle, databases}, data) do {:next_state, :recycling, - %{data | database_patterns: databases || data.database_patterns, request: {:reload, from, :recycle}}} + %{ + data + | database_patterns: databases || data.database_patterns, + request: {:reload, from, :recycle} + }} end @doc false diff --git a/test/gen_magic/apprentice_test.exs b/test/gen_magic/apprentice_test.exs index 74c2081..af325ec 100644 --- a/test/gen_magic/apprentice_test.exs +++ b/test/gen_magic/apprentice_test.exs @@ -6,13 +6,13 @@ defmodule GenMagic.ApprenticeTest do test "sends ready" do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) - on_exit(fn() -> send(port, {self(), :close}) end) + on_exit(fn -> send(port, {self(), :close}) end) assert_ready_and_init_default(port) end test "stops" do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) - on_exit(fn() -> send(port, {self(), :close}) end) + on_exit(fn -> send(port, {self(), :close}) end) assert_ready_and_init_default(port) send(port, {self(), {:command, :erlang.term_to_binary({:stop, :stop})}}) assert_receive {^port, {:exit_status, 0}} @@ -21,13 +21,18 @@ defmodule GenMagic.ApprenticeTest do test "exits with non existent database with an error" do opts = [:use_stdio, :binary, :exit_status, {:packet, 2}, {:args, []}] port = Port.open(GenMagic.Config.get_port_name(), opts) - on_exit(fn() -> send(port, {self(), :close}) end) + on_exit(fn -> send(port, {self(), :close}) end) assert_ready(port) - send(port, {self(), {:command, :erlang.term_to_binary({:add_database, "/somewhere/nowhere"})}}) + + send( + port, + {self(), {:command, :erlang.term_to_binary({:add_database, "/somewhere/nowhere"})}} + ) + assert_receive {^port, {:exit_status, 1}} end - #test "exits with a non existent database" do + # test "exits with a non existent database" do # opts = [ # {:args, ["--database-file", "/no/such/database"]}, # :use_stdio, @@ -39,12 +44,12 @@ defmodule GenMagic.ApprenticeTest do # port = Port.open(GenMagic.Config.get_port_name(), opts) # on_exit(fn() -> send(port, {self(), :close}) end) # assert_receive {^port, {:exit_status, 3}} - #end + # end describe "port" do setup do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([])) - on_exit(fn() -> send(port, {self(), :close}) end) + on_exit(fn -> send(port, {self(), :close}) end) assert_ready_and_init_default(port) %{port: port} end @@ -97,6 +102,7 @@ defmodule GenMagic.ApprenticeTest do test "works with big file path", %{port: port} do # Test with longest valid path. {dir, bigfile} = too_big(@tmp_path, "/a") + case File.mkdir_p(dir) do :ok -> File.touch!(bigfile) @@ -130,8 +136,12 @@ defmodule GenMagic.ApprenticeTest do assert_receive {^port, {:data, data}} assert {:ok, _} = :erlang.binary_to_term(data) refute_receive _ + {:error, :enametoolong} -> - Logger.info("Skipping test, operating system does not support max POSIX length for directories") + Logger.info( + "Skipping test, operating system does not support max POSIX length for directories" + ) + :ignore end end diff --git a/test/gen_magic/gen_magic_test.exs b/test/gen_magic/gen_magic_test.exs index 209b832..92574f4 100644 --- a/test/gen_magic/gen_magic_test.exs +++ b/test/gen_magic/gen_magic_test.exs @@ -41,10 +41,9 @@ defmodule GenMagicTest do end describe "custom database" do - setup do database = absolute_path("elixir.mgc") - on_exit(fn() -> File.rm(database) end) + on_exit(fn -> File.rm(database) end) {_, 0} = System.cmd("file", ["-C", "-m", absolute_path("test/elixir")]) [database: database] end @@ -66,7 +65,5 @@ defmodule GenMagicTest do :ok = GenMagic.Server.reload(pid, [database]) assert {:ok, %Result{mime_type: "text/x-elixir"}} = GenMagic.Server.perform(pid, path) end - end - end diff --git a/test/gen_magic/server_test.exs b/test/gen_magic/server_test.exs index d89ea5f..d00933a 100644 --- a/test/gen_magic/server_test.exs +++ b/test/gen_magic/server_test.exs @@ -41,5 +41,4 @@ defmodule GenMagic.ServerTest do assert :ok = GenMagic.Server.recycle(pid) assert {:ok, %{cycles: 0}} = GenMagic.Server.status(pid) end - end From 8c58ecd0572c034206711a118f71d64977900602 Mon Sep 17 00:00:00 2001 From: Jordan Bracco Date: Mon, 15 Jun 2020 18:16:42 +0200 Subject: [PATCH 7/7] Cleanup --- lib/gen_magic/server.ex | 14 ++++++++++++++ test/gen_magic/apprentice_test.exs | 14 -------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/gen_magic/server.ex b/lib/gen_magic/server.ex index 6b7f245..c15ea18 100644 --- a/lib/gen_magic/server.ex +++ b/lib/gen_magic/server.ex @@ -214,6 +214,7 @@ defmodule GenMagic.Server do end end + @doc false def starting(:info, {port, {:exit_status, code}}, %{port: port} = data) do error = case code do @@ -226,6 +227,7 @@ defmodule GenMagic.Server do {:stop, {:error, error}, data} end + @doc false def loading(:enter, _old_state, data) do databases = Enum.flat_map(List.wrap(data.database_patterns || @database_patterns), fn @@ -243,14 +245,17 @@ defmodule GenMagic.Server do {:keep_state, {databases, data}, {:state_timeout, 0, :load}} end + @doc false def loading(:state_timeout, :load_timeout, {[database | _], data}) do {:stop, {:error, {:database_loading_timeout, database}}, data} end + @doc false def loading(:state_timeout, :load, {[], data}) do {:next_state, :available, data} end + @doc false def loading(:state_timeout, :load, {[database | databases], data} = state) do command = case database do @@ -262,6 +267,7 @@ defmodule GenMagic.Server do {:keep_state, state, {:state_timeout, data.startup_timeout, :load_timeout}} end + @doc false def loading(:info, {port, {:data, response}}, {[database | databases], %{port: port} = data}) do case :erlang.binary_to_term(response) do {:ok, :loaded} -> @@ -269,6 +275,7 @@ defmodule GenMagic.Server do end end + @doc false def loading(:info, {port, {:exit_status, 1}}, {[database | _], %{port: port} = data}) do {:stop, {:error, {:database_not_found, database}}, data} end @@ -289,6 +296,7 @@ defmodule GenMagic.Server do {:keep_state_and_data, response} end + @doc false def available(:enter, _old_state, %{request: nil}) do :keep_state_and_data end @@ -307,6 +315,7 @@ defmodule GenMagic.Server do {:next_state, :processing, data} end + @doc false def available({:call, from}, {:reload, databases}, data) do send(data.port, {:reload, :reload}) @@ -318,6 +327,7 @@ defmodule GenMagic.Server do }} end + @doc false def available({:call, from}, {:recycle, databases}, data) do {:next_state, :recycling, %{ @@ -376,16 +386,19 @@ defmodule GenMagic.Server do handle_status_call(from, :recycling, data) end + @doc false # In case of timeout, force close. def recycling(:state_timeout, :stop, data) do Kernel.send(data.port, {self(), :close}) {:keep_state_and_data, {:state_timeout, data.startup_timeout, :close}} end + @doc false def recycling(:state_timeout, :close, data) do {:stop, {:error, :port_close_failed}} end + @doc false def recycling(:info, {port, :closed}, %{port: port} = data) do {:next_state, :starting, %{data | port: nil, cycles: 0}} end @@ -401,6 +414,7 @@ defmodule GenMagic.Server do Kernel.send(port, {self(), :close}) end + @doc false def terminate(_, _, _) do :ok end diff --git a/test/gen_magic/apprentice_test.exs b/test/gen_magic/apprentice_test.exs index af325ec..dfb86a0 100644 --- a/test/gen_magic/apprentice_test.exs +++ b/test/gen_magic/apprentice_test.exs @@ -32,20 +32,6 @@ defmodule GenMagic.ApprenticeTest do assert_receive {^port, {:exit_status, 1}} end - # test "exits with a non existent database" do - # opts = [ - # {:args, ["--database-file", "/no/such/database"]}, - # :use_stdio, - # :binary, - # :exit_status, - # {:packet, 2} - # ] - # - # port = Port.open(GenMagic.Config.get_port_name(), opts) - # on_exit(fn() -> send(port, {self(), :close}) end) - # assert_receive {^port, {:exit_status, 3}} - # end - describe "port" do setup do port = Port.open(GenMagic.Config.get_port_name(), GenMagic.Config.get_port_options([]))