From 42b72ff477523fdca74e20aff80922b66d460e7e Mon Sep 17 00:00:00 2001 From: Cory O'Daniel Date: Wed, 1 Apr 2026 09:22:40 -0700 Subject: [PATCH 1/5] Standardize error handling to three-tuple map details User prompts: - "Expand registry error results to include details from storage adapters" - "make the registry do most of the error handling, check for repo/upload existence early" - "come up with a good error struct standard" - "bump the minor level and open a PR" Changes: - Standardize all errors to {:error, atom_code, map()} three-tuple - Add ensure_repo_exists/3 and ensure_upload_exists/4 precondition helpers to Registry - Registry now checks repo/upload existence before delegating to storage - Fix unhandled `false` fall-through in upload_blob_chunk and complete_blob_upload - Convert all string error details to structured maps across auth, storage, handler - Remove two-tuple error match from Handler (and the placeholder message) - Update Storage.Adapter and Auth.Adapter callback specs to enforce map details - Bump version to 0.2.0 Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/oci/auth/adapter.ex | 2 +- lib/oci/auth/static.ex | 17 +++++++----- lib/oci/plug.ex | 9 +++--- lib/oci/plug/handler.ex | 16 ++--------- lib/oci/registry.ex | 56 +++++++++++++++++++++++++------------- lib/oci/storage/adapter.ex | 29 ++++++++------------ lib/oci/storage/local.ex | 22 ++++++--------- mix.exs | 2 +- test/plug_test.exs | 6 ++-- 9 files changed, 79 insertions(+), 80 deletions(-) diff --git a/lib/oci/auth/adapter.ex b/lib/oci/auth/adapter.ex index 519d0e3..269a98c 100644 --- a/lib/oci/auth/adapter.ex +++ b/lib/oci/auth/adapter.ex @@ -38,7 +38,7 @@ defmodule OCI.Auth.Adapter do """ @type subject_t :: any() - @type error_details_t :: any() + @type error_details_t :: map() @callback init(config :: map()) :: {:ok, t()} | {:error, term()} diff --git a/lib/oci/auth/static.ex b/lib/oci/auth/static.ex index 88cad82..56128f2 100644 --- a/lib/oci/auth/static.ex +++ b/lib/oci/auth/static.ex @@ -45,21 +45,24 @@ defmodule OCI.Auth.Static do subject = username {:ok, subject} else - {:error, :UNAUTHORIZED, "Invalid username or password"} + {:error, :UNAUTHORIZED, %{reason: "Invalid username or password"}} end _ -> {:error, :UNAUTHORIZED, - "Invalid authorization format, should be username:password"} + %{reason: "Invalid authorization format, should be username:password"}} end :error -> {:error, :UNAUTHORIZED, - "Failed to decode authorization, should be base64 encoded username:password"} + %{ + reason: + "Failed to decode authorization, should be base64 encoded username:password" + }} end _other -> - {:error, :UNSUPPORTED, "Unsupported authentication scheme: #{scheme}"} + {:error, :UNSUPPORTED, %{reason: "Unsupported authentication scheme: #{scheme}"}} end end @@ -73,18 +76,18 @@ defmodule OCI.Auth.Static do case required_action(ctx.method, ctx.endpoint) do nil -> - {:error, :DENIED} + {:error, :DENIED, %{repo: ctx.repo, method: ctx.method}} action -> if action in repo_perms do :ok else - {:error, :DENIED} + {:error, :DENIED, %{repo: ctx.repo, action: action}} end end _ -> - {:error, :DENIED} + {:error, :DENIED, %{subject: ctx.subject}} end end diff --git a/lib/oci/plug.ex b/lib/oci/plug.ex index 1e0827f..444a711 100644 --- a/lib/oci/plug.ex +++ b/lib/oci/plug.ex @@ -34,7 +34,9 @@ defmodule OCI.Plug do end def call(conn, _opts) do - error_resp(conn, :UNSUPPORTED, "OCI Registry must be mounted at /#{Registry.api_version()}") + error_resp(conn, :UNSUPPORTED, %{ + reason: "OCI Registry must be mounted at /#{Registry.api_version()}" + }) end def authenticate(%{private: %{oci_registry: registry}} = conn) do @@ -64,16 +66,13 @@ defmodule OCI.Plug do :ok -> conn - {:error, reason} -> - error_resp(conn, reason, nil) - {:error, reason, details} -> error_resp(conn, reason, details) end end defp authorize(_) do - {:error, :UNAUTHORIZED} + {:error, :UNAUTHORIZED, %{}} end defp challenge_resp(conn) do diff --git a/lib/oci/plug/handler.ex b/lib/oci/plug/handler.ex index a33b8af..c8168c5 100644 --- a/lib/oci/plug/handler.ex +++ b/lib/oci/plug/handler.ex @@ -30,15 +30,6 @@ defmodule OCI.Plug.Handler do %Plug.Conn{} = conn -> conn - {:error, oci_error_status} -> - you_suck_and_are_a_bad_person_messsage = """ - You suck and are a bad person. - - Please return error details for #{oci_error_status} in #{inspect(ctx)} - """ - - error_resp(conn, oci_error_status, you_suck_and_are_a_bad_person_messsage) - {:error, oci_error_status, details} -> error_resp(conn, oci_error_status, details) end @@ -55,7 +46,7 @@ defmodule OCI.Plug.Handler do repo :: String.t(), id :: String.t(), ctx :: OCI.Context.t() - ) :: Plug.Conn.t() | {:error, atom()} | {:error, atom(), String.t()} + ) :: Plug.Conn.t() | {:error, atom(), map() | String.t()} defp dispatch(%{method: "GET"} = conn, :tags_list, registry, repo, _id, ctx) do pag = pagination(conn.query_params) @@ -179,7 +170,7 @@ defmodule OCI.Plug.Handler do error_resp( conn, :BLOB_UPLOAD_INVALID, - "Content-Range header is required for PATCH requests" + %{reason: "Content-Range header is required for PATCH requests"} ) _ -> @@ -325,7 +316,7 @@ defmodule OCI.Plug.Handler do method = conn.method path = conn.request_path - {:error, :UNSUPPORTED, "Unsupported [#{method}] #{path}"} + {:error, :UNSUPPORTED, %{method: method, path: path}} end defp pagination(params) do @@ -366,7 +357,6 @@ defmodule OCI.Plug.Handler do ctx ) do {:ok, _, _} -> :ok - {:error, reason} -> {:error, reason} {:error, reason, details} -> {:error, reason, details} end end diff --git a/lib/oci/registry.ex b/lib/oci/registry.ex index c0a81d7..20ec7c9 100644 --- a/lib/oci/registry.ex +++ b/lib/oci/registry.ex @@ -80,14 +80,13 @@ defmodule OCI.Registry do end @spec validate_repository_name(registry :: t(), repo :: repo_t()) :: - {:ok, repo :: repo_t()} | {:error, :NAME_INVALID, String.t()} + {:ok, repo :: repo_t()} | {:error, :NAME_INVALID, map()} def validate_repository_name(registry, repo) do if Regex.match?(registry.repo_name_pattern, repo) do {:ok, repo} else - {:error, :NAME_INVALID, - "Invalid repo name: #{repo}, must match pattern: #{inspect(registry.repo_name_pattern)}."} + {:error, :NAME_INVALID, %{repo: repo, pattern: inspect(registry.repo_name_pattern)}} end end @@ -149,12 +148,11 @@ defmodule OCI.Registry do - `{:error, reason}` if the upload fails """ def upload_blob_chunk(%{storage: storage}, repo, uuid, chunk, maybe_chunk_range, ctx) do - reg = adapter(storage) - - with true <- reg.upload_exists?(storage, repo, uuid, ctx), - {:ok, size} <- reg.get_blob_upload_offset(storage, repo, uuid, ctx), + with :ok <- ensure_upload_exists(storage, repo, uuid, ctx), + {:ok, size} <- adapter(storage).get_blob_upload_offset(storage, repo, uuid, ctx), :ok <- verify_upload_order(size, maybe_chunk_range), - {:ok, range} <- reg.upload_blob_chunk(storage, repo, uuid, chunk, maybe_chunk_range, ctx) do + {:ok, range} <- + adapter(storage).upload_blob_chunk(storage, repo, uuid, chunk, maybe_chunk_range, ctx) do {:ok, blobs_uploads_path(repo, uuid), range} end end @@ -185,14 +183,16 @@ defmodule OCI.Registry do do: {:error, :DIGEST_INVALID, %{repo: repo, uuid: uuid}} def complete_blob_upload(%{storage: storage}, repo, uuid, digest, ctx) do - with true <- adapter(storage).upload_exists?(storage, repo, uuid, ctx), + with :ok <- ensure_upload_exists(storage, repo, uuid, ctx), :ok <- adapter(storage).complete_blob_upload(storage, repo, uuid, digest, ctx) do {:ok, blobs_digest_path(repo, digest)} end end def cancel_blob_upload(%{storage: storage}, repo, uuid, ctx) do - adapter(storage).cancel_blob_upload(storage, repo, uuid, ctx) + with :ok <- ensure_upload_exists(storage, repo, uuid, ctx) do + adapter(storage).cancel_blob_upload(storage, repo, uuid, ctx) + end end def blob_exists?(%{storage: storage}, repo, digest, ctx) do @@ -200,7 +200,9 @@ defmodule OCI.Registry do end def get_blob(%{storage: storage}, repo, digest, ctx) do - adapter(storage).get_blob(storage, repo, digest, ctx) + with :ok <- ensure_repo_exists(storage, repo, ctx) do + adapter(storage).get_blob(storage, repo, digest, ctx) + end end def delete_blob(%{enable_blob_deletion: false}, repo, digest, _ctx), @@ -298,7 +300,9 @@ defmodule OCI.Registry do def referenced_blobs(_manifest), do: [] def get_manifest(%{storage: storage}, repo, reference, ctx) do - adapter(storage).get_manifest(storage, repo, reference, ctx) + with :ok <- ensure_repo_exists(storage, repo, ctx) do + adapter(storage).get_manifest(storage, repo, reference, ctx) + end end def manifest_exists?(%{storage: storage}, repo, reference, ctx) do @@ -306,10 +310,8 @@ defmodule OCI.Registry do end def list_tags(%{storage: storage}, repo, pagination, ctx) do - if repo_exists?(storage, repo, ctx) do + with :ok <- ensure_repo_exists(storage, repo, ctx) do adapter(storage).list_tags(storage, repo, pagination, ctx) - else - {:error, :NAME_UNKNOWN, %{repo: repo}} end end @@ -366,17 +368,14 @@ defmodule OCI.Registry do Returns {:ok, location} on success, {:error, :BLOB_UNKNOWN} if the source blob doesn't exist. """ def mount_blob(%__MODULE__{storage: storage} = registry, repo, digest, from_repo, ctx) do - if repo_exists?(storage, from_repo, ctx) do + with :ok <- ensure_repo_exists(storage, from_repo, ctx) do if blob_exists?(registry, from_repo, digest, ctx) do - # credo:disable-for-next-line with :ok <- adapter(storage).mount_blob(storage, repo, digest, from_repo, ctx) do {:ok, blobs_digest_path(repo, digest)} end else initiate_blob_upload(registry, repo, ctx) end - else - {:error, :NAME_UNKNOWN, %{repo: repo}} end end @@ -431,6 +430,25 @@ defmodule OCI.Registry do "/#{api_version()}/#{repo}/blobs/uploads/#{uuid}" end + # Precondition checks — used by registry functions to validate state before + # delegating to storage adapters. + + defp ensure_repo_exists(storage, repo, ctx) do + if adapter(storage).repo_exists?(storage, repo, ctx) do + :ok + else + {:error, :NAME_UNKNOWN, %{repo: repo}} + end + end + + defp ensure_upload_exists(storage, repo, uuid, ctx) do + if adapter(storage).upload_exists?(storage, repo, uuid, ctx) do + :ok + else + {:error, :BLOB_UPLOAD_UNKNOWN, %{repo: repo, uuid: uuid}} + end + end + @doc """ Calculates the range of a chunk of data. ## Examples diff --git a/lib/oci/storage/adapter.ex b/lib/oci/storage/adapter.ex index 9c6c7bf..88e1c67 100644 --- a/lib/oci/storage/adapter.ex +++ b/lib/oci/storage/adapter.ex @@ -39,7 +39,7 @@ defmodule OCI.Storage.Adapter do @type t :: struct() - @type error_details_t :: any() + @type error_details_t :: map() @doc """ Checks if a blob exists in the repository and returns its size if found. @@ -62,7 +62,6 @@ defmodule OCI.Storage.Adapter do ctx :: OCI.Context.t() ) :: :ok - | {:error, :BLOB_UPLOAD_UNKNOWN} | {:error, :BLOB_UPLOAD_UNKNOWN, error_details_t} @doc """ @@ -75,7 +74,7 @@ defmodule OCI.Storage.Adapter do digest :: Registry.digest_t(), ctx :: OCI.Context.t() ) :: - :ok | {:error, atom()} | {:error, atom(), error_details_t} + :ok | {:error, atom(), error_details_t} @doc """ Deletes a blob from the repository. @@ -86,7 +85,7 @@ defmodule OCI.Storage.Adapter do digest :: Registry.digest_t(), ctx :: OCI.Context.t() ) :: - :ok | {:error, :BLOB_UNKNOWN} | {:error, :BLOB_UNKNOWN, error_details_t} + :ok | {:error, :BLOB_UNKNOWN, error_details_t} @doc """ Deletes a manifest from the repository. @@ -97,7 +96,7 @@ defmodule OCI.Storage.Adapter do reference :: Registry.reference_t(), ctx :: OCI.Context.t() ) :: - :ok | {:error, atom()} + :ok | {:error, atom(), error_details_t} @doc """ Retrieves a blob's content from the repository. @@ -109,7 +108,6 @@ defmodule OCI.Storage.Adapter do ctx :: OCI.Context.t() ) :: {:ok, content :: binary()} - | {:error, :BLOB_UNKNOWN} | {:error, :BLOB_UNKNOWN, error_details_t} @doc """ @@ -122,7 +120,7 @@ defmodule OCI.Storage.Adapter do ctx :: OCI.Context.t() ) :: {:ok, manifest :: binary(), content_type :: String.t()} - | {:error, atom(), error_details_t} + | {:error, :MANIFEST_UNKNOWN, error_details_t} @doc """ Gets the status of an ongoing blob upload. @@ -133,7 +131,7 @@ defmodule OCI.Storage.Adapter do uuid :: Registry.uuid_t(), ctx :: OCI.Context.t() ) :: - {:ok, range :: String.t()} | {:error, term()} | {:error, term(), error_details_t} + {:ok, range :: String.t()} | {:error, :BLOB_UPLOAD_UNKNOWN, error_details_t} @doc """ Gets the total size of an ongoing blob upload. @@ -145,8 +143,7 @@ defmodule OCI.Storage.Adapter do ctx :: OCI.Context.t() ) :: {:ok, size :: non_neg_integer()} - | {:error, term()} - | {:error, term(), error_details_t} + | {:error, :BLOB_UPLOAD_UNKNOWN, error_details_t} @doc """ Gets metadata about a manifest without retrieving its content. @@ -163,7 +160,7 @@ defmodule OCI.Storage.Adapter do Initializes a new storage adapter instance with the given configuration. """ @callback init(config :: map()) :: - {:ok, storage :: t()} | {:error, term()} | {:error, term(), error_details_t} + {:ok, storage :: t()} | {:error, atom(), error_details_t} @doc """ Initiates a blob upload session. @@ -174,8 +171,7 @@ defmodule OCI.Storage.Adapter do ctx :: OCI.Context.t() ) :: {:ok, upload_id :: Registry.uuid_t()} - | {:error, term()} - | {:error, term(), error_details_t} + | {:error, atom(), error_details_t} @doc """ Lists tags in a repository with pagination support. @@ -187,7 +183,6 @@ defmodule OCI.Storage.Adapter do ctx :: OCI.Context.t() ) :: {:ok, tags :: [Registry.tag_t()]} - | {:error, :NAME_UNKNOWN} | {:error, :NAME_UNKNOWN, error_details_t} @doc """ @@ -200,7 +195,7 @@ defmodule OCI.Storage.Adapter do from_repo :: Registry.repo_t(), ctx :: OCI.Context.t() ) :: - :ok | {:error, :BLOB_UNKNOWN} | {:error, :BLOB_UNKNOWN, error_details_t} + :ok | {:error, :BLOB_UNKNOWN, error_details_t} @doc """ Stores a manifest in the repository. @@ -214,8 +209,7 @@ defmodule OCI.Storage.Adapter do ctx :: OCI.Context.t() ) :: :ok - | {:error, :MANIFEST_BLOB_UNKNOWN | :MANIFEST_INVALID | :NAME_UNKNOWN, - error_details_t} + | {:error, atom(), error_details_t} @doc """ Checks if a repository exists. @@ -235,7 +229,6 @@ defmodule OCI.Storage.Adapter do ctx :: OCI.Context.t() ) :: {:ok, range :: String.t()} - | {:error, atom()} | {:error, atom(), error_details_t} @doc """ diff --git a/lib/oci/storage/local.ex b/lib/oci/storage/local.ex index d8709de..7ccb9db 100644 --- a/lib/oci/storage/local.ex +++ b/lib/oci/storage/local.ex @@ -50,7 +50,7 @@ defmodule OCI.Storage.Local do File.rm_rf!(upload_dir) :ok else - {:error, :BLOB_UPLOAD_UNKNOWN} + {:error, :BLOB_UPLOAD_UNKNOWN, %{repo: repo, uuid: uuid}} end end @@ -67,12 +67,6 @@ defmodule OCI.Storage.Local do with :ok <- OCI.Registry.verify_digest(data, digest), :ok <- File.write!(digest_path, data) do :ok - else - {:error, :DIGEST_INVALID} -> - {:error, :DIGEST_INVALID} - - _ -> - {:error, :BLOB_UPLOAD_UNKNOWN} end end @@ -84,7 +78,7 @@ defmodule OCI.Storage.Local do File.rm!(path) :ok else - {:error, :BLOB_UNKNOWN} + {:error, :BLOB_UNKNOWN, %{repo: repo, digest: digest}} end end @@ -96,7 +90,7 @@ defmodule OCI.Storage.Local do File.rm!(manifest_path) :ok else - {:error, :MANIFEST_UNKNOWN} + {:error, :MANIFEST_UNKNOWN, %{repo: repo, reference: digest}} end end @@ -107,7 +101,7 @@ defmodule OCI.Storage.Local do if File.exists?(path) do {:ok, File.read!(path)} else - {:error, :BLOB_UNKNOWN} + {:error, :BLOB_UNKNOWN, %{repo: repo, digest: digest}} end end @@ -120,7 +114,7 @@ defmodule OCI.Storage.Local do {:ok, manifest, @manifest_v1_content_type} _ -> - {:error, :MANIFEST_UNKNOWN, "Reference `#{reference}` not found for repo #{repo}"} + {:error, :MANIFEST_UNKNOWN, %{repo: repo, reference: reference}} end end @@ -130,7 +124,7 @@ defmodule OCI.Storage.Local do get_manifest(storage, repo, digest, ctx) _ -> - {:error, :MANIFEST_UNKNOWN, "Reference `#{tag}` not found for repo #{repo}"} + {:error, :MANIFEST_UNKNOWN, %{repo: repo, reference: tag}} end end @@ -150,7 +144,7 @@ defmodule OCI.Storage.Local do range = OCI.Registry.calculate_range(data, 0) {:ok, range} else - {:error, :BLOB_UPLOAD_UNKNOWN} + {:error, :BLOB_UPLOAD_UNKNOWN, %{repo: repo, uuid: uuid}} end end @@ -258,7 +252,7 @@ defmodule OCI.Storage.Local do File.cp!(source_path, target_path) :ok else - {:error, :BLOB_UNKNOWN} + {:error, :BLOB_UNKNOWN, %{repo: from_repo, digest: digest}} end end diff --git a/mix.exs b/mix.exs index 778b423..f20b755 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule OCI.MixProject do def project do [ app: :oci, - version: "0.1.0", + version: "0.2.0", elixir: "~> 1.15", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, diff --git a/test/plug_test.exs b/test/plug_test.exs index cf83d20..13ef370 100644 --- a/test/plug_test.exs +++ b/test/plug_test.exs @@ -111,8 +111,10 @@ defmodule OCI.PlugTest do "errors" => [ %{ "code" => "NAME_INVALID", - "detail" => - "Invalid repo name: nosinglelevelnames, must match pattern: ~r/^[a-z0-9]+\\/[a-z0-9]+$/.", + "detail" => %{ + "repo" => "nosinglelevelnames", + "pattern" => "~r/^[a-z0-9]+\\/[a-z0-9]+$/" + }, "message" => "invalid repository name" } ] From 13e26e3bbef77483392fa2dd4cf45e067abffc93 Mon Sep 17 00:00:00 2001 From: Cory O'Daniel Date: Wed, 1 Apr 2026 11:14:52 -0700 Subject: [PATCH 2/5] Disable credo ABC size check for Context.call User prompts: - "add a credo ignore rule for ABC size on context.ex" Changes: - Add credo:disable-for-next-line for Credo.Check.Refactor.ABCSize on Context.call Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/oci/plug/context.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/oci/plug/context.ex b/lib/oci/plug/context.ex index 21ca1cb..0262354 100644 --- a/lib/oci/plug/context.ex +++ b/lib/oci/plug/context.ex @@ -12,6 +12,7 @@ defmodule OCI.Plug.Context do def init(opts), do: opts + # credo:disable-for-next-line Credo.Check.Refactor.ABCSize def call(conn, _opts \\ []) do segments = conn.path_info |> Enum.reverse() From 7a73bec1dcf8481124fc4f0b6bdda23adb5cdab7 Mon Sep 17 00:00:00 2001 From: Cory O'Daniel Date: Fri, 3 Apr 2026 08:52:44 -0700 Subject: [PATCH 3/5] Preserve raw manifest bytes for consistent hashing User prompts: - "we need the raw http body to properly hash, currently we parse and pass on the json object in parse_oci_manifest" - "store the raw and digest in assigns, pass manifest, manifest_raw, and digest to store_manifest" Changes: - Parser stores raw body in conn.assigns[:oci_raw_manifest] alongside digest - Handler passes parsed manifest, raw bytes, and digest to Registry.store_manifest - Registry uses parsed map for validation/indexing, raw bytes for storage - Local storage writes raw bytes to disk instead of re-encoding JSON - Storage adapter callback type updated from map() to binary() Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/oci/plug/handler.ex | 22 ++++++++--------- lib/oci/plug/parser.ex | 50 ++++++++++++++------------------------ lib/oci/registry.ex | 16 ++++++------ lib/oci/storage/adapter.ex | 2 +- lib/oci/storage/local.ex | 4 +-- 5 files changed, 37 insertions(+), 57 deletions(-) diff --git a/lib/oci/plug/handler.ex b/lib/oci/plug/handler.ex index c8168c5..4ab9a54 100644 --- a/lib/oci/plug/handler.ex +++ b/lib/oci/plug/handler.ex @@ -268,21 +268,12 @@ defmodule OCI.Plug.Handler do defp dispatch(%{method: "PUT"} = conn, :manifests, registry, repo, reference, ctx) do manifest = conn.params + raw_manifest = conn.assigns[:oci_raw_manifest] manifest_digest = conn.assigns[:oci_digest] - with :ok <- Registry.store_manifest(registry, repo, reference, manifest, manifest_digest, ctx) do - maybe_set_oci_subject = fn conn -> - case get_in(conn.params, ["subject", "digest"]) do - nil -> - conn - - subject_digest -> - put_resp_header(conn, "oci-subject", subject_digest) - end - end - + with :ok <- Registry.store_manifest(registry, repo, reference, manifest, raw_manifest, manifest_digest, ctx) do conn - |> maybe_set_oci_subject.() + |> maybe_set_oci_subject(manifest) |> put_resp_header("location", Registry.manifests_reference_path(repo, reference)) |> send_resp(201, "") end @@ -319,6 +310,13 @@ defmodule OCI.Plug.Handler do {:error, :UNSUPPORTED, %{method: method, path: path}} end + defp maybe_set_oci_subject(conn, manifest) do + case get_in(manifest, ["subject", "digest"]) do + nil -> conn + subject_digest -> put_resp_header(conn, "oci-subject", subject_digest) + end + end + defp pagination(params) do n = if params["n"], do: String.to_integer(params["n"]), else: nil last = params["last"] diff --git a/lib/oci/plug/parser.ex b/lib/oci/plug/parser.ex index a460f2f..375b4bf 100644 --- a/lib/oci/plug/parser.ex +++ b/lib/oci/plug/parser.ex @@ -25,32 +25,15 @@ defmodule OCI.Plug.Parser do ## Content Types Handled ### application/octet-stream - Handles binary blob uploads by reading the full body and storing - it in the connection assigns under the `:oci_blob_chunk` key. + Reads the full body and stores it in `conn.assigns[:oci_blob_chunk]`. - ### application/vnd.oci.image.manifest.v1+json - Handles OCI image manifest uploads by: - 1. Reading the full body - 2. Computing its SHA256 digest - 3. Storing the digest in the connection assigns - 4. Decoding the JSON manifest + ### OCI manifest types + Reads the full body, computes its SHA256 digest, decodes the JSON, and stores + the raw body and digest in conn assigns (`:oci_raw_manifest` and `:oci_digest`). + The decoded manifest is returned as params. ### Other Content Types - Passes through to the next parser in the chain. - - ## Parameters - - conn: The Plug.Conn struct - - type: The content type - - subtype: The content subtype - - headers: The request headers - - opts: Parser options containing a :json_decoder key for manifest parsing - - ## Returns - - For octet-stream: `{:ok, %{}, conn}` on successful parsing - - For manifest: `{:ok, manifest, conn}` on successful parsing - - For other types: `{:next, conn}` to pass to the next parser - - `{:error, reason}` on failure - - Raises `Plug.Parsers.ParseError` on JSON decode failure for manifests + Passes through to the next parser via `{:next, conn}`. """ def parse(conn, "application", "octet-stream", _headers, opts) do read_full_body(conn, opts, "") @@ -65,22 +48,25 @@ defmodule OCI.Plug.Parser do end end - def parse(conn, "application", "vnd.oci.image.manifest.v1+json", headers, opts) do - parse_oci_manifest(conn, headers, opts) + def parse(conn, "application", "vnd.oci.image.manifest.v1+json", _headers, opts) do + read_oci_manifest(conn, opts) end - def parse(conn, "application", "vnd.oci.image.index.v1+json", headers, opts) do - parse_oci_manifest(conn, headers, opts) + def parse(conn, "application", "vnd.oci.image.index.v1+json", _headers, opts) do + read_oci_manifest(conn, opts) end def parse(conn, _type, _subtype, _headers, _opts), do: {:next, conn} - defp parse_oci_manifest(conn, _headers, opts) do - read_full_body(conn, opts, "") - |> case do + defp read_oci_manifest(conn, opts) do + case read_full_body(conn, opts, "") do {:ok, full_body, conn} -> digest = :crypto.hash(:sha256, full_body) |> Base.encode16(case: :lower) - conn = Plug.Conn.assign(conn, :oci_digest, "sha256:#{digest}") + + conn = + conn + |> Plug.Conn.assign(:oci_digest, "sha256:#{digest}") + |> Plug.Conn.assign(:oci_raw_manifest, full_body) decoder = Keyword.fetch!(opts, :json_decoder) @@ -92,7 +78,7 @@ defmodule OCI.Plug.Parser do raise Plug.Parsers.ParseError, exception: %Plug.Parsers.BadEncodingError{} end - err -> + {:error, _} = err -> err end end diff --git a/lib/oci/registry.ex b/lib/oci/registry.ex index 20ec7c9..511df42 100644 --- a/lib/oci/registry.ex +++ b/lib/oci/registry.ex @@ -223,7 +223,7 @@ defmodule OCI.Registry do end end - def store_manifest(%{storage: storage}, repo, reference, manifest, manifest_digest, ctx) do + def store_manifest(%{storage: storage}, repo, reference, manifest, raw_manifest, manifest_digest, ctx) do required_blobs = referenced_blobs(manifest) missing = @@ -239,11 +239,11 @@ defmodule OCI.Registry do storage, repo, reference, - manifest, + raw_manifest, manifest_digest, ctx ) do - maybe_index_referrer(storage, repo, manifest, manifest_digest, ctx) + maybe_index_referrer(storage, repo, manifest, raw_manifest, manifest_digest, ctx) :ok end end @@ -259,10 +259,10 @@ defmodule OCI.Registry do adapter(storage).list_referrers(storage, repo, digest, filters, ctx) end - defp maybe_index_referrer(storage, repo, manifest, manifest_digest, ctx) do + defp maybe_index_referrer(storage, repo, manifest, raw_manifest, manifest_digest, ctx) do case manifest["subject"] do %{"digest" => subject_digest} when is_binary(subject_digest) -> - descriptor = build_referrer_descriptor(manifest, manifest_digest) + descriptor = build_referrer_descriptor(manifest, raw_manifest, manifest_digest) adapter(storage).put_referrer(storage, repo, subject_digest, descriptor, ctx) _ -> @@ -270,16 +270,14 @@ defmodule OCI.Registry do end end - defp build_referrer_descriptor(manifest, manifest_digest) do - manifest_json = Jason.encode!(manifest) - + defp build_referrer_descriptor(manifest, raw_manifest, manifest_digest) do artifact_type = manifest["artifactType"] || get_in(manifest, ["config", "mediaType"]) descriptor = %{ "mediaType" => manifest["mediaType"], "digest" => manifest_digest, - "size" => byte_size(manifest_json), + "size" => byte_size(raw_manifest), "artifactType" => artifact_type } diff --git a/lib/oci/storage/adapter.ex b/lib/oci/storage/adapter.ex index 88e1c67..fdcc3a4 100644 --- a/lib/oci/storage/adapter.ex +++ b/lib/oci/storage/adapter.ex @@ -204,7 +204,7 @@ defmodule OCI.Storage.Adapter do storage :: t(), repo :: Registry.repo_t(), reference :: Registry.reference_t(), - manifest :: map(), + manifest :: binary(), manifest_digest :: Registry.digest_t(), ctx :: OCI.Context.t() ) :: diff --git a/lib/oci/storage/local.ex b/lib/oci/storage/local.ex index 7ccb9db..e74d5a8 100644 --- a/lib/oci/storage/local.ex +++ b/lib/oci/storage/local.ex @@ -258,10 +258,8 @@ defmodule OCI.Storage.Local do @impl true def store_manifest(storage, repo, reference, manifest, manifest_digest, _ctx) do - manifest_json = Jason.encode!(manifest) - :ok = File.mkdir_p!(manifests_dir(storage, repo)) - File.write!(digest_path(storage, repo, manifest_digest), manifest_json) + File.write!(digest_path(storage, repo, manifest_digest), manifest) # If reference is a tag, create a tag reference if !String.starts_with?(reference, "sha256:") do From 9143c6f19f04cc5f19335810c35fe734c0925df1 Mon Sep 17 00:00:00 2001 From: Cory O'Daniel Date: Fri, 3 Apr 2026 08:54:05 -0700 Subject: [PATCH 4/5] Pass parsed manifest map to storage adapter User prompts: - "pass the parsed map to store_manifest so they dont have to reparse" Changes: - Storage adapter callback accepts both manifest (map) and raw_manifest (binary) - Registry passes both through to the adapter - Local adapter receives parsed map for future use, writes raw bytes to disk Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/oci/registry.ex | 1 + lib/oci/storage/adapter.ex | 3 ++- lib/oci/storage/local.ex | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/oci/registry.ex b/lib/oci/registry.ex index 511df42..f047450 100644 --- a/lib/oci/registry.ex +++ b/lib/oci/registry.ex @@ -239,6 +239,7 @@ defmodule OCI.Registry do storage, repo, reference, + manifest, raw_manifest, manifest_digest, ctx diff --git a/lib/oci/storage/adapter.ex b/lib/oci/storage/adapter.ex index fdcc3a4..013a689 100644 --- a/lib/oci/storage/adapter.ex +++ b/lib/oci/storage/adapter.ex @@ -204,7 +204,8 @@ defmodule OCI.Storage.Adapter do storage :: t(), repo :: Registry.repo_t(), reference :: Registry.reference_t(), - manifest :: binary(), + manifest :: map(), + raw_manifest :: binary(), manifest_digest :: Registry.digest_t(), ctx :: OCI.Context.t() ) :: diff --git a/lib/oci/storage/local.ex b/lib/oci/storage/local.ex index e74d5a8..aaa815f 100644 --- a/lib/oci/storage/local.ex +++ b/lib/oci/storage/local.ex @@ -257,9 +257,9 @@ defmodule OCI.Storage.Local do end @impl true - def store_manifest(storage, repo, reference, manifest, manifest_digest, _ctx) do + def store_manifest(storage, repo, reference, _manifest, raw_manifest, manifest_digest, _ctx) do :ok = File.mkdir_p!(manifests_dir(storage, repo)) - File.write!(digest_path(storage, repo, manifest_digest), manifest) + File.write!(digest_path(storage, repo, manifest_digest), raw_manifest) # If reference is a tag, create a tag reference if !String.starts_with?(reference, "sha256:") do From 6f5c9fad02db5fc7d20b96bf52cb5a18bdaeeba1 Mon Sep 17 00:00:00 2001 From: Cory O'Daniel Date: Wed, 6 May 2026 13:28:36 -0700 Subject: [PATCH 5/5] fmt + content-length:0 --- lib/oci/plug/handler.ex | 29 ++++++++++++++++++++--------- lib/oci/registry.ex | 10 +++++++++- test/plug_test.exs | 27 +++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/lib/oci/plug/handler.ex b/lib/oci/plug/handler.ex index 4ab9a54..024b07d 100644 --- a/lib/oci/plug/handler.ex +++ b/lib/oci/plug/handler.ex @@ -271,7 +271,16 @@ defmodule OCI.Plug.Handler do raw_manifest = conn.assigns[:oci_raw_manifest] manifest_digest = conn.assigns[:oci_digest] - with :ok <- Registry.store_manifest(registry, repo, reference, manifest, raw_manifest, manifest_digest, ctx) do + with :ok <- + Registry.store_manifest( + registry, + repo, + reference, + manifest, + raw_manifest, + manifest_digest, + ctx + ) do conn |> maybe_set_oci_subject(manifest) |> put_resp_header("location", Registry.manifests_reference_path(repo, reference)) @@ -334,18 +343,20 @@ defmodule OCI.Plug.Handler do end defp maybe_upload_final_chunk(conn, registry, repo, uuid, ctx) do - # The Content-Length header is required, but may be 0 if no final chunk is being uploaded. - conn - |> get_req_header("content-length") - |> List.first() - |> String.to_integer() - |> case do + # The closing PUT may have no body and no Content-Length header when the + # final chunk was uploaded earlier via PATCH (spec: "Content-Length: " and "OPTIONAL: "). + content_length = + case get_req_header(conn, "content-length") do + [val | _] -> String.to_integer(val) + [] -> 0 + end + + case content_length do 0 -> - # No chunk to upload with final PUT :ok _ -> - # Final chunk is included, upload it before completing the blob case Registry.upload_blob_chunk( registry, repo, diff --git a/lib/oci/registry.ex b/lib/oci/registry.ex index f047450..14b463f 100644 --- a/lib/oci/registry.ex +++ b/lib/oci/registry.ex @@ -223,7 +223,15 @@ defmodule OCI.Registry do end end - def store_manifest(%{storage: storage}, repo, reference, manifest, raw_manifest, manifest_digest, ctx) do + def store_manifest( + %{storage: storage}, + repo, + reference, + manifest, + raw_manifest, + manifest_digest, + ctx + ) do required_blobs = referenced_blobs(manifest) missing = diff --git a/test/plug_test.exs b/test/plug_test.exs index 13ef370..a8fa5ee 100644 --- a/test/plug_test.exs +++ b/test/plug_test.exs @@ -121,4 +121,31 @@ defmodule OCI.PlugTest do } end end + + describe "PUT /v2//blobs/uploads/ closing PUT" do + test "finalizes a zero-byte blob when Content-Length header is omitted", %{conn: conn} do + open = post(conn, "/myimage/blobs/uploads") + assert open.status == 202 + [location] = get_resp_header(open, "location") + uuid = location |> String.split("/") |> List.last() + + empty_digest = digest("") + + assert empty_digest == + "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + + closing = + :put + |> conn("/myimage/blobs/uploads/#{uuid}?digest=#{empty_digest}", nil) + |> Map.put(:script_name, ["v2"]) + |> Map.put(:assigns, %{oci_opts: conn.assigns.oci_opts}) + |> basic_auth("myuser", "mypass") + |> OCI.Plug.call(conn.assigns.oci_opts) + + assert get_req_header(closing, "content-length") == [] + assert closing.status == 201 + assert [blob_location] = get_resp_header(closing, "location") + assert blob_location == "/v2/myimage/blobs/#{empty_digest}" + end + end end