From bccb1600ebdbe7d4bf369a7e59b77599564aa696 Mon Sep 17 00:00:00 2001 From: treejamie Date: Thu, 30 Apr 2026 09:55:21 +0100 Subject: [PATCH 1/4] feat: credo will need configuring so add a config file --- .credo.exs | 14 ++++++++++++++ lib/router.ex | 8 +++----- 2 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 .credo.exs diff --git a/.credo.exs b/.credo.exs new file mode 100644 index 0000000..0c5786b --- /dev/null +++ b/.credo.exs @@ -0,0 +1,14 @@ +# https://hexdocs.pm/credo/config_file.html + configs: [ + %{ + name: "default", + checks: %{ + disabled: [ + # this means that `TabsOrSpaces` will not run + {Credo.Check.Consistency.TabsOrSpaces, []}, + ] + } + + } + ] +} diff --git a/lib/router.ex b/lib/router.ex index 12928d1..7e09ca6 100644 --- a/lib/router.ex +++ b/lib/router.ex @@ -8,11 +8,9 @@ defmodule Server.Router do 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 From fe9568e1bbf17346f76984e0282667be287a1ea8 Mon Sep 17 00:00:00 2001 From: treejamie Date: Thu, 30 Apr 2026 10:13:17 +0100 Subject: [PATCH 2/4] feat: codebase now clear of credo warnings --- .credo.exs | 4 ++-- lib/files.ex | 23 ++++++++++++++++++++--- lib/handler.ex | 2 +- lib/main.ex | 39 +++++++++++++++++++++++---------------- lib/parser.ex | 3 +++ lib/response.ex | 3 +++ lib/router.ex | 31 ++++++++++++++++++++----------- mix.exs | 6 ++++++ 8 files changed, 78 insertions(+), 33 deletions(-) diff --git a/.credo.exs b/.credo.exs index 0c5786b..d71e2ac 100644 --- a/.credo.exs +++ b/.credo.exs @@ -1,14 +1,14 @@ # https://hexdocs.pm/credo/config_file.html +%{ configs: [ %{ name: "default", checks: %{ disabled: [ # this means that `TabsOrSpaces` will not run - {Credo.Check.Consistency.TabsOrSpaces, []}, + {Credo.Check.Consistency.TabsOrSpaces, []} ] } - } ] } diff --git a/lib/files.ex b/lib/files.ex index 53fdea0..36e750e 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 Path.join([ Application.get_env(:codecrafters_http_server, :directory), filename diff --git a/lib/handler.ex b/lib/handler.ex index cb47c62..67175e4 100644 --- a/lib/handler.ex +++ b/lib/handler.ex @@ -1,5 +1,5 @@ defmodule Server.Handler do - @doc """ + @moduledoc """ A very simple handler that takes a request string and returns a response """ def handle(request) do diff --git a/lib/main.ex b/lib/main.ex index bc366e2..03e53c8 100644 --- a/lib/main.ex +++ b/lib/main.ex @@ -1,4 +1,7 @@ defmodule Server do + @moduledoc """ + The entrypoint for the main server loop. + """ use Application require Logger @@ -7,7 +10,7 @@ defmodule Server do Supervisor.start_link([{Task, fn -> Server.listen() end}], strategy: :one_for_one) end - def listen() do + def listen do {:ok, socket} = :gen_tcp.listen(4221, [:binary, active: false, reuseaddr: true]) loop(socket) end @@ -22,25 +25,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 diff --git a/lib/parser.ex b/lib/parser.ex index 42cf3e9..3046d13 100644 --- a/lib/parser.ex +++ b/lib/parser.ex @@ -1,4 +1,7 @@ defmodule Server.Parser do + @moduledoc """ + Transforms a plain text HTTP request into a Response. + """ alias Server.Response @supported_encodings [ diff --git a/lib/response.ex b/lib/response.ex index fac6071..4df2e37 100644 --- a/lib/response.ex +++ b/lib/response.ex @@ -1,4 +1,7 @@ defmodule Server.Response do + @moduledoc """ + The Response struct is the central idea in the http server + """ defstruct method: nil, path: nil, params: %{}, diff --git a/lib/router.ex b/lib/router.ex index 7e09ca6..02ee41a 100644 --- a/lib/router.ex +++ b/lib/router.ex @@ -1,7 +1,15 @@ 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 def route(%Response{method: "GET", path: "/"} = response) do %{response | status: 200} @@ -16,16 +24,17 @@ defmodule Server.Router do 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"] From 7c5c0919d3a0265c4e3cbe7223ece2a55225e265 Mon Sep 17 00:00:00 2001 From: treejamie Date: Thu, 30 Apr 2026 10:57:45 +0100 Subject: [PATCH 3/4] chore: all public functions spec'd and all structs typed --- .credo.exs | 3 +++ lib/files.ex | 2 +- lib/handler.ex | 2 ++ lib/main.ex | 9 ++++++--- lib/parser.ex | 1 + lib/response.ex | 18 ++++++++++++++++++ lib/router.ex | 1 + 7 files changed, 32 insertions(+), 4 deletions(-) diff --git a/.credo.exs b/.credo.exs index d71e2ac..fd1d573 100644 --- a/.credo.exs +++ b/.credo.exs @@ -4,6 +4,9 @@ %{ 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 36e750e..fd535f7 100644 --- a/lib/files.ex +++ b/lib/files.ex @@ -29,7 +29,7 @@ defmodule Server.Files do end end - def build_path(filename) do + 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 67175e4..dec519b 100644 --- a/lib/handler.ex +++ b/lib/handler.ex @@ -2,6 +2,8 @@ defmodule Server.Handler do @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 03e53c8..6479f84 100644 --- a/lib/main.ex +++ b/lib/main.ex @@ -5,11 +5,13 @@ defmodule Server do 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 + @spec listen :: ListenSocket def listen do {:ok, socket} = :gen_tcp.listen(4221, [:binary, active: false, reuseaddr: true]) loop(socket) @@ -57,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 3046d13..6f757d1 100644 --- a/lib/parser.ex +++ b/lib/parser.ex @@ -8,6 +8,7 @@ defmodule Server.Parser do "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 4df2e37..af1be99 100644 --- a/lib/response.ex +++ b/lib/response.ex @@ -2,6 +2,18 @@ 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: %{}, @@ -13,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 @@ -27,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" @@ -37,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 -> "" @@ -58,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 || "" @@ -68,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 02ee41a..974dcb5 100644 --- a/lib/router.ex +++ b/lib/router.ex @@ -11,6 +11,7 @@ defmodule Server.Router do alias Server.Files alias Server.Response + @spec route(Response.t()) :: Response.t() def route(%Response{method: "GET", path: "/"} = response) do %{response | status: 200} end From 415b94a925e7e54a74e2dd67fb0bb7176b005907 Mon Sep 17 00:00:00 2001 From: treejamie Date: Thu, 30 Apr 2026 11:20:16 +0100 Subject: [PATCH 4/4] fix: only the first two bytes of gzip are stable (31 & 139) so pattern match on them --- test/server_integration_test.exs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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