diff --git a/CHANGELOG.md b/CHANGELOG.md index 2789960..57ca70e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,18 @@ speed of 0 B/s reported on a torrent moving at 100 KB/s. oscillates, and a torrent slower than one piece per window reads 0 until its first piece lands. +- A disk error while serving a BEP 52 hash request returns an error instead of + raising. `Merkle.leaf_range_response_from_disk/7` documents + `{:error, term()}` and handles a failed `:file.open/2` that way, but the + per-leaf reads underneath it pattern-matched `{:ok, block} = :file.pread/3`, so + an I/O error — or a file truncated between the stat that produced `file_length` + and the read — raised `MatchError` from the middle of the function. `HashServe` + catches that and answers `hash_reject`, which is correct on the wire but + reached by the wrong path; any other caller got an exception for a disk + condition. `:file.pread/3` also answers a bare `:eof` rather than an error + tuple, which is now distinguished from the legitimate padded-leaf case that + reads no bytes at all. + ### Changed - The HTTP stack behind tracker announces and BEP 19 web seeds moved up: hackney diff --git a/lib/elixir_torrent/torrent/merkle.ex b/lib/elixir_torrent/torrent/merkle.ex index 784c40f..487b91b 100644 --- a/lib/elixir_torrent/torrent/merkle.ex +++ b/lib/elixir_torrent/torrent/merkle.ex @@ -679,9 +679,8 @@ defmodule Torrent.Merkle do ctx.proof_layers ) - cache = read_leaf_cache(fd, ctx.file_length, ctx.block_count, indices) - - with :ok <- + with {:ok, cache} <- read_leaf_cache(fd, ctx.file_length, ctx.block_count, indices), + :ok <- verify_piece_subtrees( cache, ctx.piece_hashes, @@ -1274,34 +1273,48 @@ defmodule Torrent.Merkle do |> MapSet.filter(&(&1 < block_count)) end + # Stops at the first unreadable leaf rather than hashing the rest, because the + # cache is only useful complete — every consumer below does `Map.fetch!/2`. defp read_leaf_cache(fd, file_length, block_count, indices) do - Enum.reduce(indices, %{}, fn leaf, acc -> - Map.put(acc, leaf, leaf_hash_from_fd(fd, file_length, block_count, leaf)) + Enum.reduce_while(indices, {:ok, %{}}, fn leaf, {:ok, acc} -> + case leaf_hash_from_fd(fd, file_length, block_count, leaf) do + {:ok, hash} -> {:cont, {:ok, Map.put(acc, leaf, hash)}} + {:error, reason} -> {:halt, {:error, reason}} + end end) end defp leaf_hash_from_fd(fd, file_length, block_count, leaf_index) do offset = leaf_index * @block_size - data = + with {:ok, data} <- read_leaf_bytes(fd, offset, file_length) do cond do - offset >= file_length -> - <<>> + leaf_index >= block_count -> {:ok, @zero_hash} + byte_size(data) == 0 -> {:ok, @zero_hash} + true -> {:ok, :crypto.hash(:sha256, data)} + end + end + end - offset + @block_size > file_length -> - size = file_length - offset - {:ok, block} = :file.pread(fd, offset, size) - block + # A leaf past the end of the file is a legitimate request on a padded tree, not + # a read at all: BEP 52 pads the leaf layer to a power of two and those leaves + # hash to `@zero_hash`. + defp read_leaf_bytes(_fd, offset, file_length) when offset >= file_length, do: {:ok, <<>>} - true -> - {:ok, block} = :file.pread(fd, offset, @block_size) - block - end + defp read_leaf_bytes(fd, offset, file_length) do + case :file.pread(fd, offset, min(@block_size, file_length - offset)) do + {:ok, block} -> + {:ok, block} - cond do - leaf_index >= block_count -> @zero_hash - byte_size(data) == 0 -> @zero_hash - true -> :crypto.hash(:sha256, data) + # `pread` answers a bare `:eof` instead of an error tuple when there is + # nothing at the offset. Reaching it here means `file_length` no longer + # describes the file — truncated under us between the `File.stat` and this + # read — so it is a real failure and not the padded-leaf case above. + :eof -> + {:error, :eof} + + {:error, reason} -> + {:error, reason} end end diff --git a/test/torrent_merkle_test.exs b/test/torrent_merkle_test.exs index fadc9fc..782875c 100644 --- a/test/torrent_merkle_test.exs +++ b/test/torrent_merkle_test.exs @@ -533,6 +533,78 @@ defmodule Torrent.MerkleTest do assert Merkle.leaf_range_response_from_disk(path, 1, [hash("x")], @block_size, 1, 2, 0) == {:error, :invalid_index} end + + test "returns an error tuple when the file is shorter than the declared length" do + # `file_length` comes from a stat taken before the read, so a file truncated + # or replaced in between leaves `:file.pread/3` reading past the real EOF. + # The function documents `{:error, term()}` and already handles a failed + # `:file.open/2` that way; a failed read used to raise `MatchError` out of + # the middle instead. `HashServe` catches that and answers `hash_reject`, + # which is the right thing on the wire (BEP 52) reached by the wrong path — + # and anything else calling this got an exception for a disk condition. + blocks = for byte <- [?a, ?b, ?c, ?d], do: :binary.copy(<>, @block_size) + content = IO.iodata_to_binary(blocks) + {:ok, tree} = Merkle.build(content) + piece_length = 2 * @block_size + {:ok, layer_bin} = Merkle.piece_layer(tree, piece_length) + + piece_hashes = + for <> do + digest + end + + dir = Path.join(System.tmp_dir!(), "merkle_short_#{System.unique_integer([:positive])}") + File.mkdir_p!(dir) + path = Path.join(dir, "truncated.bin") + # Only the first block is actually on disk. + File.write!(path, :binary.part(content, 0, @block_size)) + on_exit(fn -> File.rm_rf!(dir) end) + + assert Merkle.leaf_range_response_from_disk( + path, + byte_size(content), + piece_hashes, + piece_length, + 0, + 4, + 0 + ) == {:error, :eof} + end + + test "still serves a whole file whose last block is short" do + # The guard added alongside the error contract must not disturb the ordinary + # ragged-tail case: a final partial block is normal, not a truncated file. + full = for byte <- [?a, ?b, ?c, ?d, ?e, ?f, ?g], do: :binary.copy(<>, @block_size) + content = IO.iodata_to_binary(full) <> :binary.copy(<>, 100) + {:ok, tree} = Merkle.build(content) + root = Merkle.root(tree) + piece_length = 4 * @block_size + {:ok, layer_bin} = Merkle.piece_layer(tree, piece_length) + + piece_hashes = + for <> do + digest + end + + dir = Path.join(System.tmp_dir!(), "merkle_ragged_#{System.unique_integer([:positive])}") + File.mkdir_p!(dir) + path = Path.join(dir, "ragged.bin") + File.write!(path, content) + on_exit(fn -> File.rm_rf!(dir) end) + + assert {:ok, hashes} = + Merkle.leaf_range_response_from_disk( + path, + byte_size(content), + piece_hashes, + piece_length, + 0, + 2, + 2 + ) + + assert Merkle.verify_hashes(root, 0, 0, 2, 2, hashes, 8) + end end describe "libtorrent flat proof helpers" do