* go mod init; rm -rf vendor
* tweak proto files and generation
* go mod vendor
* clean up build.go
* protobuf literals in tests
* downgrade gogo/protobuf
This changes the TLS and certificate handling in a few ways:
- We always use TLS 1.2, both for sync connections (as previously) and
the GUI/REST/discovery stuff. This is a tightening of the requirements
on the GUI. AS far as I can tell from caniusethis.com every browser from
2013 and forward supports TLS 1.2, so I think we should be fine.
- We always greate ECDSA certificates. Previously we'd create
ECDSA-with-RSA certificates for sync connections and pure RSA
certificates for the web stuff. The new default is more modern and the
same everywhere. These certificates are OK in TLS 1.2.
- We use the Go CPU detection stuff to choose the cipher suites to use,
indirectly. The TLS package uses CPU capabilities probing to select
either AES-GCM (fast if we have AES-NI) or ChaCha20 (faster if we
don't). These CPU detection things aren't exported though, so the tlsutil
package now does a quick TLS handshake with itself as part of init().
If the chosen cipher suite was AES-GCM we prioritize that, otherwise we
prefer ChaCha20. Some might call this ugly. I think it's awesome.
This adds a thin type that holds the state associated with the
leveldb.DB, leaving the huge Instance type more or less stateless. Also
moves some keying stuff into the DB package so that other packages need
not know the keying specifics.
(This does not, yet, fix the cmd/stindex program, in order to keep the
diff size down. Hence the keying constants are still exported.)
* lib/model, cmd/syncthing: Wait for folder restarts to complete (fixes#5233)
This is the somewhat ugly - but on the other hand clear - fix for what
is really a somewhat thorny issue. To avoid zombie folder runners a new
mutex is introduced that protects the RestartFolder operation. I hate
adding more mutexes but the alternatives I can think of are worse.
The other part of it is that the POST /rest/system/config operation now
waits for the config commit to complete. The point of this is that until
the commit has completed we should not accept another config commit. If
we did, we could end up with two separate RestartFolders queued in the
background. While they are both correct, and will run without
interfering with each other, we can't guarantee the order in which they
will run. Thus it could happen that the newer config got committed
first, and the older config commited after that, leaving us with the
wrong config running.
* test
* wip
* hax
* hax
* unflake test
* per folder mutexes
* paranoia
* race
The previous "Bad Request" was really confusing as it implies it's
somethign wrong with the request, which there isn't - the problem is
that server configuration forbids the request.
Adds a receive only folder type that does not send changes, and where the user can optionally revert local changes. Also changes some of the icons to make the three folder types distinguishable.
We have the invalid bit to indicate that a file isn't good. That's enough for remote devices. For ourselves, it would be good to know sometimes why the file isn't good - because it's an unsupported type, because it matches an ignore pattern, or because we detected the data is bad and we need to rescan it.
Or, and this is the main future reason for the PR, because it's a change detected on a receive only device. We will want something like the invalid flag for those changes, but marking them as invalid today means the scanner will rehash them. Hence something more fine grained is required.
This introduces a LocalFlags fields to the FileInfo where we can stash things that we care about locally. For example,
FlagLocalUnsupported = 1 << 0 // The kind is unsupported, e.g. symlinks on Windows
FlagLocalIgnored = 1 << 1 // Matches local ignore patterns
FlagLocalMustRescan = 1 << 2 // Doesn't match content on disk, must be rechecked fully
The LocalFlags fields isn't sent over the wire; instead the Invalid attribute is calculated based on the flags at index sending time. It's on the FileInfo anyway because that's what we serialize to database etc.
The actual Invalid flag should after this just be considered when building the global state and figuring out availability for remote devices. It is not used for local file index entries.
When dropping delta index IDs due to upgrade, only drop our local one.
Previously, when dropping all of them, we would trigger a full send in
both directions on first connect after upgrade. Then the other side
would upgrade, doing the same thing. Net effect is full index data gets
sent twice in both directions.
With this change we just drop our local ID, meaning we will send our
full index on first connect after upgrade. When the other side upgrades,
they will do the same. This is a bit less cruel.
The current 500 "test failed" looks and sounds like a problem in the
relay pool server, while it actually indicates a problem on the
announcing side. Instead use 400 "connection test failed" to indicate
that the request was bad and what was the test.
Since #4340 pulls aren't happening every 10s anymore and may be delayed up to 1h.
This means that no folder error event reaches the web UI for a long time, thus no
failed items will show up for a long time. Now errors are populated when the
web UI is opened.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4650
LGTM: AudriusButkevicius
This is a new revision of the discovery server. Relevant changes and
non-changes:
- Protocol towards clients is unchanged.
- Recommended large scale design is still to be deployed nehind nginx (I
tested, and it's still a lot faster at terminating TLS).
- Database backend is leveldb again, only. It scales enough, is easy to
setup, and we don't need any backend to take care of.
- Server supports replication. This is a simple TCP channel - protect it
with a firewall when deploying over the internet. (We deploy this within
the same datacenter, and with firewall.) Any incoming client announces
are sent over the replication channel(s) to other peer discosrvs.
Incoming replication changes are applied to the database as if they came
from clients, but without the TLS/certificate overhead.
- Metrics are exposed using the prometheus library, when enabled.
- The database values and replication protocol is protobuf, because JSON
was quite CPU intensive when I tried that and benchmarked it.
- The "Retry-After" value for failed lookups gets slowly increased from
a default of 120 seconds, by 5 seconds for each failed lookup,
independently by each discosrv. This lowers the query load over time for
clients that are never seen. The Retry-After maxes out at 3600 after a
couple of weeks of this increase. The number of failed lookups is
stored in the database, now and then (avoiding making each lookup a
database put).
All in all this means clients can be pointed towards a cluster using
just multiple A / AAAA records to gain both load sharing and redundancy
(if one is down, clients will talk to the remaining ones).
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4648
This keeps the data we need about sequence numbers and object counts
persistently in the database. The sizeTracker is expanded into a
metadataTracker than handled multiple folders, and the Counts struct is
made protobuf serializable. It gains a Sequence field to assist in
tracking that as well, and a collection of Counts become a CountsSet
(for serialization purposes).
The initial database scan is also a consistency check of the global
entries. This shouldn't strictly be necessary. Nonetheless I added a
created timestamp to the metadata and set a variable to compare against
that. When the time since the metadata creation is old enough, we drop
the metadata and rebuild from scratch like we used to, while also
consistency checking.
A new environment variable STCHECKDBEVERY can override this interval,
and for example be set to zero to force the check immediately.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4547
LGTM: imsodin
So STDEADLOCK seems to do the same thing as STDEADLOCKTIMEOUT, except in
the other package. Consolidate?
STDEADLOCKTHRESHOLD is actually called STLOCKTHRESHOLD, correct the help
text.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4598
Two issues since the filesystem migration;
- filepath.Base() doesn't do what the code expected when the path ends
in a slash, and we make sure the path ends in a slash.
- the return should be fully qualified, not just the last component.
With this change it works as before, and passes the new test for it.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4591
LGTM: imsodin
This should address issue as described in https://forum.syncthing.net/t/stun-nig-party-with-paused-devices/10942/13
Essentially the model and the connection service goes out of sync in terms of thinking if we are connected or not.
Resort to model as being the ultimate source of truth.
I can't immediately pin down how this happens, yet some ideas.
ConfigSaved happens in separate routine, so it's possbile that we have some sort of device removed yet connection comes in parallel kind of thing.
However, in this case the connection exists in the model, and does not exist in the connection service and the only way for the connection to be removed
in the connection service is device removal from the config.
Given the subject, this might also be related to the device being paused.
Also, adds more info to the logs
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4533
When STHASHING is set, don't benchmark as it's already decided. If weak
hashing isn't set to "auto", don't benchmark that either.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4349
Starting stuff from init() is an antipattern, and the innerProcess
variable isn't 100% reliable. We should sort out the other uses of it as
well in due time.
Also removing the hack on innerProcess as I happened to see it and the
affected versions are now <1% users.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4185
The folder already knew how to stop properly, but the fs.Walk() didn't
and can potentially take a very long time. This adds context support to
Walk and the underlying scanning stuff, and passes in an appropriate
context from above. The stop channel in model.folder is replaced with a
context for this purpose.
To test I added an infiniteFS that represents a large amount of data
(not actually infinite, but close) and verify that walking it is
properly stopped. For that to be implemented smoothly I moved out the
Walk function to it's own type, as typically the implementer of a new
filesystem type might not need or want to reimplement Walk.
It's somewhat tricky to test that this actually works properly on the
actual sendReceiveFolder and so on, as those are started from inside the
model and the filesystem isn't easily pluggable etc. Instead I've tested
that part manually by adding a huge folder and verifying that pause,
resume and reconfig do the right things by looking at debug output.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4117
This adds a parameter "events" to the /rest/events endpoint. It should
be a comma separated list of the events the consumer is interested in.
When not given it defaults to the current set of events, so it's
backwards compatible.
The API service then manages subscriptions, creating them as required
for each requested event mask. Old subscriptions are not "garbage
collected" - it's assumed that in normal usage the set of event
subscriptions will be small enough. Possibly lower than before, as we
will not set up the disk event subscription unless it's actually used.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4092
This deprecates the current minDiskFreePct setting and introduces
minDiskFree. The latter is, in it's serialized form, a string with a
unit. We accept percentages ("2.35%") and absolute values ("250 k", "12.5
Gi"). Common suffixes are understood. The config editor lets the user
enter the string, and validates it.
We still default to "1 %", but the user can change that to an absolute
value at will.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4087
LGTM: AudriusButkevicius, imsodin
After this change,
- Symlinks on Windows are always unsupported. Sorry.
- Symlinks are always enabled on other platforms. They are just a small
file like anything else. There is no need to special case them. If you
don't want to sync some symlinks, ignore them.
- The protocol doesn't differentiate between different "types" of
symlinks. If that distinction ever does become relevant the individual
devices can figure it out by looking at the destination when they
create the link.
It's backwards compatible in that all the old symlink types are still
understood to be symlinks, and the new SYMLINK type is equivalent to the
old SYMLINK_UNKNOWN which was always a valid way to do it.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3962
LGTM: AudriusButkevicius
The monitor process should not set STNORESTART as this indicates the
intention from the user. Setting STMONITORED is enough, as this tells
the next Syncthing instance that it is running under the monitor
process.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3932
The Ping event is important, as it means that requests complete within
a sensible time. The disk events API didn't have the Ping event, so
if there were no disk events, the request would keep taking forever.
Unless, of course, there's a reverse proxy which times the request out
after a suitably large interval (or something else aborts it), in which
case Syncthing isn't very happy.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3929
Instead of
[I6KAH] 19:05:56 INFO: Single thread hash performance is 359 MB/s using minio/sha256-simd (354 MB/s using crypto/sha256).
it now says
[I6KAH] 19:06:16 INFO: Single thread SHA256 performance is 359 MB/s using minio/sha256-simd (354 MB/s using crypto/sha256).
[I6KAH] 19:06:17 INFO: Actual hashing performance is 299.01 MB/s
which is more informative. This is also the number it reports in usage
reporting.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3918
Adds support for -auditfile= where is "-" for stdout, "--" for stderr, or a
filename. It can be left blank (or left out entirely) for the original
behaviour of creating a timestamped filename.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3860
This avoids unnecessary browser request failures and retries. Eg:
- Browser reuses existing HTTP connection for GUI refresh request
- Server closes connection with request in flight
- Browser retries GET request.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3854
This adds support for AES_256_GCM_SHA384 (in there since Go 1.5, a bit
of a shame we missed it) and ChaCha20-Poly1305 (if built with Go 1.8;
ignored on older Gos).
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3822
Instead, trust (and test) that the temp file has appropriate permissions
from the start. The only place where this changes our behavior is for
ignores which go from 0644 to 0600. I'm OK with that.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3756
The current way is quite confusing for new users - we create a default
folder, but it's not usable with the default folder created somewhere
else. Instead, when setting up for the first time with two devices, the
default folder must be removed and recreated on one of them. This comes
up on IRC and the forum now and then.
I think this matches expectactions better.
Another alternative would be to remove it entirely (not create a default
folder), but then we should also add some guidance in the UI on how to
proceed.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3751
This makes the device ID a real type that can be used in the protobuf
schema. That avoids the juggling back and forth from []byte in a bunch
of places and simplifies the code.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3695
This adds autodetection of the fastest hashing library on startup, thus
handling the performance regression. It also adds an environment
variable to control the selection, STHASHING=standard (Go standard
library version, avoids SIGILL crash when the minio library has bugs on
odd CPUs), STHASHING=minio (to force using the minio version) or unset
for the default autodetection.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3617
When the GUI/API is bound to localhost, we enforce that the Host header
looks like localhost. This can be disabled by setting
insecureSkipHostCheck in the GUI config.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3558
We used to consider deleted files & directories 128 bytes large. After
the delta indexes change a bug slipped in where deleted files would be
weighted according to their old non-deleted size. Both ways are
incorrect (but the latest change made it worse), as if there are more
files deleted than remaining data in the repo the needSize can be
greater than the globalSize, resulting in a negative completion
percentage.
This change makes it so that deleted items are zero bytes large, which
makes more sense. Instead we expose the number of files that we need to
delete as a separate field in the Completion() result, and hack the
percentage down to 95% complete if it was 100% complete but we need to
delete files. This latter part is sort of ugly, but necessary to give
the user some sort of feedback.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3556
Furthermore:
1. Cleans configs received, migrates them as we receive them.
2. Clears indexes of devices we no longer share the folder with
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3478
This adds a new nanoseconds field to the FileInfo, populates it during
scans and sets the non-truncated time in Chtimes calls.
The actual file modification time is defined as modified_s seconds +
modified_ns nanoseconds. It's expected that the modified_ns field is <=
1e9 (that is, all whole seconds should go in the modified_s field) but
not really enforced. Given that it's an int32 the timestamp can be
adjusted += ~2.9 seconds by the modified_ns field...
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3431
This adds a config to enable debug functions on the API server, which is
by default disabled. When enabled, the /rest/debug things become
available and become available without requiring a CSRF token (although
authentication is required if configured).
We also add a new endpoint /rest/debug/cpuprof?duration=15s (with the
duration being configurable, defaulting to 30s). This runs a CPU profile
for the duration and returns it as a file. It sets headers so that a
browser will save the file with an informative name.
The same is done for heap profiles, /rest/debug/heapprof, which does not
take any parameters.
The purpose of this is that any user can enable debugging under
advanced, then point their browser to the endpoint above and get a file
that contains a CPU or heap profile we can use, with the filename
telling us what version and architecture the profile is from.
On the command line, this becomes
curl -O -J http://localhost:8082/rest/debug/cpuprof?duration=5s
curl: Saved to filename
'syncthing-cpu-darwin-amd64-v0.14.3+4-g935bcc0-110307.pprof'
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3467
This used to happen by itself as the connecting device always sent an
Index message and we triggered on that. Nowadays there's no guarantee
for that, but we anyway need to send out one event to let listeners know
the state of folders shared with the device.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3438
This slightly changes the interface used for committing configuration
changes. The two parts are now:
- VerifyConfiguration, which runs synchronously and locked, and can
abort the config change. These callbacks shouldn't *do* anything
apart from looking at the config changes and saying yes or no. No
change from previously.
- CommitConfiguration, which runs asynchronously (one goroutine per
call) *after* replacing the config and releasing any locks. Returning
false from these methods sets the "requires restart" flag, which now
lives in the config.Wrapper.
This should be deadlock free as the CommitConfiguration calls can take
as long as they like and can wait for locks to be released when they
need to tweak things. I think this should be safe compared to before as
the CommitConfiguration calls were always made from a random background
goroutine (typically one from the HTTP server), so it was always
concurrent with everything else anyway.
Hence the CommitResponse type is gone, instead you get an error back on
verification failure only, and need to explicitly check
w.RequiresRestart() afterwards if you care.
As an added bonus this fixes a bug where we would reset the "requires
restart" indicator if a config that did not require restart was saved,
even if we already were in the requires-restart state.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3386
* relaypoolsrv/master: (32 commits)
Fetch deps of deps X_x
Here we go with gvt bugs
Screw godep
Add solaris support back in
Add font awesome
No value is less than zero
Screw solaris
Godeps
Refactor javascript, always show table, add sorting
Add local geoip
Update dependencies
Hey look, had to check all code out on linux to fix the deps
Update godeps, reduce amount of time spent testing a relay. Goddamit godeps.
Add timeouts, deal with overlapping markers, add a table, increase circle radiuses
Fix a couple of issues with the relays map (geoip, 'data unavailable')
Rate infos are in kbps, not kBps
Add support for header holding IP address
Update relay parameters even if it already exists (fixes#3)
Add missing space
Add homepage
...
This changes the BEP protocol to use protocol buffer serialization
instead of XDR, and therefore also the database format. The local
discovery protocol is also updated to be protocol buffer format.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3276
LGTM: AudriusButkevicius
If the listen address scheme is set to tcp4:// or tcp6://, it needs to be
made sure that the remote address matches this scheme before it is added to
the database.
This prevents invalid URIs like tcp4://<IPv6 address>:<port> or tcp6://<IPv4
address>:<port>.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3378
This contains the following behavioral changes:
- Duplicate folder IDs is now fatal during startup
- Invalid folder flags in the ClusterConfig is fatal for the connection
(this will go away soon with the proto changes, as we won't have any
unknown flags any more then)
- Empty path is a folder error reported at runtime
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3370
Events API consumers rely on being able to detect that events were skipped
by the fact that the event ID has increased by more than 1. This is
documented, and is absolutely necessary when trying to maintain a local
model of Syncthing's state.
With the introduction of LocalChangeDetected, which is not exposed to the
Events API, this contract was broken.
This commit introduces separate concepts of a "Global ID" and a
"Subscription ID". The Global ID of an event is unique across all
subscriptions. The Subscription ID is local to a particular subscription,
and always increments by 1. They are both exposed over the Events API, but
the Subscription ID uses the key "id" for backwards compatibility, and
the "?since=xx" parameter refers to the Subscription ID (making the Global
ID for information only).
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3351
LGTM: calmh
While attempting to fix#2782 I thought the problem was the
CheckFolderHealth method, so I cleaned it up. That turned out not to be
the case, but I think this is better anyhow.
It also moves the "create folder and marker if the folder was empty in
the index" code to StartFolder where I think it makes better sense.
This is covered by a number of existing tests.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3343
The purpose of this operation is to separate the serving of GUI assets a
bit from the serving of the REST API. It's by no means complete. The end
goal is something like a combined server type that embeds a statics
server and an API server and wraps it in authentication and HTTPS and
stuff, plus possibly a named pipe server that only provides the API and
does not wrap in the same authentication etc.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3273
This sacrifices the ability to return an error when creating the service
for being more persistent in keeping it running.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3270
LGTM: AudriusButkevicius, canton7
As noted in the ticket I no longer agree that dev builds should not auto
upgrade. The main reason is that we give dev builds to users to test
specific fixes, and noone is happier by them being inadvertently stuck
on that version when a newer version including the fix is released.
For developers, it's first of all probably unlikely that development is
happening on a build that's older than release, and secondly STNOUPGRADE
can be set in the environment once and for all if it an issue.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3244
* relaysrv/master: (60 commits)
Add new dependencies
Add more logging in the case of relaypoolsrv internal server error
Dependency update
Update deps
Update packages, fix testutil. Goddamit godep.
Typo
Add signal handlers (fixes#15)
Update readme (fixes#16)
Limit number of connections (fixes#23)
Enable extra logging in pool.go even when -debug not specified
Add Antony Male to CONTRIBUTORS
Allow extAddress to be set from the command line
URLs should have Go units
Add CORS headers
Fix units
Expose provided by in status endpoint
Add ability to advertise provider
Change the URL
Rename relaysrv binary, see #11
Jail the whole thing a bit more
...
* discosrv/master: (64 commits)
Use atomics for statistics handling (fixes#45)
Lower case JSON fields are nicer
Change v13 to v2
Remove explicit relay handling
Update vendored github.com/cznic/ql (fixes#34)
Defer fd.Close() (fixes#37)
There is no "get dependencies" step
Add vendor/golang.org/x/net/context
Use Go 1.5 vendoring instead of Godeps
Add debug performance logging per request
Must close result sets
Set Retry-After header
Ignores
lru.Cache is not concurrency safe
We need a limit on the number of PostgreSQL connections
Correct example DSN (fixes#29)
Allow plain HTTP serving behind a proxy
Fix Query/Answer stats
Reduce our patience with slow clients somewhat
Discovery server should print device ID of certificate at startup
...
With the previous setup, browsers were free to use a local cache for any
length of time they pleased: we didn't set an 'Expires' header (or max-age
directive), and Cache-Control just said "you're free to cache this".
Therefore be more explicit: we don't mind if browsers cache things, but they
MUST revalidate everything on every request.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3221
The intention for this package is to provide a combination of the
security of crypto/rand and the convenience of math/rand. It should be
the first choice of random data unless ultimate performance is required
and the usage is provably irrelevant from a security standpoint.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3186
We need to set these properties *before* Angular starts making requests,
and doing that from the response to a request is too late. The obvious
choice (to me) would be to use the angular $cookies service, but that
service isn't available until after initialization so we can't use it.
Instead, add a special file that is loaded by index.html and includes
the info we need before the JS app even starts running.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3152
Without this the summary service doesn't know to recalculate completion
percentage for remote devices when DownloadProgress messages come in.
That means that completion percentage isn't updated in the GUI while
transfers of large files are ongoing. With this change, it updates
correctly.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3144
I think this better reflects what it means. Also tweaks the verbose
format to be more like our other things and lightly refactors the code
to not have the boolean and include the folder in the event.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3121
Because json.NewDecoder(r).Decode(&v) doesn't necessarily consume all
data on the reader, that means an HTTP connection can't be reused. We
don't do a lot of HTTP traffic where we read JSON responses, but the
discovery is one such place. The other two are for POSTs from the GUI,
where it's not exactly critical but still nice if the connection still
can be keep-alive'd after the request as well.
Also ensure that we call req.Body.Close() for clarity, even though this
should by all accounts not really be necessary.
GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3050
