Integer type policy

Integers are for numbers, enabling arithmetic like subtractions and for
loops without getting shot in the foot. Unsigneds are for bitfields.

- "int" for numbers that will always be laughably smaller than four
  billion, and where we don't care about the serialization format.

- "int32" for numbers that will always be laughably smaller than four
  billion, and will be serialized to four bytes.

- "int64" for numbers that may approach four billion or will be
  serialized to eight bytes.

- "uint32" and "uint64" for bitfields, depending on required number of
  bits and serialization format. Likewise "uint8" and "uint16", although
  rare in this project since they don't exist in XDR.

- "int8", "int16" and plain "uint" are almost never useful.

internal/model/model.go

@@ -701,6 +701,10 @@ func (m *Model) Close(device protocol.DeviceID, err error) {
 // Request returns the specified data segment by reading it from local disk.
 // Implements the protocol.Model interface.
 func (m *Model) Request(deviceID protocol.DeviceID, folder, name string, offset int64, size int) ([]byte, error) {
+	if offset < 0 || size < 0 {
+		return nil, ErrNoSuchFile
+	}
+
 	if !m.folderSharedWith(folder, deviceID) {
 		l.Warnf("Request from %s for file %s in unshared folder %q", deviceID, name, folder)
 		return nil, ErrNoSuchFile
@@ -970,12 +974,12 @@ func sendIndexes(conn protocol.Connection, folder string, fs *db.FileSet, ignore
 	}
 }
 
-func sendIndexTo(initial bool, minLocalVer uint64, conn protocol.Connection, folder string, fs *db.FileSet, ignores *ignore.Matcher) (uint64, error) {
+func sendIndexTo(initial bool, minLocalVer int64, conn protocol.Connection, folder string, fs *db.FileSet, ignores *ignore.Matcher) (int64, error) {
 	deviceID := conn.ID()
 	name := conn.Name()
 	batch := make([]protocol.FileInfo, 0, indexBatchSize)
 	currentBatchSize := 0
-	maxLocalVer := uint64(0)
+	maxLocalVer := int64(0)
 	var err error
 
 	fs.WithHave(protocol.LocalDeviceID, func(fi db.FileIntf) bool {
@@ -1349,7 +1353,7 @@ func (m *Model) Override(folder string) {
 // CurrentLocalVersion returns the change version for the given folder.
 // This is guaranteed to increment if the contents of the local folder has
 // changed.
-func (m *Model) CurrentLocalVersion(folder string) uint64 {
+func (m *Model) CurrentLocalVersion(folder string) int64 {
 	m.fmut.RLock()
 	fs, ok := m.folderFiles[folder]
 	m.fmut.RUnlock()
@@ -1365,7 +1369,7 @@ func (m *Model) CurrentLocalVersion(folder string) uint64 {
 // RemoteLocalVersion returns the change version for the given folder, as
 // sent by remote peers. This is guaranteed to increment if the contents of
 // the remote or global folder has changed.
-func (m *Model) RemoteLocalVersion(folder string) uint64 {
+func (m *Model) RemoteLocalVersion(folder string) int64 {
 	m.fmut.RLock()
 	defer m.fmut.RUnlock()
 
@@ -1376,7 +1380,7 @@ func (m *Model) RemoteLocalVersion(folder string) uint64 {
 		return 0
 	}
 
-	var ver uint64
+	var ver int64
 	for _, n := range m.folderDevices[folder] {
 		ver += fs.LocalVersion(n)
 	}

internal/model/model_test.go

@@ -110,6 +110,33 @@ func TestRequest(t *testing.T) {
 	if bs != nil {
 		t.Errorf("Unexpected non nil data on insecure file read: %q", string(bs))
 	}
+
+	// Larger block than available
+	bs, err = m.Request(device1, "default", "foo", 0, 42)
+	if err == nil {
+		t.Error("Unexpected nil error on insecure file read")
+	}
+	if bs != nil {
+		t.Errorf("Unexpected non nil data on insecure file read: %q", string(bs))
+	}
+
+	// Negative offset
+	bs, err = m.Request(device1, "default", "foo", -4, 6)
+	if err == nil {
+		t.Error("Unexpected nil error on insecure file read")
+	}
+	if bs != nil {
+		t.Errorf("Unexpected non nil data on insecure file read: %q", string(bs))
+	}
+
+	// Negative size
+	bs, err = m.Request(device1, "default", "foo", 4, -4)
+	if err == nil {
+		t.Error("Unexpected nil error on insecure file read")
+	}
+	if bs != nil {
+		t.Errorf("Unexpected non nil data on insecure file read: %q", string(bs))
+	}
 }
 
 func genFiles(n int) []protocol.FileInfo {

internal/model/puller.go

@@ -100,7 +100,7 @@ func (p *Puller) Serve() {
 		p.model.setState(p.folder, FolderIdle)
 	}()
 
-	var prevVer uint64
+	var prevVer int64
 	var prevIgnoreHash string
 
 	// We don't start pulling files until a scan has been completed.
@@ -623,9 +623,9 @@ func (p *Puller) handleFile(file protocol.FileInfo, copyChan chan<- copyBlocksSt
 		folder:     p.folder,
 		tempName:   tempName,
 		realName:   realName,
-		copyTotal:  uint32(len(blocks)),
-		copyNeeded: uint32(len(blocks)),
-		reused:     uint32(reused),
+		copyTotal:  len(blocks),
+		copyNeeded: len(blocks),
+		reused:     reused,
 	}
 
 	if debug {
@@ -720,7 +720,7 @@ func (p *Puller) copierRoutine(in <-chan copyBlocksState, pullChan chan<- pullBl
 
 		for _, block := range state.blocks {
 			buf = buf[:int(block.Size)]
-			found := p.model.finder.Iterate(block.Hash, func(folder, file string, index uint32) bool {
+			found := p.model.finder.Iterate(block.Hash, func(folder, file string, index int32) bool {
 				path := filepath.Join(folderRoots[folder], file)
 
 				var fd *os.File

internal/model/puller_test.go

@@ -199,7 +199,7 @@ func TestCopierFinder(t *testing.T) {
 	// Update index
 	m.updateLocal("default", existingFile)
 
-	iterFn := func(folder, file string, index uint32) bool {
+	iterFn := func(folder, file string, index int32) bool {
 		return true
 	}
 
@@ -264,7 +264,7 @@ func TestCopierFinder(t *testing.T) {
 
 // Test that updating a file removes it's old blocks from the blockmap
 func TestCopierCleanup(t *testing.T) {
-	iterFn := func(folder, file string, index uint32) bool {
+	iterFn := func(folder, file string, index int32) bool {
 		return true
 	}
 
@@ -341,7 +341,7 @@ func TestLastResortPulling(t *testing.T) {
 	// with a different name (causing to copy that particular block)
 	file.Name = "newfile"
 
-	iterFn := func(folder, file string, index uint32) bool {
+	iterFn := func(folder, file string, index int32) bool {
 		return true
 	}
 

internal/model/sharedpullerstate.go

@@ -33,27 +33,27 @@ type sharedPullerState struct {
 	folder   string
 	tempName string
 	realName string
-	reused   uint32 // Number of blocks reused from temporary file
+	reused   int // Number of blocks reused from temporary file
 
 	// Mutable, must be locked for access
 	err        error      // The first error we hit
 	fd         *os.File   // The fd of the temp file
-	copyTotal  uint32     // Total number of copy actions for the whole job
-	pullTotal  uint32     // Total number of pull actions for the whole job
-	copyOrigin uint32     // Number of blocks copied from the original file
-	copyNeeded uint32     // Number of copy actions still pending
-	pullNeeded uint32     // Number of block pulls still pending
+	copyTotal  int        // Total number of copy actions for the whole job
+	pullTotal  int        // Total number of pull actions for the whole job
+	copyOrigin int        // Number of blocks copied from the original file
+	copyNeeded int        // Number of copy actions still pending
+	pullNeeded int        // Number of block pulls still pending
 	mut        sync.Mutex // Protects the above
 }
 
 // A momentary state representing the progress of the puller
 type pullerProgress struct {
-	Total               uint32
-	Reused              uint32
-	CopiedFromOrigin    uint32
-	CopiedFromElsewhere uint32
-	Pulled              uint32
-	Pulling             uint32
+	Total               int
+	Reused              int
+	CopiedFromOrigin    int
+	CopiedFromElsewhere int
+	Pulled              int
+	Pulling             int
 	BytesDone           int64
 	BytesTotal          int64
 }