Fix issue #96

In the native file-based storage, mark pieces incomplete if the necessary file data is missing, or there's a read error on a piece.

diff --git a/storage/file.go b/storage/file.go
index 257d92a3f6a3f2f39c9788327ac1a55cd22c5bdd..00ec0603512855ac929f6c14e98e75bd199f163a 100644 (file)
--- a/storage/file.go
+++ b/storage/file.go
@@ -26,6 +26,7 @@ func NewFile(baseDir string) Client {
 func (fs *fileStorage) OpenTorrent(info *metainfo.InfoEx) (Torrent, error) {
        return &fileTorrentStorage{
                fs,
+               &info.Info,
                pieceCompletionForDir(fs.baseDir),
        }, nil
 }
@@ -33,15 +34,13 @@ func (fs *fileStorage) OpenTorrent(info *metainfo.InfoEx) (Torrent, error) {
 // File-based torrent storage, not yet bound to a Torrent.
 type fileTorrentStorage struct {
        fs         *fileStorage
+       info       *metainfo.Info
        completion pieceCompletion
 }
 
 func (fts *fileTorrentStorage) Piece(p metainfo.Piece) Piece {
        // Create a view onto the file-based torrent storage.
-       _io := &fileStorageTorrent{
-               p.Info,
-               fts.fs.baseDir,
-       }
+       _io := fileStorageTorrent{fts}
        // Return the appropriate segments of this.
        return &fileStoragePiece{
                fts,
@@ -56,31 +55,14 @@ func (fs *fileTorrentStorage) Close() error {
        return nil
 }
 
-type fileStoragePiece struct {
-       *fileTorrentStorage
-       p metainfo.Piece
-       io.WriterAt
-       io.ReaderAt
-}
-
-func (fs *fileStoragePiece) GetIsComplete() bool {
-       return fs.completion.Get(fs.p)
-}
-
-func (fs *fileStoragePiece) MarkComplete() error {
-       fs.completion.Set(fs.p, true)
-       return nil
-}
-
 // Exposes file-based storage of a torrent, as one big ReadWriterAt.
 type fileStorageTorrent struct {
-       info    *metainfo.InfoEx
-       baseDir string
+       fts *fileTorrentStorage
 }
 
 // Returns EOF on short or missing file.
 func (fst *fileStorageTorrent) readFileAt(fi metainfo.FileInfo, b []byte, off int64) (n int, err error) {
-       f, err := os.Open(fst.fileInfoName(fi))
+       f, err := os.Open(fst.fts.fileInfoName(fi))
        if os.IsNotExist(err) {
                // File missing is treated the same as a short file.
                err = io.EOF
@@ -108,8 +90,8 @@ func (fst *fileStorageTorrent) readFileAt(fi metainfo.FileInfo, b []byte, off in
 }
 
 // Only returns EOF at the end of the torrent. Premature EOF is ErrUnexpectedEOF.
-func (fst *fileStorageTorrent) ReadAt(b []byte, off int64) (n int, err error) {
-       for _, fi := range fst.info.UpvertedFiles() {
+func (fst fileStorageTorrent) ReadAt(b []byte, off int64) (n int, err error) {
+       for _, fi := range fst.fts.info.UpvertedFiles() {
                for off < fi.Length {
                        n1, err1 := fst.readFileAt(fi, b, off)
                        n += n1
@@ -136,8 +118,8 @@ func (fst *fileStorageTorrent) ReadAt(b []byte, off int64) (n int, err error) {
        return
 }
 
-func (fst *fileStorageTorrent) WriteAt(p []byte, off int64) (n int, err error) {
-       for _, fi := range fst.info.UpvertedFiles() {
+func (fst fileStorageTorrent) WriteAt(p []byte, off int64) (n int, err error) {
+       for _, fi := range fst.fts.info.UpvertedFiles() {
                if off >= fi.Length {
                        off -= fi.Length
                        continue
@@ -146,7 +128,7 @@ func (fst *fileStorageTorrent) WriteAt(p []byte, off int64) (n int, err error) {
                if int64(n1) > fi.Length-off {
                        n1 = int(fi.Length - off)
                }
-               name := fst.fileInfoName(fi)
+               name := fst.fts.fileInfoName(fi)
                os.MkdirAll(filepath.Dir(name), 0770)
                var f *os.File
                f, err = os.OpenFile(name, os.O_WRONLY|os.O_CREATE, 0660)
@@ -168,6 +150,6 @@ func (fst *fileStorageTorrent) WriteAt(p []byte, off int64) (n int, err error) {
        return
 }
 
-func (fst *fileStorageTorrent) fileInfoName(fi metainfo.FileInfo) string {
-       return filepath.Join(append([]string{fst.baseDir, fst.info.Name}, fi.Path...)...)
+func (fts *fileTorrentStorage) fileInfoName(fi metainfo.FileInfo) string {
+       return filepath.Join(append([]string{fts.fs.baseDir, fts.info.Name}, fi.Path...)...)
 }

diff --git a/storage/file_misc.go b/storage/file_misc.go
new file mode 100644 (file)
index 0000000..674eda3
--- /dev/null
+++ b/storage/file_misc.go
@@ -0,0 +1,29 @@
+package storage
+
+import "github.com/anacrolix/torrent/metainfo"
+
+func extentCompleteRequiredLengths(info *metainfo.Info, off, n int64) (ret []metainfo.FileInfo) {
+       if n == 0 {
+               return
+       }
+       for _, fi := range info.UpvertedFiles() {
+               if off >= fi.Length {
+                       off -= fi.Length
+                       continue
+               }
+               n1 := n
+               if off+n1 > fi.Length {
+                       n1 = fi.Length - off
+               }
+               ret = append(ret, metainfo.FileInfo{
+                       Path:   fi.Path,
+                       Length: off + n1,
+               })
+               n -= n1
+               if n == 0 {
+                       return
+               }
+               off = 0
+       }
+       panic("extent exceeds torrent bounds")
+}

diff --git a/storage/file_misc_test.go b/storage/file_misc_test.go
new file mode 100644 (file)
index 0000000..d95d024
--- /dev/null
+++ b/storage/file_misc_test.go
@@ -0,0 +1,40 @@
+package storage
+
+import (
+       "testing"
+
+       "github.com/stretchr/testify/assert"
+
+       "github.com/anacrolix/torrent/metainfo"
+)
+
+func TestExtentCompleteRequiredLengths(t *testing.T) {
+       info := &metainfo.InfoEx{
+               Info: metainfo.Info{
+                       Files: []metainfo.FileInfo{
+                               {Path: []string{"a"}, Length: 2},
+                               {Path: []string{"b"}, Length: 3},
+                       },
+               },
+       }
+       assert.Empty(t, extentCompleteRequiredLengths(&info.Info, 0, 0))
+       assert.EqualValues(t, []metainfo.FileInfo{
+               {Path: []string{"a"}, Length: 1},
+       }, extentCompleteRequiredLengths(&info.Info, 0, 1))
+       assert.EqualValues(t, []metainfo.FileInfo{
+               {Path: []string{"a"}, Length: 2},
+       }, extentCompleteRequiredLengths(&info.Info, 0, 2))
+       assert.EqualValues(t, []metainfo.FileInfo{
+               {Path: []string{"a"}, Length: 2},
+               {Path: []string{"b"}, Length: 1},
+       }, extentCompleteRequiredLengths(&info.Info, 0, 3))
+       assert.EqualValues(t, []metainfo.FileInfo{
+               {Path: []string{"b"}, Length: 2},
+       }, extentCompleteRequiredLengths(&info.Info, 2, 2))
+       assert.EqualValues(t, []metainfo.FileInfo{
+               {Path: []string{"b"}, Length: 3},
+       }, extentCompleteRequiredLengths(&info.Info, 4, 1))
+       assert.Len(t, extentCompleteRequiredLengths(&info.Info, 5, 0), 0)
+       assert.Panics(t, func() { extentCompleteRequiredLengths(&info.Info, 6, 1) })
+
+}

diff --git a/storage/file_storage_piece.go b/storage/file_storage_piece.go
new file mode 100644 (file)
index 0000000..fec993a
--- /dev/null
+++ b/storage/file_storage_piece.go
@@ -0,0 +1,55 @@
+package storage
+
+import (
+       "io"
+       "os"
+
+       "github.com/anacrolix/torrent/metainfo"
+)
+
+type fileStoragePiece struct {
+       *fileTorrentStorage
+       p metainfo.Piece
+       io.WriterAt
+       r io.ReaderAt
+}
+
+func (fs *fileStoragePiece) GetIsComplete() (ret bool) {
+       ret = fs.completion.Get(fs.p)
+       if !ret {
+               return
+       }
+       // If it's allegedly complete, check that its constituent files have the
+       // necessary length.
+       for _, fi := range extentCompleteRequiredLengths(&fs.p.Info.Info, fs.p.Offset(), fs.p.Length()) {
+               s, err := os.Stat(fs.fileInfoName(fi))
+               if err != nil || s.Size() < fi.Length {
+                       ret = false
+                       break
+               }
+       }
+       if ret {
+               return
+       }
+       // The completion was wrong, fix it.
+       fs.completion.Set(fs.p, false)
+       return
+}
+
+func (fs *fileStoragePiece) MarkComplete() error {
+       fs.completion.Set(fs.p, true)
+       return nil
+}
+
+func (fsp *fileStoragePiece) ReadAt(b []byte, off int64) (n int, err error) {
+       n, err = fsp.r.ReadAt(b, off)
+       if n != 0 {
+               err = nil
+               return
+       }
+       if off < 0 || off >= fsp.p.Length() {
+               return
+       }
+       fsp.completion.Set(fsp.p, false)
+       return
+}

diff --git a/storage/issue96_test.go b/storage/issue96_test.go
new file mode 100644 (file)
index 0000000..b3bc4d9
--- /dev/null
+++ b/storage/issue96_test.go
@@ -0,0 +1,37 @@
+package storage
+
+import (
+       "io/ioutil"
+       "os"
+       "testing"
+
+       "github.com/stretchr/testify/require"
+
+       "github.com/anacrolix/torrent/metainfo"
+)
+
+func testMarkedCompleteMissingOnRead(t *testing.T, csf func(string) Client) {
+       td, err := ioutil.TempDir("", "")
+       require.NoError(t, err)
+       defer os.RemoveAll(td)
+       cs := csf(td)
+       info := &metainfo.InfoEx{
+               Info: metainfo.Info{
+                       PieceLength: 1,
+                       Files:       []metainfo.FileInfo{{Path: []string{"a"}, Length: 1}},
+               },
+       }
+       ts, err := cs.OpenTorrent(info)
+       require.NoError(t, err)
+       p := ts.Piece(info.Piece(0))
+       require.NoError(t, p.MarkComplete())
+       require.False(t, p.GetIsComplete())
+       n, err := p.ReadAt(make([]byte, 1), 0)
+       require.Error(t, err)
+       require.EqualValues(t, 0, n)
+       require.False(t, p.GetIsComplete())
+}
+
+func TestMarkedCompleteMissingOnReadFile(t *testing.T) {
+       testMarkedCompleteMissingOnRead(t, NewFile)
+}