diff --git a/lib/hexdocs/bucket.ex b/lib/hexdocs/bucket.ex index 03da4c9..61f0388 100644 --- a/lib/hexdocs/bucket.ex +++ b/lib/hexdocs/bucket.ex @@ -167,25 +167,22 @@ defmodule Hexdocs.Bucket do case Hexdocs.Store.get_to_file(:repo_bucket, key, tarball_path) do :ok -> - case Hexdocs.Tar.unpack_to_dir({:file, tarball_path}, - repository: repository, - package: package, - version: version - ) do - {:ok, dir, files} -> - upload_files = - list_upload_files(repository, package, new_latest_version, dir, files, :both) - - paths = MapSet.new(upload_files, &elem(&1, 0)) - update_versions = [version, new_latest_version] - - upload_new_files(upload_files) - delete_old_docs(repository, package, update_versions, paths, :both) - purge_hexdocs_cache(repository, package, update_versions, :both) - - {:error, reason} -> - Logger.error("Failed unpack #{repository}/#{package} #{version}: #{reason}") - end + {dir, files} = + Hexdocs.Tar.unpack_to_dir!({:file, tarball_path}, + repository: repository, + package: package, + version: new_latest_version + ) + + upload_files = + list_upload_files(repository, package, new_latest_version, dir, files, :both) + + paths = MapSet.new(upload_files, &elem(&1, 0)) + update_versions = [version, new_latest_version] + + upload_new_files(upload_files) + delete_old_docs(repository, package, update_versions, paths, :both) + purge_hexdocs_cache(repository, package, update_versions, :both) nil -> Logger.error("Failed to get tarball #{repository}/#{package} #{new_latest_version}") diff --git a/lib/hexdocs/queue.ex b/lib/hexdocs/queue.ex index f9a101c..84031ed 100644 --- a/lib/hexdocs/queue.ex +++ b/lib/hexdocs/queue.ex @@ -66,19 +66,16 @@ defmodule Hexdocs.Queue do case Hexdocs.Store.get_to_file(:repo_bucket, key, tarball_path) do :ok -> - case Hexdocs.Tar.unpack_to_dir({:file, tarball_path}, - repository: repository, - package: package, - version: version - ) do - {:ok, _dir, files} -> - update_index_sitemap(repository, key) - update_package_sitemap(repository, key, package, files) - Logger.info("#{key}: done") - - {:error, reason} -> - Logger.error("Failed unpack #{repository}/#{package} #{version}: #{reason}") - end + {_dir, files} = + Hexdocs.Tar.unpack_to_dir!({:file, tarball_path}, + repository: repository, + package: package, + version: version + ) + + update_index_sitemap(repository, key) + update_package_sitemap(repository, key, package, files) + Logger.info("#{key}: done") nil -> Logger.error("#{key}: package not found in store") @@ -163,36 +160,33 @@ defmodule Hexdocs.Queue do {version, all_versions, retired_versions} end - case Hexdocs.Tar.unpack_to_dir(input, - repository: repository, - package: package, - version: version - ) do - {:ok, dir, files} -> - rewrite_files(dir, files) - - Hexdocs.Bucket.upload( - repository, - package, - version, - all_versions, - retired_versions, - dir, - files - ) + {dir, files} = + Hexdocs.Tar.unpack_to_dir!(input, + repository: repository, + package: package, + version: version + ) - if Hexdocs.Utils.latest_version?(package, version, all_versions) do - update_index_sitemap(repository, key) - update_package_sitemap(repository, key, package, files) - update_package_names_csv(repository) - end + rewrite_files(dir, files) - elapsed = System.os_time(:millisecond) - start - Logger.info("FINISHED UPLOADING DOCS #{key} #{elapsed}ms") + Hexdocs.Bucket.upload( + repository, + package, + version, + all_versions, + retired_versions, + dir, + files + ) - {:error, reason} -> - Logger.error("Failed unpack #{repository}/#{package} #{version}: #{reason}") + if Hexdocs.Utils.latest_version?(package, version, all_versions) do + update_index_sitemap(repository, key) + update_package_sitemap(repository, key, package, files) + update_package_names_csv(repository) end + + elapsed = System.os_time(:millisecond) - start + Logger.info("FINISHED UPLOADING DOCS #{key} #{elapsed}ms") end defp process_search(key, repository, package, version, input, start) do @@ -205,15 +199,16 @@ defmodule Hexdocs.Queue do :error when package in @special_package_names -> version end - case Hexdocs.Tar.unpack_to_dir(input, package: package, version: version) do - {:ok, dir, files} -> - update_search_index(key, package, version, dir, files) - elapsed = System.os_time(:millisecond) - start - Logger.info("FINISHED INDEXING DOCS #{key} #{elapsed}ms") + {dir, files} = + Hexdocs.Tar.unpack_to_dir!(input, + repository: repository, + package: package, + version: version + ) - {:error, reason} -> - Logger.error("Failed unpack #{package} #{version}: #{reason}") - end + update_search_index(key, package, version, dir, files) + elapsed = System.os_time(:millisecond) - start + Logger.info("FINISHED INDEXING DOCS #{key} #{elapsed}ms") end end diff --git a/lib/hexdocs/tar.ex b/lib/hexdocs/tar.ex index 491c8f4..a867ba6 100644 --- a/lib/hexdocs/tar.ex +++ b/lib/hexdocs/tar.ex @@ -1,13 +1,22 @@ defmodule Hexdocs.Tar do require Logger + defmodule UnpackError do + defexception [:repository, :package, :version, :reason] + + @impl true + def message(%{repository: repository, package: package, version: version, reason: reason}) do + "Failed to unpack #{repository}/#{package} #{version}: #{reason}" + end + end + def create(files) do files = for {path, contents} <- files, do: {String.to_charlist(path), contents} {:ok, tarball} = :hex_tarball.create_docs(files) tarball end - def unpack_to_dir({:file, path}, opts \\ []) do + def unpack_to_dir!({:file, path}, opts \\ []) do repository = Keyword.get(opts, :repository, "UNKNOWN") package = Keyword.get(opts, :package, "UNKNOWN") version = Keyword.get(opts, :version, "UNKNOWN") @@ -26,14 +35,15 @@ defmodule Hexdocs.Tar do |> Enum.map(&Path.relative_to(&1, output_dir)) files = fix_paths(repository, package, version, files) - - case check_version_dirs(files) do - :ok -> {:ok, output_dir, files} - {:error, _} = error -> error - end + check_version_dirs!(repository, package, version, files) + {output_dir, files} {:error, reason} -> - {:error, inspect(reason)} + raise UnpackError, + repository: repository, + package: package, + version: version, + reason: inspect(reason) end end @@ -55,17 +65,19 @@ defmodule Hexdocs.Tar do end) end - defp check_version_dirs(files) do - result = + defp check_version_dirs!(repository, package, version, files) do + ok? = Enum.all?(files, fn path -> first = Path.split(path) |> hd() Version.parse(first) == :error end) - if result do - :ok - else - {:error, "root file or directory name not allowed to match a semver version"} + if not ok? do + raise UnpackError, + repository: repository, + package: package, + version: version, + reason: "root file or directory name not allowed to match a semver version" end end diff --git a/test/hexdocs/tar_test.exs b/test/hexdocs/tar_test.exs index 299e1ff..4b30ad3 100644 --- a/test/hexdocs/tar_test.exs +++ b/test/hexdocs/tar_test.exs @@ -2,12 +2,12 @@ defmodule Hexdocs.TarTest do use ExUnit.Case, async: true alias Hexdocs.Tar - test "unpack_to_dir" do + test "unpack_to_dir!" do blob = Tar.create([{"index.html", "contents"}, {"foo.bar", "contents"}]) path = Hexdocs.TmpDir.tmp_file("test-tarball") File.write!(path, blob) - assert {:ok, dir, files} = Tar.unpack_to_dir({:file, path}) + assert {dir, files} = Tar.unpack_to_dir!({:file, path}) assert File.dir?(dir) assert length(files) == 2 assert "index.html" in files @@ -19,19 +19,59 @@ defmodule Hexdocs.TarTest do test "invalid gzip" do path = Hexdocs.TmpDir.tmp_file("test-tarball") File.write!(path, "") - assert {:error, _} = Tar.unpack_to_dir({:file, path}) + + assert_raise Tar.UnpackError, ~r/Failed to unpack hexpm\/foo 1\.0\.0:/, fn -> + Tar.unpack_to_dir!({:file, path}, repository: "hexpm", package: "foo", version: "1.0.0") + end end test "do not allow root files/directories with version names" do - reason = "root file or directory name not allowed to match a semver version" - blob = Tar.create([{"1.0.0", "contents"}]) path = Hexdocs.TmpDir.tmp_file("test-tarball") File.write!(path, blob) - assert Tar.unpack_to_dir({:file, path}) == {:error, reason} + + assert_raise Tar.UnpackError, + ~r/root file or directory name not allowed to match a semver version/, + fn -> + Tar.unpack_to_dir!({:file, path}, + repository: "hexpm", + package: "foo", + version: "1.0.0" + ) + end blob = Tar.create([{"1.0.0/index.html", "contents"}]) File.write!(path, blob) - assert Tar.unpack_to_dir({:file, path}) == {:error, reason} + + assert_raise Tar.UnpackError, + ~r/root file or directory name not allowed to match a semver version/, + fn -> + Tar.unpack_to_dir!({:file, path}, + repository: "hexpm", + package: "foo", + version: "1.0.0" + ) + end + end + + test "raises on tarball with duplicate mode-0 entries" do + path = Hexdocs.TmpDir.tmp_file("test-tarball") + {:ok, tar} = :hex_erl_tar.open(String.to_charlist(path), [:write, :compressed]) + + for _ <- 1..3 do + :ok = :hex_erl_tar.add(tar, "contents", ~c"#", [{:mode, 0}]) + end + + :ok = :hex_erl_tar.close(tar) + + assert_raise Tar.UnpackError, + ~r/Failed to unpack hexpm\/lustre 5\.7\.0: :eacces/, + fn -> + Tar.unpack_to_dir!({:file, path}, + repository: "hexpm", + package: "lustre", + version: "5.7.0" + ) + end end end