cmd/strelaysrv: Correct go get command in README

Skip-check: authors

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3564

build.go

@@ -642,7 +642,9 @@ func ldflags() string {
 
 func rmr(paths ...string) {
 	for _, path := range paths {
-		log.Println("rm -r", path)
+		if debug {
+			log.Println("rm -r", path)
+		}
 		os.RemoveAll(path)
 	}
 }

cmd/strelaysrv/README.md

@@ -5,7 +5,7 @@ strelaysrv
 
 This is the relay server for the `syncthing` project.
 
-To get it, run `go get github.com/syncthing/strelaysrv` or download the
+To get it, run `go get github.com/syncthing/syncthing/cmd/strelaysrv` or download the
 [latest build](http://build.syncthing.net/job/strelaysrv/lastSuccessfulBuild/artifact/)
 from the build server.
 

cmd/syncthing/gui.go

@@ -71,7 +71,7 @@ type modelIntf interface {
 	Completion(device protocol.DeviceID, folder string) model.FolderCompletion
 	Override(folder string)
 	NeedFolderFiles(folder string, page, perpage int) ([]db.FileInfoTruncated, []db.FileInfoTruncated, []db.FileInfoTruncated, int)
-	NeedSize(folder string) (nfiles int, bytes int64)
+	NeedSize(folder string) (nfiles, ndeletes int, bytes int64)
 	ConnectionStats() map[string]interface{}
 	DeviceStatistics() map[string]stats.DeviceStatistics
 	FolderStatistics() map[string]stats.FolderStatistics
@@ -313,6 +313,11 @@ func (s *apiService) Serve() {
 	// Add the CORS handling
 	handler = corsMiddleware(handler)
 
+	if addressIsLocalhost(guiCfg.Address()) && !guiCfg.InsecureSkipHostCheck {
+		// Verify source host
+		handler = localhostMiddleware(handler)
+	}
+
 	handler = debugMiddleware(handler)
 
 	srv := http.Server{
@@ -495,6 +500,17 @@ func withDetailsMiddleware(id protocol.DeviceID, h http.Handler) http.Handler {
 	})
 }
 
+func localhostMiddleware(h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if addressIsLocalhost(r.Host) {
+			h.ServeHTTP(w, r)
+			return
+		}
+
+		http.Error(w, "Host check error", http.StatusForbidden)
+	})
+}
+
 func (s *apiService) whenDebugging(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if s.cfg.GUI().Debugging {
@@ -503,7 +519,6 @@ func (s *apiService) whenDebugging(h http.Handler) http.Handler {
 		}
 
 		http.Error(w, "Debugging disabled", http.StatusBadRequest)
-		return
 	})
 }
 
@@ -588,6 +603,7 @@ func (s *apiService) getDBCompletion(w http.ResponseWriter, r *http.Request) {
 		"completion":  comp.CompletionPct,
 		"needBytes":   comp.NeedBytes,
 		"globalBytes": comp.GlobalBytes,
+		"needDeletes": comp.NeedDeletes,
 	})
 }
 
@@ -608,8 +624,8 @@ func folderSummary(cfg configIntf, m modelIntf, folder string) map[string]interf
 	localFiles, localDeleted, localBytes := m.LocalSize(folder)
 	res["localFiles"], res["localDeleted"], res["localBytes"] = localFiles, localDeleted, localBytes
 
-	needFiles, needBytes := m.NeedSize(folder)
-	res["needFiles"], res["needBytes"] = needFiles, needBytes
+	needFiles, needDeletes, needBytes := m.NeedSize(folder)
+	res["needFiles"], res["needDeletes"], res["needBytes"] = needFiles, needDeletes, needBytes
 
 	res["inSyncFiles"], res["inSyncBytes"] = globalFiles-needFiles, globalBytes-needBytes
 
@@ -1291,3 +1307,17 @@ func dirNames(dir string) []string {
 	sort.Strings(dirs)
 	return dirs
 }
+
+func addressIsLocalhost(addr string) bool {
+	host, _, err := net.SplitHostPort(addr)
+	if err != nil {
+		// There was no port, so we assume the address was just a hostname
+		host = addr
+	}
+	switch host {
+	case "127.0.0.1", "::1", "localhost":
+		return true
+	default:
+		return false
+	}
+}

cmd/syncthing/gui_test.go

@@ -16,6 +16,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -460,7 +461,6 @@ func TestHTTPLogin(t *testing.T) {
 	if resp.StatusCode != http.StatusOK {
 		t.Errorf("Unexpected non-200 return code %d for authed request (ISO-8859-1)", resp.StatusCode)
 	}
-
 }
 
 func startHTTP(cfg *mockedConfig) (string, error) {
@@ -491,7 +491,12 @@ func startHTTP(cfg *mockedConfig) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("Weird address from API service: %v", err)
 	}
-	baseURL := fmt.Sprintf("http://127.0.0.1:%d", tcpAddr.Port)
+
+	host, _, _ := net.SplitHostPort(cfg.gui.RawAddress)
+	if host == "" || host == "0.0.0.0" {
+		host = "127.0.0.1"
+	}
+	baseURL := fmt.Sprintf("http://%s", net.JoinHostPort(host, strconv.Itoa(tcpAddr.Port)))
 
 	return baseURL, nil
 }
@@ -666,3 +671,189 @@ func testConfigPost(data io.Reader) (*http.Response, error) {
 	req.Header.Set("X-API-Key", testAPIKey)
 	return cli.Do(req)
 }
+
+func TestHostCheck(t *testing.T) {
+	// An API service bound to localhost should reject non-localhost host Headers
+
+	cfg := new(mockedConfig)
+	cfg.gui.RawAddress = "127.0.0.1:0"
+	baseURL, err := startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A normal HTTP get to the localhost-bound service should succeed
+
+	resp, err := http.Get(baseURL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Regular HTTP get: expected 200 OK, not", resp.Status)
+	}
+
+	// A request with a suspicious Host header should fail
+
+	req, _ := http.NewRequest("GET", baseURL, nil)
+	req.Host = "example.com"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusForbidden {
+		t.Error("Suspicious Host header: expected 403 Forbidden, not", resp.Status)
+	}
+
+	// A request with an explicit "localhost:8384" Host header should pass
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "localhost:8384"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Explicit localhost:8384: expected 200 OK, not", resp.Status)
+	}
+
+	// A request with an explicit "localhost" Host header (no port) should pass
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "localhost"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Explicit localhost: expected 200 OK, not", resp.Status)
+	}
+
+	// A server with InsecureSkipHostCheck set behaves differently
+
+	cfg = new(mockedConfig)
+	cfg.gui.RawAddress = "127.0.0.1:0"
+	cfg.gui.InsecureSkipHostCheck = true
+	baseURL, err = startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A request with a suspicious Host header should be allowed
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "example.com"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Incorrect host header, check disabled: expected 200 OK, not", resp.Status)
+	}
+
+	// A server bound to a wildcard address also doesn't do the check
+
+	cfg = new(mockedConfig)
+	cfg.gui.RawAddress = "0.0.0.0:0"
+	cfg.gui.InsecureSkipHostCheck = true
+	baseURL, err = startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A request with a suspicious Host header should be allowed
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "example.com"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Incorrect host header, wildcard bound: expected 200 OK, not", resp.Status)
+	}
+
+	// This should all work over IPv6 as well
+
+	cfg = new(mockedConfig)
+	cfg.gui.RawAddress = "[::1]:0"
+	baseURL, err = startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A normal HTTP get to the localhost-bound service should succeed
+
+	resp, err = http.Get(baseURL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Regular HTTP get (IPv6): expected 200 OK, not", resp.Status)
+	}
+
+	// A request with a suspicious Host header should fail
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "example.com"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusForbidden {
+		t.Error("Suspicious Host header (IPv6): expected 403 Forbidden, not", resp.Status)
+	}
+
+	// A request with an explicit "localhost:8384" Host header should pass
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "localhost:8384"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Explicit localhost:8384 (IPv6): expected 200 OK, not", resp.Status)
+	}
+}
+
+func TestAddressIsLocalhost(t *testing.T) {
+	testcases := []struct {
+		address string
+		result  bool
+	}{
+		// These are all valid localhost addresses
+		{"localhost", true},
+		{"::1", true},
+		{"127.0.0.1", true},
+		{"localhost:8080", true},
+		{"[::1]:8080", true},
+		{"127.0.0.1:8080", true},
+
+		// These are all non-localhost addresses
+		{"example.com", false},
+		{"example.com:8080", false},
+		{"192.0.2.10", false},
+		{"192.0.2.10:8080", false},
+		{"0.0.0.0", false},
+		{"0.0.0.0:8080", false},
+		{"::", false},
+		{"[::]:8080", false},
+		{":8080", false},
+	}
+
+	for _, tc := range testcases {
+		result := addressIsLocalhost(tc.address)
+		if result != tc.result {
+			t.Errorf("addressIsLocalhost(%q)=%v, expected %v", tc.address, result, tc.result)
+		}
+	}
+}

cmd/syncthing/main.go

@@ -889,19 +889,32 @@ func loadOrCreateConfig() *config.Wrapper {
 }
 
 func archiveAndSaveConfig(cfg *config.Wrapper) error {
-	// To prevent previous config from being cleaned up, quickly touch it too
-	now := time.Now()
-	_ = os.Chtimes(cfg.ConfigPath(), now, now) // May return error on Android etc; no worries
-
+	// Copy the existing config to an archive copy
 	archivePath := cfg.ConfigPath() + fmt.Sprintf(".v%d", cfg.Raw().OriginalVersion)
 	l.Infoln("Archiving a copy of old config file format at:", archivePath)
-	if err := osutil.Rename(cfg.ConfigPath(), archivePath); err != nil {
+	if err := copyFile(cfg.ConfigPath(), archivePath); err != nil {
 		return err
 	}
 
+	// Do a regular atomic config sve
 	return cfg.Save()
 }
 
+func copyFile(src, dst string) error {
+	bs, err := ioutil.ReadFile(src)
+	if err != nil {
+		return err
+	}
+
+	if err := ioutil.WriteFile(dst, bs, 0600); err != nil {
+		// Attempt to clean up
+		os.Remove(dst)
+		return err
+	}
+
+	return nil
+}
+
 func startAuditing(mainService *suture.Supervisor) {
 	auditFile := timestampedLoc(locAuditLog)
 	fd, err := os.OpenFile(auditFile, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600)

cmd/syncthing/mocked_model_test.go

@@ -31,8 +31,8 @@ func (m *mockedModel) NeedFolderFiles(folder string, page, perpage int) ([]db.Fi
 	return nil, nil, nil, 0
 }
 
-func (m *mockedModel) NeedSize(folder string) (nfiles int, bytes int64) {
-	return 0, 0
+func (m *mockedModel) NeedSize(folder string) (nfiles, ndeletes int, bytes int64) {
+	return 0, 0, 0
 }
 
 func (m *mockedModel) ConnectionStats() map[string]interface{} {

gui/default/syncthing/core/syncthingController.js

@@ -439,13 +439,14 @@ angular.module('syncthing.core')
         }
 
         function recalcCompletion(device) {
-            var total = 0, needed = 0;
+            var total = 0, needed = 0, deletes = 0;
             for (var folder in $scope.completion[device]) {
                 if (folder === "_total") {
                     continue;
                 }
                 total += $scope.completion[device][folder].globalBytes;
                 needed += $scope.completion[device][folder].needBytes;
+                deletes += $scope.completion[device][folder].needDeletes;
             }
             if (total == 0) {
                 $scope.completion[device]._total = 100;
@@ -453,6 +454,13 @@ angular.module('syncthing.core')
                 $scope.completion[device]._total = 100 * (1 - needed / total);
             }
 
+            if (needed == 0 && deletes > 0) {
+                // We don't need any data, but we have deletes that we need
+                // to do. Drop down the completion percentage to indicate
+                // that we have stuff to do.
+                $scope.completion[device]._total = 95;
+            }
+
             console.log("recalcCompletion", device, $scope.completion[device]);
         }
 

jenkins/build-linux.bash

@@ -0,0 +1,60 @@
+#!/bin/bash
+set -euo pipefail
+
+# Copyright (C) 2016 The Syncthing Authors.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# You can obtain one at http://mozilla.org/MPL/2.0/.
+
+# This script should be run by Jenkins as './src/github.com/syncthing/syncthing/jenkins/build-linux.bash',
+# that is, it should be run from $GOPATH.
+
+. src/github.com/syncthing/syncthing/jenkins/common.bash
+
+init
+
+# after init we are in the source directory
+
+clean
+fetchExtra
+buildSource
+build
+test
+testWithCoverage
+
+platforms=(
+	dragonfly-amd64
+	freebsd-amd64 freebsd-386
+	linux-amd64 linux-386 linux-arm linux-arm64 linux-ppc64 linux-ppc64le
+	netbsd-amd64 netbsd-386
+	openbsd-amd64 openbsd-386
+	solaris-amd64
+)
+
+echo Building
+for plat in "${platforms[@]}"; do
+	echo Building "$plat"
+
+	goos="${plat%-*}"
+	goarch="${plat#*-}"
+	go run build.go -goos "$goos" -goarch "$goarch" tar
+	mv *.tar.gz "$WORKSPACE"
+	echo
+done
+
+go run build.go -goarch amd64 deb
+fakeroot sh -c 'chown -R root:root deb ; dpkg-deb -b deb .'
+mv *.deb "$WORKSPACE"
+
+go run build.go -goarch i386 deb
+fakeroot sh -c 'chown -R root:root deb ; dpkg-deb -b deb .'
+mv *.deb "$WORKSPACE"
+
+go run build.go -goarch armel deb
+fakeroot sh -c 'chown -R root:root deb ; dpkg-deb -b deb .'
+mv *.deb "$WORKSPACE"
+
+go run build.go -goarch armhf deb
+fakeroot sh -c 'chown -R root:root deb ; dpkg-deb -b deb .'
+mv *.deb "$WORKSPACE"

jenkins/build-macos.bash

@@ -0,0 +1,37 @@
+#!/bin/bash
+set -euo pipefail
+
+# Copyright (C) 2016 The Syncthing Authors.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# You can obtain one at http://mozilla.org/MPL/2.0/.
+
+# This script should be run by Jenkins as './src/github.com/syncthing/syncthing/jenkins/build-macos.bash',
+# that is, it should be run from $GOPATH.
+
+. src/github.com/syncthing/syncthing/jenkins/common.bash
+
+init
+
+# after init we are in the source directory
+
+clean
+fetchExtra
+build
+test
+
+platforms=(
+	darwin-amd64 darwin-386
+)
+
+echo Building
+for plat in "${platforms[@]}"; do
+	echo Building "$plat"
+
+	goos="${plat%-*}"
+	goarch="${plat#*-}"
+	go run build.go -goos "$goos" -goarch "$goarch" tar
+	mv *.tar.gz "$WORKSPACE"
+	echo
+done

jenkins/build-windows.bat

@@ -0,0 +1,54 @@
+@echo off
+
+rem Copyright (C) 2016 The Syncthing Authors.
+rem
+rem This Source Code Form is subject to the terms of the Mozilla Public
+rem License, v. 2.0. If a copy of the MPL was not distributed with this file,
+rem You can obtain one at http://mozilla.org/MPL/2.0/.
+
+rem This batch file should be run from the GOPATH.
+rem It expects to run on amd64, for windows-amd64 Go to be installed in C:\go
+rem and for windows-386 Go to be installed in C:\go-386.
+
+rem cURL should be installed in C:\Program Files\cURL.
+
+set ORIGPATH="C:\Program Files\cURL\bin";%PATH%
+set PATH=c:\go\bin;%ORIGPATH%
+set GOROOT=c:\go
+
+cd >gopath
+set /p GOPATH= <gopath
+
+cd src\github.com\syncthing\syncthing
+
+echo Initializing ^& cleaning
+go version
+git clean -fxd || goto error
+echo.
+
+echo Fetching extras
+mkdir extra
+curl -s -L -o extra/Getting-Started.pdf https://docs.syncthing.net/pdf/Getting-Started.pdf || goto :error
+curl -s -L -o extra/FAQ.pdf https://docs.syncthing.net/pdf/FAQ.pdf || goto :error
+echo.
+
+echo Testing
+go run build.go test || goto :error
+echo.
+
+echo Building (amd64)
+go run build.go zip || goto :error
+echo.
+
+set PATH=c:\go-386\bin;%ORIGPATH%
+set GOROOT=c:\go-386
+
+echo building (386)
+go run build.go zip || goto :error
+echo.
+
+goto :EOF
+
+:error
+echo code #%errorlevel%.
+exit /b %errorlevel%

jenkins/common.bash

@@ -0,0 +1,85 @@
+#!/bin/bash
+set -euo pipefail
+
+# Copyright (C) 2016 The Syncthing Authors.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# You can obtain one at http://mozilla.org/MPL/2.0/.
+
+ulimit -t 600 || true
+ulimit -d 1024000 || true
+ulimit -m 1024000 || true
+
+export CGO_ENABLED=0
+export GO386=387
+export GOARM=5
+
+function init {
+    echo Initializing
+    export GOPATH=$(pwd)
+    export WORKSPACE="${WORKSPACE:-$GOPATH}"
+    go version
+    rm -f *.tar.gz *.zip *.deb
+    cd src/github.com/syncthing/syncthing
+
+    version=$(git describe)
+    echo "Building $version"
+    echo
+}
+
+function clean {
+    echo Cleaning
+    rm -rf "$GOPATH/pkg"
+    git clean -fxd
+    git fetch --prune
+    echo
+}
+
+function fetchExtra {
+    echo Fetching extra resources
+    mkdir extra
+    curl -s -o extra/Getting-Started.pdf http://docs.syncthing.net/pdf/Getting-Started.pdf
+    curl -s -o extra/FAQ.pdf http://docs.syncthing.net/pdf/FAQ.pdf
+    echo
+}
+
+function checkAuthorsCopyright {
+    echo Basic metadata checks
+    go run script/check-authors.go
+    go run script/check-copyright.go lib/ cmd/ script/
+    echo
+}
+
+function build {
+    echo Build
+    go run build.go
+    echo
+}
+
+function test {
+    echo Test with race
+    CGO_ENABLED=1 go run build.go -race test
+    echo
+}
+
+function testWithCoverage {
+    echo Test with coverage
+    CGO_ENABLED=1 ./build.sh test-cov
+
+    notCovered=$(egrep -c '\s0$' coverage.out)
+    total=$(wc -l coverage.out | awk '{print $1}')
+    coverPct=$(awk "BEGIN{print (1 - $notCovered / $total) * 100}")
+    echo "$coverPct" > "coverage.txt"
+    echo "Test coverage is $coverPct%%"
+    echo
+}
+
+function buildSource {
+    echo Archiving source
+    echo "$version" > RELEASE
+    pushd .. >/dev/null
+    tar c -z -f "$WORKSPACE/syncthing-source-$version.tar.gz" --exclude .git syncthing
+    popd >/dev/null
+    echo
+}

lib/config/guiconfiguration.go

@@ -13,15 +13,16 @@ import (
 )
 
 type GUIConfiguration struct {
-	Enabled             bool   `xml:"enabled,attr" json:"enabled" default:"true"`
-	RawAddress          string `xml:"address" json:"address" default:"127.0.0.1:8384"`
-	User                string `xml:"user,omitempty" json:"user"`
-	Password            string `xml:"password,omitempty" json:"password"`
-	RawUseTLS           bool   `xml:"tls,attr" json:"useTLS"`
-	APIKey              string `xml:"apikey,omitempty" json:"apiKey"`
-	InsecureAdminAccess bool   `xml:"insecureAdminAccess,omitempty" json:"insecureAdminAccess"`
-	Theme               string `xml:"theme" json:"theme" default:"default"`
-	Debugging           bool   `xml:"debugging,attr" json:"debugging"`
+	Enabled               bool   `xml:"enabled,attr" json:"enabled" default:"true"`
+	RawAddress            string `xml:"address" json:"address" default:"127.0.0.1:8384"`
+	User                  string `xml:"user,omitempty" json:"user"`
+	Password              string `xml:"password,omitempty" json:"password"`
+	RawUseTLS             bool   `xml:"tls,attr" json:"useTLS"`
+	APIKey                string `xml:"apikey,omitempty" json:"apiKey"`
+	InsecureAdminAccess   bool   `xml:"insecureAdminAccess,omitempty" json:"insecureAdminAccess"`
+	Theme                 string `xml:"theme" json:"theme" default:"default"`
+	Debugging             bool   `xml:"debugging,attr" json:"debugging"`
+	InsecureSkipHostCheck bool   `xml:"insecureSkipHostcheck,omitempty" json:"insecureSkipHostcheck"`
 }
 
 func (c GUIConfiguration) Address() string {

lib/db/structs.go

@@ -47,8 +47,11 @@ func (f FileInfoTruncated) HasPermissionBits() bool {
 }
 
 func (f FileInfoTruncated) FileSize() int64 {
-	if f.IsDirectory() || f.IsDeleted() {
-		return 128
+	if f.Deleted {
+		return 0
+	}
+	if f.IsDirectory() {
+		return protocol.SyntheticDirectorySize
 	}
 	return f.Size
 }

lib/model/model.go

@@ -463,6 +463,7 @@ type FolderCompletion struct {
 	CompletionPct float64
 	NeedBytes     int64
 	GlobalBytes   int64
+	NeedDeletes   int64
 }
 
 // Completion returns the completion status, in percent, for the given device
@@ -487,14 +488,20 @@ func (m *Model) Completion(device protocol.DeviceID, folder string) FolderComple
 	counts := m.deviceDownloads[device].GetBlockCounts(folder)
 	m.pmut.RUnlock()
 
-	var need, fileNeed, downloaded int64
+	var need, fileNeed, downloaded, deletes int64
 	rf.WithNeedTruncated(device, func(f db.FileIntf) bool {
 		ft := f.(db.FileInfoTruncated)
 
+		// If the file is deleted, we account it only in the deleted column.
+		if ft.Deleted {
+			deletes++
+			return true
+		}
+
 		// This might might be more than it really is, because some blocks can be of a smaller size.
 		downloaded = int64(counts[ft.Name] * protocol.BlockSize)
 
-		fileNeed = ft.Size - downloaded
+		fileNeed = ft.FileSize() - downloaded
 		if fileNeed < 0 {
 			fileNeed = 0
 		}
@@ -505,12 +512,22 @@ func (m *Model) Completion(device protocol.DeviceID, folder string) FolderComple
 
 	needRatio := float64(need) / float64(tot)
 	completionPct := 100 * (1 - needRatio)
+
+	// If the completion is 100% but there are deletes we need to handle,
+	// drop it down a notch. Hack for consumers that look only at the
+	// percentage (our own GUI does the same calculation as here on it's own
+	// and needs the same fixup).
+	if need == 0 && deletes > 0 {
+		completionPct = 95 // chosen by fair dice roll
+	}
+
 	l.Debugf("%v Completion(%s, %q): %f (%d / %d = %f)", m, device, folder, completionPct, need, tot, needRatio)
 
 	return FolderCompletion{
 		CompletionPct: completionPct,
 		NeedBytes:     need,
 		GlobalBytes:   tot,
+		NeedDeletes:   deletes,
 	}
 }
 
@@ -547,7 +564,7 @@ func (m *Model) LocalSize(folder string) (nfiles, deleted int, bytes int64) {
 }
 
 // NeedSize returns the number and total size of currently needed files.
-func (m *Model) NeedSize(folder string) (nfiles int, bytes int64) {
+func (m *Model) NeedSize(folder string) (nfiles, ndeletes int, bytes int64) {
 	m.fmut.RLock()
 	defer m.fmut.RUnlock()
 	if rf, ok := m.folderFiles[folder]; ok {
@@ -559,7 +576,8 @@ func (m *Model) NeedSize(folder string) (nfiles int, bytes int64) {
 			}
 
 			fs, de, by := sizeOfFile(f)
-			nfiles += fs + de
+			nfiles += fs
+			ndeletes += de
 			bytes += by
 			return true
 		})
@@ -1717,41 +1735,46 @@ func (m *Model) internalScanFolderSubdirs(folder string, subDirs []string) error
 		subDirs = []string{""}
 	}
 
-	// Do a scan of the database for each prefix, to check for deleted files.
+	// Do a scan of the database for each prefix, to check for deleted and
+	// ignored files.
 	batch = batch[:0]
 	for _, sub := range subDirs {
 		var iterError error
 
 		fs.WithPrefixedHaveTruncated(protocol.LocalDeviceID, sub, func(fi db.FileIntf) bool {
 			f := fi.(db.FileInfoTruncated)
-			if !f.IsDeleted() {
-				if len(batch) == batchSizeFiles {
-					if err := m.CheckFolderHealth(folder); err != nil {
-						iterError = err
-						return false
-					}
-					m.updateLocalsFromScanning(folder, batch)
-					batch = batch[:0]
+			if len(batch) == batchSizeFiles {
+				if err := m.CheckFolderHealth(folder); err != nil {
+					iterError = err
+					return false
 				}
+				m.updateLocalsFromScanning(folder, batch)
+				batch = batch[:0]
+			}
 
-				if !f.IsInvalid() && (ignores.Match(f.Name).IsIgnored() || symlinkInvalid(folder, f)) {
-					// File has been ignored or an unsupported symlink. Set invalid bit.
-					l.Debugln("setting invalid bit on ignored", f)
-					nf := protocol.FileInfo{
-						Name:          f.Name,
-						Type:          f.Type,
-						Size:          f.Size,
-						ModifiedS:     f.ModifiedS,
-						ModifiedNs:    f.ModifiedNs,
-						Permissions:   f.Permissions,
-						NoPermissions: f.NoPermissions,
-						Invalid:       true,
-						Version:       f.Version, // The file is still the same, so don't bump version
-					}
-					batch = append(batch, nf)
-				} else if _, err := mtimefs.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil {
-					// File has been deleted.
+			switch {
+			case !f.IsInvalid() && (ignores.Match(f.Name).IsIgnored() || symlinkInvalid(folder, f)):
+				// File was valid at last pass but has been ignored or is an
+				// unsupported symlink. Set invalid bit.
+				l.Debugln("setting invalid bit on ignored", f)
+				nf := protocol.FileInfo{
+					Name:          f.Name,
+					Type:          f.Type,
+					Size:          f.Size,
+					ModifiedS:     f.ModifiedS,
+					ModifiedNs:    f.ModifiedNs,
+					Permissions:   f.Permissions,
+					NoPermissions: f.NoPermissions,
+					Invalid:       true,
+					Version:       f.Version, // The file is still the same, so don't bump version
+				}
+				batch = append(batch, nf)
 
+			case !f.IsInvalid() && !f.IsDeleted():
+				// The file is valid and not deleted. Lets check if it's
+				// still here.
+
+				if _, err := mtimefs.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil {
 					// We don't specifically verify that the error is
 					// os.IsNotExist because there is a corner case when a
 					// directory is suddenly transformed into a file. When that
@@ -1762,7 +1785,7 @@ func (m *Model) internalScanFolderSubdirs(folder string, subDirs []string) error
 					nf := protocol.FileInfo{
 						Name:       f.Name,
 						Type:       f.Type,
-						Size:       f.Size,
+						Size:       0,
 						ModifiedS:  f.ModifiedS,
 						ModifiedNs: f.ModifiedNs,
 						Deleted:    true,
@@ -1927,6 +1950,7 @@ func (m *Model) Override(folder string) {
 			need.Deleted = true
 			need.Blocks = nil
 			need.Version = need.Version.Update(m.shortID)
+			need.Size = 0
 		} else {
 			// We have the file, replace with our version
 			have.Version = have.Version.Merge(need.Version).Update(m.shortID)

lib/model/model_test.go

@@ -93,6 +93,7 @@ func TestRequest(t *testing.T) {
 	m.AddFolder(defaultFolderConfig)
 	m.StartFolder("default")
 	m.ServeBackground()
+	defer m.Stop()
 	m.ScanFolder("default")
 
 	bs := make([]byte, protocol.BlockSize)
@@ -168,6 +169,7 @@ func benchmarkIndex(b *testing.B, nfiles int) {
 	m.AddFolder(defaultFolderConfig)
 	m.StartFolder("default")
 	m.ServeBackground()
+	defer m.Stop()
 
 	files := genFiles(nfiles)
 	m.Index(device1, "default", files)
@@ -197,6 +199,7 @@ func benchmarkIndexUpdate(b *testing.B, nfiles, nufiles int) {
 	m.AddFolder(defaultFolderConfig)
 	m.StartFolder("default")
 	m.ServeBackground()
+	defer m.Stop()
 
 	files := genFiles(nfiles)
 	ufiles := genFiles(nufiles)
@@ -278,6 +281,7 @@ func BenchmarkRequest(b *testing.B) {
 	m := NewModel(defaultConfig, protocol.LocalDeviceID, "device", "syncthing", "dev", db, nil)
 	m.AddFolder(defaultFolderConfig)
 	m.ServeBackground()
+	defer m.Stop()
 	m.ScanFolder("default")
 
 	const n = 1000
@@ -346,6 +350,7 @@ func TestDeviceRename(t *testing.T) {
 	m.AddConnection(conn, hello)
 
 	m.ServeBackground()
+	defer m.Stop()
 
 	if cfg.Devices()[device1].Name != "" {
 		t.Errorf("Device already has a name")
@@ -424,6 +429,7 @@ func TestClusterConfig(t *testing.T) {
 	m.AddFolder(cfg.Folders[0])
 	m.AddFolder(cfg.Folders[1])
 	m.ServeBackground()
+	defer m.Stop()
 
 	cm := m.generateClusterConfig(device2)
 
@@ -495,6 +501,7 @@ func TestIgnores(t *testing.T) {
 	m.AddFolder(defaultFolderConfig)
 	m.StartFolder("default")
 	m.ServeBackground()
+	defer m.Stop()
 
 	expected := []string{
 		".*",
@@ -590,6 +597,7 @@ func TestROScanRecovery(t *testing.T) {
 	m.AddFolder(fcfg)
 	m.StartFolder("default")
 	m.ServeBackground()
+	defer m.Stop()
 
 	waitFor := func(status string) error {
 		timeout := time.Now().Add(2 * time.Second)
@@ -676,6 +684,7 @@ func TestRWScanRecovery(t *testing.T) {
 	m.AddFolder(fcfg)
 	m.StartFolder("default")
 	m.ServeBackground()
+	defer m.Stop()
 
 	waitFor := func(status string) error {
 		timeout := time.Now().Add(2 * time.Second)
@@ -739,6 +748,7 @@ func TestGlobalDirectoryTree(t *testing.T) {
 	m := NewModel(defaultConfig, protocol.LocalDeviceID, "device", "syncthing", "dev", db, nil)
 	m.AddFolder(defaultFolderConfig)
 	m.ServeBackground()
+	defer m.Stop()
 
 	b := func(isfile bool, path ...string) protocol.FileInfo {
 		typ := protocol.FileInfoTypeDirectory
@@ -1646,6 +1656,109 @@ func TestSharedWithClearedOnDisconnect(t *testing.T) {
 	}
 }
 
+func TestIssue3496(t *testing.T) {
+	// It seems like lots of deleted files can cause negative completion
+	// percentages. Lets make sure that doesn't happen. Also do some general
+	// checks on the completion calculation stuff.
+
+	dbi := db.OpenMemory()
+	m := NewModel(defaultConfig, protocol.LocalDeviceID, "device", "syncthing", "dev", dbi, nil)
+	m.AddFolder(defaultFolderConfig)
+	m.StartFolder("default")
+	m.ServeBackground()
+	defer m.Stop()
+
+	m.ScanFolder("default")
+
+	addFakeConn(m, device1)
+	addFakeConn(m, device2)
+
+	// Reach into the model and grab the current file list...
+
+	m.fmut.RLock()
+	fs := m.folderFiles["default"]
+	m.fmut.RUnlock()
+	var localFiles []protocol.FileInfo
+	fs.WithHave(protocol.LocalDeviceID, func(i db.FileIntf) bool {
+		localFiles = append(localFiles, i.(protocol.FileInfo))
+		return true
+	})
+
+	// Mark all files as deleted and fake it as update from device1
+
+	for i := range localFiles {
+		localFiles[i].Deleted = true
+		localFiles[i].Version = localFiles[i].Version.Update(device1.Short())
+		localFiles[i].Blocks = nil
+	}
+
+	// Also add a small file that we're supposed to need, or the global size
+	// stuff will bail out early due to the entire folder being zero size.
+
+	localFiles = append(localFiles, protocol.FileInfo{
+		Name:    "fake",
+		Size:    1234,
+		Type:    protocol.FileInfoTypeFile,
+		Version: protocol.Vector{Counters: []protocol.Counter{{ID: device1.Short(), Value: 42}}},
+	})
+
+	m.IndexUpdate(device1, "default", localFiles)
+
+	// Check that the completion percentage for us makes sense
+
+	comp := m.Completion(protocol.LocalDeviceID, "default")
+	if comp.NeedBytes > comp.GlobalBytes {
+		t.Errorf("Need more bytes than exist, not possible: %d > %d", comp.NeedBytes, comp.GlobalBytes)
+	}
+	if comp.CompletionPct < 0 {
+		t.Errorf("Less than zero percent complete, not possible: %.02f%%", comp.CompletionPct)
+	}
+	if comp.NeedBytes == 0 {
+		t.Error("Need no bytes even though some files are deleted")
+	}
+	if comp.CompletionPct == 100 {
+		t.Errorf("Fully complete, not possible: %.02f%%", comp.CompletionPct)
+	}
+	t.Log(comp)
+
+	// Check that NeedSize does the correct thing
+	files, deletes, bytes := m.NeedSize("default")
+	if files != 1 || bytes != 1234 {
+		// The one we added synthetically above
+		t.Errorf("Incorrect need size; %d, %d != 1, 1234", files, bytes)
+	}
+	if deletes != len(localFiles)-1 {
+		// The rest
+		t.Errorf("Incorrect need deletes; %d != %d", deletes, len(localFiles)-1)
+	}
+}
+
+func addFakeConn(m *Model, dev protocol.DeviceID) {
+	conn1 := connections.Connection{
+		IntermediateConnection: connections.IntermediateConnection{
+			Conn:     tls.Client(&fakeConn{}, nil),
+			Type:     "foo",
+			Priority: 10,
+		},
+		Connection: &FakeConnection{
+			id: dev,
+		},
+	}
+	m.AddConnection(conn1, protocol.HelloResult{})
+
+	m.ClusterConfig(device1, protocol.ClusterConfig{
+		Folders: []protocol.Folder{
+			{
+				ID: "default",
+				Devices: []protocol.Device{
+					{ID: device1[:]},
+					{ID: device2[:]},
+				},
+			},
+		},
+	})
+}
+
 type fakeAddr struct{}
 
 func (fakeAddr) Network() string {

lib/protocol/bep_extensions.go

@@ -16,6 +16,10 @@ import (
 	"github.com/syncthing/syncthing/lib/rand"
 )
 
+const (
+	SyntheticDirectorySize = 128
+)
+
 var (
 	sha256OfEmptyBlock = sha256.Sum256(make([]byte, BlockSize))
 	HelloMessageMagic  = uint32(0x2EA7D90B)
@@ -56,8 +60,11 @@ func (f FileInfo) HasPermissionBits() bool {
 }
 
 func (f FileInfo) FileSize() int64 {
-	if f.IsDirectory() || f.IsDeleted() {
-		return 128
+	if f.Deleted {
+		return 0
+	}
+	if f.IsDirectory() {
+		return SyntheticDirectorySize
 	}
 	return f.Size
 }