diff --git a/.credo.exs b/.credo.exs new file mode 100644 index 0000000..fd1d573 --- /dev/null +++ b/.credo.exs @@ -0,0 +1,17 @@ +# https://hexdocs.pm/credo/config_file.html +%{ + configs: [ + %{ + name: "default", + checks: %{ + enabled: [ + {Credo.Check.Readability.Specs, []} + ], + disabled: [ + # this means that `TabsOrSpaces` will not run + {Credo.Check.Consistency.TabsOrSpaces, []} + ] + } + } + ] +} diff --git a/lib/files.ex b/lib/files.ex index 53fdea0..fd535f7 100644 --- a/lib/files.ex +++ b/lib/files.ex @@ -1,18 +1,35 @@ defmodule Server.Files do + @moduledoc """ + For processing and handling files + """ + @spec write_file(String.t(), binary()) :: :ok | {:error, File.posix()} def write_file(filename, content) do build_path(filename) - |> File.write(content) + |> Path.safe_relative() + |> case do + {:ok, path} -> + File.write(path, content) + + _ -> + {:error, :unsafe_or_invalid_path} + end end @spec read_file(String.t()) :: {:ok, binary} | {:error, File.posix()} def read_file(filename) do build_path(filename) - |> File.read() + |> Path.safe_relative() + |> case do + {:ok, path} -> + File.read(path) + + _ -> + {:error, :unsafe_or_invalid_path} + end end - def build_path(filename) do - # TODO: harden against traversal attacks + defp build_path(filename) do Path.join([ Application.get_env(:codecrafters_http_server, :directory), filename diff --git a/lib/handler.ex b/lib/handler.ex index cb47c62..dec519b 100644 --- a/lib/handler.ex +++ b/lib/handler.ex @@ -1,7 +1,9 @@ defmodule Server.Handler do - @doc """ + @moduledoc """ A very simple handler that takes a request string and returns a response """ + + @spec handle(String.t()) :: Server.Response.t() def handle(request) do request |> Server.Parser.parse() diff --git a/lib/main.ex b/lib/main.ex index bc366e2..6479f84 100644 --- a/lib/main.ex +++ b/lib/main.ex @@ -1,13 +1,18 @@ defmodule Server do + @moduledoc """ + The entrypoint for the main server loop. + """ use Application require Logger + @spec start(Application.start_type(), map()) :: {:error, term()} | {:ok, pid()} def start(_type, _args) do Logger.info("Starting server on http://127.0.0.1:4221") Supervisor.start_link([{Task, fn -> Server.listen() end}], strategy: :one_for_one) end - def listen() do + @spec listen :: ListenSocket + def listen do {:ok, socket} = :gen_tcp.listen(4221, [:binary, active: false, reuseaddr: true]) loop(socket) end @@ -22,25 +27,29 @@ defmodule Server do defp handle_client(client_socket) do # this is the boundary - naive assumption checks of a perfect world. - with {:ok, request} <- :gen_tcp.recv(client_socket, 0) do - # logging - becasue reasons - Logger.debug("#{inspect(request)}") + case :gen_tcp.recv(client_socket, 0) do + {:ok, request} -> + # logging - becasue reasons + Logger.debug("#{inspect(request)}") - # make the respone - response = Server.Handler.handle(request) + # make the respone + response = Server.Handler.handle(request) - # return the reply - reply(response, client_socket) + # return the reply + reply(response, client_socket) - # now close or recurse - if response.close? do + # now close or recurse + if response.close? do + :gen_tcp.close(client_socket) + else + handle_client(client_socket) + end + + {:error, :closed} -> :gen_tcp.close(client_socket) - else - handle_client(client_socket) - end - else - {:error, :closed} -> :gen_tcp.close(client_socket) - {:error, error} -> Logger.error("loop error: #{inspect(error)}") + + {:error, error} -> + Logger.error("loop error: #{inspect(error)}") end end @@ -50,9 +59,10 @@ defmodule Server do |> then(fn response -> :gen_tcp.send(client_socket, response) end) end - def main(args) do - # parse the args - {opts, _args, _invalid} = OptionParser.parse(args, strict: [directory: :string]) + @spec main(Keyword.t()) :: no_return() + def main(opts) do + # parse the opts + {opts, _args, _invalid} = OptionParser.parse(opts, strict: [directory: :string]) directory = Keyword.get(opts, :directory) # set the config diff --git a/lib/parser.ex b/lib/parser.ex index 42cf3e9..6f757d1 100644 --- a/lib/parser.ex +++ b/lib/parser.ex @@ -1,10 +1,14 @@ defmodule Server.Parser do + @moduledoc """ + Transforms a plain text HTTP request into a Response. + """ alias Server.Response @supported_encodings [ "gzip" ] + @spec parse(String.t()) :: Response.t() def parse(request) do # split out request body first as that's \r\n\r\n [request | [request_body]] = String.split(request, "\r\n\r\n") diff --git a/lib/response.ex b/lib/response.ex index fac6071..af1be99 100644 --- a/lib/response.ex +++ b/lib/response.ex @@ -1,4 +1,19 @@ defmodule Server.Response do + @moduledoc """ + The Response struct is the central idea in the http server + """ + @type t :: %__MODULE__{ + method: String.t() | nil, + path: String.t() | nil, + params: map(), + headers: map(), + content_length: non_neg_integer() | nil, + content_type: String.t(), + close?: boolean(), + body: iodata() | nil, + status: non_neg_integer() | nil, + request_body: binary() | nil + } defstruct method: nil, path: nil, params: %{}, @@ -10,10 +25,12 @@ defmodule Server.Response do status: nil, request_body: nil + @spec full_status(__MODULE__.t()) :: String.t() def full_status(%__MODULE__{} = response) do "#{response.status} #{status_reason(response.status)}" end + @spec gzip?(__MODULE__.t()) :: bool def gzip?(%__MODULE__{} = response) do case Map.get(response.headers, "Content-Encoding") do "gzip" -> true @@ -24,6 +41,7 @@ defmodule Server.Response do @doc """ If we need to close the connection. Send the header """ + @spec maybe_connection?(__MODULE__.t()) :: String.t() def maybe_connection?(response) do case response.close? do true -> "Connection: close\r\n" @@ -34,6 +52,7 @@ defmodule Server.Response do @doc """ If there's content encoding, show the header """ + @spec maybe_content_encoding?(map()) :: String.t() def maybe_content_encoding?(headers) do case Map.get(headers, "Content-Encoding") do nil -> "" @@ -55,6 +74,7 @@ defmodule Server.Response do end defimpl String.Chars, for: Server.Response do + @spec get_body(Server.Response.t()) :: String.t() | iodata() def get_body(response) do # body could be nil, so defend against taht body = response.body || "" @@ -65,6 +85,7 @@ defimpl String.Chars, for: Server.Response do end end + @spec to_string(Server.Response.t()) :: String.t() def to_string(%Server.Response{} = response) do # some parts needs to be calculated body = get_body(response) diff --git a/lib/router.ex b/lib/router.ex index 12928d1..974dcb5 100644 --- a/lib/router.ex +++ b/lib/router.ex @@ -1,33 +1,41 @@ defmodule Server.Router do + @moduledoc """ + Maps http responses through defined routes. + + There's a conflict in being the server and the application and having routes does + actually blur the line a little so consider this module as a very crude interface + to validate http behavior through integration tests. + + """ require Logger - alias Server.Response alias Server.Files + alias Server.Response + @spec route(Response.t()) :: Response.t() def route(%Response{method: "GET", path: "/"} = response) do %{response | status: 200} end def route(%Response{method: "POST", path: "/files/" <> filename} = response) do - with :ok <- Files.write_file(filename, response.request_body) do - %{response | status: 201} - else - {:error, _} -> - %{response | status: 500} + case Files.write_file(filename, response.request_body) do + :ok -> %{response | status: 201} + _ -> %{response | status: 500} end end def route(%Response{method: "GET", path: "/files/" <> filename} = response) do # now we are either 200 or 404 - with {:ok, content} <- Files.read_file(filename) do - %{ - response - | body: content, - status: 200, - content_length: byte_size(content), - content_type: "application/octet-stream" - } - else - {:error, _} -> + case Files.read_file(filename) do + {:ok, content} -> + %{ + response + | body: content, + status: 200, + content_length: byte_size(content), + content_type: "application/octet-stream" + } + + _ -> content = "Not Found: #{filename}" %{ diff --git a/mix.exs b/mix.exs index 33efcdc..29f2802 100644 --- a/mix.exs +++ b/mix.exs @@ -14,6 +14,12 @@ defmodule App.MixProject do ] end + def cli do + [ + preferred_envs: [check: :test] + ] + end + defp aliases do [ check: ["format --check-formatted", "credo --strict", "dialyzer", "test"] diff --git a/test/server_integration_test.exs b/test/server_integration_test.exs index 313963b..5104039 100644 --- a/test/server_integration_test.exs +++ b/test/server_integration_test.exs @@ -25,9 +25,13 @@ defmodule Server.IntegrationTest do response = request("GET /echo/foo HTTP/1.1\r\nHost: localhost:4221\r\nAccept-Encoding: gzip\r\n\r\n") - assert response =~ - <<31, 139, 8, 0, 0, 0, 0, 0, 0, 19, 75, 203, 207, 7, 0, 33, 101, 115, 140, 3, 0, 0, - 0>> + [_headers, body] = String.split(response, "\r\n\r\n", parts: 2) + + # pattern match to assert gzip magic bytes + assert <<31, 139, _rest::binary>> = body + + # decompress and assert actual content + assert :zlib.gunzip(body) == "foo" end test "when accept-encoding is present as a series of comma seperate values, we send back a validContent-Encoding" do