Skip to content

Commit dfec67f

Browse files
0g-peterzhbclaude
andcommitted
transfer: compare download roots as hashes, not as text (#195)
validateDownloadFile compared the caller's root string against Hash.Hex's canonical rendering. Every other step normalises it through common.HexToHash - downloadAndValidate does exactly that for the file query and the existence check - but this one compared raw text, and HexToHash accepts forms Hex never emits: uppercase digits, a missing 0x prefix, fewer than 64 digits. So download --root 0xABC... transferred the whole file, validated its merkle root correctly, then failed with "Merkle root mismatch" over the spelling of the argument. The message printed only the downloaded root, so the two values looked identical apart from case and gave no hint the comparison was textual. Compare hashes, and name both roots in the error so a genuine mismatch is legible. Also changed checkFileExistence from comparing Hex renderings to comparing hashes. That one was already correct, since both sides were canonical, but comparing hashes is clearer and removes the pattern from the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 89513af commit dfec67f

2 files changed

Lines changed: 77 additions & 3 deletions

File tree

transfer/downloader.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ func checkFileExistence(filename string, hash common.Hash) error {
288288
return errors.WithMessage(err, "Failed to create file merkle tree")
289289
}
290290

291-
if tree.Root().Hex() == hash.Hex() {
291+
if tree.Root() == hash {
292292
return ErrFileAlreadyExists
293293
}
294294

@@ -338,8 +338,13 @@ func (downloader *Downloader) validateDownloadFile(root, filename string, fileSi
338338
return errors.WithMessage(err, "Failed to create merkle tree")
339339
}
340340

341-
if rootHex := tree.Root().Hex(); rootHex != root {
342-
return errors.Errorf("Merkle root mismatch, downloaded = %v", rootHex)
341+
// Compare hashes, not strings. The caller's root is whatever text they passed in, and
342+
// common.HexToHash accepts forms that Hash.Hex never produces - uppercase digits, a
343+
// missing 0x prefix, fewer than 64 digits - so every other step here normalised it
344+
// while this one rejected it. The download would complete and then be discarded for
345+
// its spelling.
346+
if expected := common.HexToHash(root); tree.Root() != expected {
347+
return errors.Errorf("Merkle root mismatch, expected = %v, downloaded = %v", expected.Hex(), tree.Root().Hex())
343348
}
344349

345350
downloader.logger.Info("Succeeded to validate the downloaded file")
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package transfer
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
9+
"github.com/0gfoundation/0g-storage-client/core"
10+
"github.com/sirupsen/logrus"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// Every step of the download normalises the caller's root through common.HexToHash,
16+
// except the final validation, which compared it as text against Hash.Hex's canonical
17+
// form. So a root spelled in any other accepted way - uppercase digits, no 0x prefix,
18+
// fewer than 64 digits - completed its download and was then rejected for its spelling.
19+
func TestValidateDownloadFile_AcceptsEquivalentRootForms(t *testing.T) {
20+
path := filepath.Join(t.TempDir(), "data.dat")
21+
content := []byte("some downloaded content")
22+
require.NoError(t, os.WriteFile(path, content, 0644))
23+
24+
root, err := core.MerkleRoot(path)
25+
require.NoError(t, err)
26+
canonical := root.Hex()
27+
28+
downloader := &Downloader{logger: logrus.New()}
29+
30+
for _, form := range []struct {
31+
name string
32+
root string
33+
}{
34+
{"canonical", canonical},
35+
{"uppercase digits", "0x" + strings.ToUpper(strings.TrimPrefix(canonical, "0x"))},
36+
{"no 0x prefix", strings.TrimPrefix(canonical, "0x")},
37+
} {
38+
t.Run(form.name, func(t *testing.T) {
39+
assert.NoError(t, downloader.validateDownloadFile(form.root, path, int64(len(content))),
40+
"%q denotes the same hash as %q", form.root, canonical)
41+
})
42+
}
43+
}
44+
45+
// A genuinely different root must still be rejected, and the message should name both.
46+
func TestValidateDownloadFile_RejectsDifferentRoot(t *testing.T) {
47+
path := filepath.Join(t.TempDir(), "data.dat")
48+
content := []byte("some downloaded content")
49+
require.NoError(t, os.WriteFile(path, content, 0644))
50+
51+
downloader := &Downloader{logger: logrus.New()}
52+
err := downloader.validateDownloadFile(
53+
"0xbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbad0", path, int64(len(content)))
54+
55+
require.Error(t, err)
56+
assert.Contains(t, err.Error(), "Merkle root mismatch")
57+
assert.Contains(t, err.Error(), "expected", "the message should name the root that was asked for")
58+
}
59+
60+
func TestValidateDownloadFile_RejectsSizeMismatch(t *testing.T) {
61+
path := filepath.Join(t.TempDir(), "data.dat")
62+
require.NoError(t, os.WriteFile(path, []byte("short"), 0644))
63+
64+
root, err := core.MerkleRoot(path)
65+
require.NoError(t, err)
66+
67+
downloader := &Downloader{logger: logrus.New()}
68+
assert.ErrorContains(t, downloader.validateDownloadFile(root.Hex(), path, 9999), "File size mismatch")
69+
}

0 commit comments

Comments
 (0)