* lib/versioner: Reduce surface area
This is a refactor while I was anyway rooting around in the versioner.
Instead of exporting every possible implementation and the factory and
letting the caller do whatever, this now encapsulates all that and
exposes a New() that takes a config.VersioningConfiguration.
Given that and that we don't know (from the outside) how a versioner
works or what state it keeps, we now just construct it once per folder
and keep it around. Previously it was recreated for each restore
request.
* unparam
* wip
We incorrectly gave a too small buffer to lz4.Compress, causing it to
allocate in some cases (when the data actually becomes larger when
compressed). This then panicked when passed to the buffer pool.
This ensures a buffer that is large enough, and adds tripwires closer to
the source in case this ever pops up again. There is a test that
exercises the issue.
* gui, lib/api: Adds support for prefers-color-scheme on default theme (fixes#6115)
- Renames current default theme into a new "light" theme
- Modifies assets serving to allow getting assets from different themes
* lib/api: Serve assets from arbitrary theme when path starts with "theme-assets"
* lib/api: Moves constant out of function
* Loads light theme in browsers without support for prefers-color-scheme
* gui: Disables dark theme when printing
* Prevents repeated injection and adds support for older browsers
The CSS is always loaded if there is no support for `matchMedia`.
* Fix bufferpool puts (ref #4976)
There was a logic error in Put() which made us put all large blocks into
segment zero, where we subsequently did not look for them.
I also added a lowest threshold, as we otherwise allocate a 128KiB
buffer when we need 24 bytes for a header and such.
* wip
* wip
* wip
* wip
* wip
* wip
* wip
* wip
* wip
* wip
* smaller stress
* cap/len
* wip
* wip
This adds a certificate lifetime parameter to our certificate generation
and hard codes it to twenty years in some uninteresting places. In the
main binary there are a couple of constants but it results in twenty
years for the device certificate and 820 days for the HTTPS one. 820 is
less than the 825 maximum Apple allows nowadays.
This also means we must be prepared for certificates to expire, so I add
some handling for that and generate a new certificate when needed. For
self signed certificates we regenerate a month ahead of time. For other
certificates we leave well enough alone.
The relay and discosrv didn't use the new lib/build package, now they
do. Conversely the lib/build package wasn't aware there might be other
users and hard coded the program name - now it's set by the build
script
This adds a field `guiAddressUsed` to the system status response, that
holds the current listening address actually in use. This may be
different from the one stored in the config because it may have been
overridden by environment or command line flag.
The GUI now checks this field to see if we are listening on localhost.
If we are not, the authentication required warning is displayed,
regardless of the *configured* listening address.
This is an experiment in testing, based on the advise to always call
t.Parallel() at the start of every test. Doing so makes tests run in
parallel, which is usually faster, but also exposes package level state
and potential race conditions better.
To support this I had to redesign the CSRF manager to not be package
global, which was indeed an improvement. And tests run five times faster
now.
This splits large writes into smaller ones when using a rate limit,
making them into a legitimate trickle rather than large bursts with a
long time in between.
This is the result of:
- Changing build.go to take the protobuf version from the modules
instead of hardcoded
- `go get github.com/gogo/protobuf@v1.3.0` to upgrade
- `go run build.go proto` to regenerate our code
Assume a folder error was set due to bad ignores on the latest scan.
Previously, doing a manual rescan would result in:
1. Clearing the folder error, which schedules (immediately) an fs
watcher restart
2. Attempting to load the ignores, which fails, so we set a folder
error and bail.
3. Now the fs watcher restarts, as scheduled, so we trigger a scan.
Goto 1.
This change fixes this by not clearing the error until the error is
actually cleared, that is, if both the health check and ignore loading
succeeds.
This introduces a better set of defaults for large databases. I've
experimentally determined that it results in much better throughput in a
couple of scenarios with large databases, but I can't give any
guarantees the values are always optimal. They're probably no worse than
the defaults though.
NATSymmetricUDPFirewall actually is not NAT at all, but means the machine has a global IP address and an UDP firewall in front (RFC calls it Symmetric UDP Firewall). This is punchable fine, both theoretically and also practically in testing.
* lib/model: Don't panic on failed chmod-back on directory (fixes#5836)
This makes the "in writable dir"-wrapper log chmod-back errors instead
of panicking. To do that we need a logger so the function moved into the
model package which is also the only place it's used. The tests came
along.
(The test also exercised osutil.RenameOrCopy like some sort of
piggybacking. I removed that.)
This adds a set of magical environment variables that can be used to
tweak the database parameters. It's totally undocumented and not
intended to be a long term or supported thing.
It's ugly, but there is a backstory. I have a couple of large
installations where the database options are inefficient or otherwise
suboptimal (24/7 compaction going on and stuff like that). I don't
*know* the correct database parameters, nor yet the formula or method to
derive them by, so this requires experimentation. Experimentation needs
to happen partly in production, and rolling out new builds for every
tweak isn't practical. This provides override points for all reasonable
values, while not changing anything by default.
Ideally, at the end of such experimentation, we'll know which values are
relevant to change and in what manner, and can provide a more user
friendly knob to do so - or do it automatically based on the database
size.
* add skeleton for lib/syncthing
* copy syncthingMain to lib/syncthing (verbatim)
* Remove code to deduplicate copies of syncthingMain
* fix simple build errors
* move stuff from main to syncthing with minimal mod
* merge runtime options
* actually use syncthing.App
* pass io.writer to lib/syncthing for auditing
* get rid of env stuff in lib/syncthing
* add .Error() and comments
* review: Remove fs interactions from lib
* and go 1.13 happened
* utility functions
Per the sync/atomic bug note:
> On ARM, x86-32, and 32-bit MIPS, it is the caller's
> responsibility to arrange for 64-bit alignment of 64-bit words
> accessed atomically. The first word in a variable or in an
> allocated struct, array, or slice can be relied upon to be
> 64-bit aligned.
All atomic accesses of 64-bit variables in syncthing code base are
currently ok (i.e they are all 64-bit aligned).
Generally, the bug is triggered because of incorrect alignement
of struct fields. Free variables (declared in a function) are
guaranteed to be 64-bit aligned by the Go compiler.
To ensure the code remains correct upon further addition/removal
of fields, which would change the currently correct alignment, I
added the following comment where required:
// atomic, must remain 64-bit aligned
See https://golang.org/pkg/sync/atomic/#pkg-note-BUG.
* lib/versioner: Add placeholder to provide the absolute file path to external commands
This commit adds support for a new placeholder, %FILE_PATH_FULL%, to the
command of the external versioner. The placeholder will be replaced by
the absolute path of the file that should be deleted.
* Revert "lib/versioner: Add placeholder to provide the absolute file path to external commands"
This reverts commit fb48962b947358e90a76e87a7794adb6057de476.
* lib/versioner: Replace all placeholders in external command (fixes#5849)
Before this commit, only these placeholders were replaced that span a
whole word, for example "%FOLDER_PATH%". Words that consisted of more
than one placeholder or additional characters, for example
"%FOLDER_PATH%/%FILE_PATH%", were left untouched.
* fixup! lib/versioner: Replace all placeholders in external command (fixes#5849)
* lib/ur: Implement crash (panic) reporting (fixes#959)
This implements a simple crash reporting method. It piggybacks on the
panic log files created by the monitor process, picking these up and
uploading them from the usage reporting routine.
A new config value points to the crash receiver base URL, which defaults
to "https://crash.syncthing.net/newcrash" (following the pattern of
"https://data.syncthing.net/newdata" for usage reports, but allowing us
to separate the service as required).
* lib/fs, lib/model: Add error channel to Watch to avoid panics (fixes#5697)
* forgot unsupported watch
* and more non(-standard)-unixy fixes
* and windows test
* review
* lib/api, lib/connections, gui: Show connection error for disconnected devices (fixes#3345)
This adds functionality in the connetions service to track the last
error per address. That is in turn exposed in the /rest/system/status
API method, as that is also where we already show the listener status
from the connection service.
The GUI uses this info where it lists addresses, showing errors (if any)
in red underneath each address.
I also slightly refactored the existing status method on the connection
service to have a better name and return typed information.
* ok
* review
* formatting
* review
The check in ClusterConfig() when iterating through announced devices
in a folder explicitly skips entries without a non-zero IndexID.
Therefore, the check for IndexID == 0 just below will never be true
and the intended cleanup of local index data will not happen.
Plainly remove that check to make the intended case distinction work.
* lib/protocol: Wait for reader/writer loops on close (fixes#4170)
* waitgroup
* lib/model: Don't hold lock while closing connection
* fix comments
* review (lock once, func argument) and naming
* lib/model: Send cluster config before releasing pmut
* reshuffle
* add model.connReady to track cluster-config status
* Corrected comments/strings
* do it in protocol
This constructs the map of hashes of zero blocks from constants instead
of calculating it at startup time. A new test verifies that the map is
correct.
* cmd/syncthing, lib/gui: Separate gui into own package (ref #4085)
* fix tests
* Don't use main as interface name (make old go happy)
* gui->api
* don't leak state via locations and use in-tree config
* let api (un-)subscribe to config
* interface naming and exporting
* lib/ur
* fix tests and lib/foldersummary
* shorter URVersion and ur debug fix
* review
* model.JsonCompletion(FolderCompletion) -> FolderCompletion.Map()
* rename debug facility https -> api
* folder summaries in model
* disassociate unrelated constants
* fix merge fail
* missing id assignement
* lib/tlsutil: Enable TLS 1.3 when available, on test builds (fixes#5065)
This enables TLS 1.3 negotiation on Go 1.12 by setting the GODEBUG
variable. For now, this just gets enabled on test versions (those with a
dash in the version number).
Users wishing to enable this on production builds can set GODEBUG
manually.
The string representation of connections now includes the TLS version
and cipher suite. This becomes part of the log output on connections.
That is, when talking to an old client:
Established secure connection .../TLS1.2-TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
and now potentially:
Established secure connection .../TLS1.3-TLS_AES_128_GCM_SHA256
(The cipher suite was there previously in the log output, but not the
TLS version.)
I also added this info as a new Crypto() method on the connection, and
propagate this out to the API and GUI, where it can be seen in the
connection address hover (although with bad word wrapping sometimes).
* wip
* wip
Flush the batch when exceeding a certain size, instead of when reaching a number
of batched operations.
Move batch to lowlevel to be able to use it in NamespacedKV.
Increase the leveldb memory buffer from 4 to 16 MiB.
* cleanup Fatal in lib/config/config.go
* cleanup Fatal in lib/config/folderconfiguration.go
* cleanup Fatal in lib/model/model.go
* cleanup Fatal in cmd/syncthing/monitor.go
* cleanup Fatal in cmd/syncthing/main.go
* cleanup Fatal in lib/api
* remove Fatal methods from logger
* lowercase in errors.Wrap
* one less channel
I'm working through linter complaints, these are some fixes. Broad
categories:
1) Ignore errors where we can ignore errors: add "_ = ..." construct.
you can argue that this is annoying noise, but apart from silencing the
linter it *does* serve the purpose of highlighting that an error is
being ignored. I think this is OK, because the linter highlighted some
error cases I wasn't aware of (starting CPU profiles, for example).
2) Untyped constants where we though we had set the type.
3) A real bug where we ineffectually assigned to a shadowed err.
4) Some dead code removed.
There'll be more of these, because not all packages are fixed, but the
diff was already large enough.
This adds a folder option "CopyOwnershipFromParent" which, when set,
makes Syncthing attempt to retain the owner/group information when
syncing files. Specifically, at the finisher stage we look at the parent
dir to get owner/group and then attempt a Lchown call on the temp file.
For this to succeed Syncthing must be running with the appropriate
permissions. On Linux this is CAP_FOWNER, which can be granted by the
service manager on startup or set on the binary in the filesystem. Other
operating systems do other things, but often it's not required to run as
full "root". On Windows this patch does nothing - ownership works
differently there and is generally less of a deal, as permissions are
inherited as ACLs anyway.
There are unit tests on the Lchown functionality, which requires the
above permissions to run. There is also a unit test on the folder which
uses the fake filesystem and hence does not need special permissions.
To do so the BlockMap struct has been removed. It behaves like any other prefixed
part of the database, but was not integrated in the recent keyer refactor. Now
the database is only flushed when files are in a consistent state.
There was a problem in iterating the sequence index that could result
in missing updates. The issue is that while the index was (correctly)
iterated in a snapshot, the actual file infos were read dirty outside of
the snapshot. This fixes this by doing the reads inside the snapshot,
and also updates a couple of other places that did the same thing more
or less harmfully (I didn't investigate).
To avoid similar issues in the future I did some renaming of the
getFile* methods - the ones in a transaction are just getFile, while the
ones directly on the database are variants of getFileDirty to highlight
what's going on.
* lib/fs, lib/model: Improve filesystem operations during tests (fixes#5422)
Introduces MustFilesystem that panics on errors and should be used for operations
during testing which must never fail.
Create temporary directories outside of testdata.
* don't do a filesystem, just a wrapper around os for testing
* fix copyright
This avoids waiting until next ping and timeout until the connection is actually
closed both by notifying the peer of the disconnect and by immediately closing
the local end of the connection after that. As a nice side effect, info level
logging about dropped connections now have the actual reason in it, not a generic
timeout error which looks like a real problem with the connection.
* 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
Here the event Logger is rewritten as a service with a main loop instead
of mutexes. This loop has a select with essentially two legs: incoming
events, and subscription changes. When both are possible select will
chose one randomly, thus ensuring that in practice unsubscribes will
happen timely and not block the system.
Updates the package and fixes a test that depended on the old behavior
of Write() being equivalent to Reset()+Write() which is no longer the
case. The scanner already did resets after each block write, so this is
fine.
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.
In a recent change (#5201) this return disappeared. The effect is that
we first shortcut the file and then also treat it normally. This results
in to database updates after each other, which are bound to end up in
the same batch. This means we remove one sequence entry and add two.
Not marking the issues as fixed, because I need to do more testing and
there are other discrepancies...
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
* lib/fs: Add fakefs
This adds a new fake filesystem type. It's described rather extensively
in fakefs.go, but the main point is that it's for testing: when you want
to spin up a Syncthing and have a terabyte or two of random files that
can be synced somewhere, or an inifitely large filesystem to sync files
into.
It has pseudorandom properties such that data read from one fakefs can
be written into another fakefs and read back and it will look
consistent, without any of the data actually being stored.
To use:
<folder id="default" path="whatever" ...>
<filesystemType>fake</filesystemType>
This will create an empty fake filesystem. You can also specify that it
should be prefilled with files:
<folder id="default" path="whatever?size=2000000" ...>
<filesystemType>fake</filesystemType>
This will create a filesystem filled with 2TB of random data that can be
scanned and synced. There are more options, see fakefs.go.
Prefilled data is based on a deterministic seed, so you can index the
data and restart Syncthing and the index is still correct for all the
stored data.
The problem here is that we would update the sequence index before
updating the FileInfos, which would result in a high sequence number
pointing to a low-sequence FileInfo. The index sender would pick up the
high sequence number, send the old file, and think everything was good.
On the receiving side the old file is a no-op and ignored. The file
remains out of sync until another update for it happens.
This fixes that by correcting the order of operations in the database
update: first we remove old sequence index entries, then we update the
FileInfos (which now don't have anything pointing to them) and then we
add the sequence indexes (which the index sender can see).
The other option is to add "proper" transactions where required at the
database layer. I actually have a branch for that, but it's literally
thousands of lines of diff and I'm putting that off for another day as
this solves the problem...
The problem here is that we would update the sequence index before
updating the FileInfos, which would result in a high sequence number
pointing to a low-sequence FileInfo. The index sender would pick up the
high sequence number, send the old file, and think everything was good.
On the receiving side the old file is a no-op and ignored. The file
remains out of sync until another update for it happens.
This fixes that by correcting the order of operations in the database
update: first we remove old sequence index entries, then we update the
FileInfos (which now don't have anything pointing to them) and then we
add the sequence indexes (which the index sender can see).
The other option is to add "proper" transactions where required at the
database layer. I actually have a branch for that, but it's literally
thousands of lines of diff and I'm putting that off for another day as
this solves the problem...
This removes the out of disk space check from CheckHealth. The disk space is now
only checked if there are files to pull, in which case pulling those files is
stopped, but everything else (dirs, links, deletes) keeps running -> can recover
disk space through pulling.
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.
This is an improvement of PR #4493 and related to (and maybe fixing) #4961
and #4475. Maybe fixing, because there is no clear reproducer for that
problem.
The previous PR added a mechanism to resurrect missing parent directories,
if there is a valid child file to be pulled. The same mechanism does not
exist for dirs and symlinks, even though a missing parent can happen for
those items as well. Therefore this PR extends the resurrection to all types
of pulled items.
In addition I moved the IsDeleted branch while iterating over
processDirectly to the existing IsDeleted branch in the WithNeed iteration.
This saves one pointless assignment and IsDeleted query. Also
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.
To optimize WithNeed, which is called for the local device whenever an index
update is received. No tracking for remote devices to conserve db space, as
WithNeed is only queried for completion.
I'm trying to slowly clean this up a bit, and moving functionality out
into the folder types and having those methods not reach into model is
part of it. That can mean takign some odd arguments in the meantime,
some of those should probably become interfaces or properties on folder
in the long term.
The functionality was anyway mostly implemented there and not isolated
in the folderScanner type. The attempt to refactor it out in the other
direction wouldn't work given that the event loop and stuff is on
`folder`.
This adds a couple of utilities for transporting databases in JSON and a
test to load a database and verify a couple of invalid bits. The test
itself is quite pointless at the moment, but it lays the groundwork for
testing the migration of this data in the next step (after the invalid
bit should be changed to local flags for local files).
When that happens we need to have a database in the old format already
there in order to be able to test the migration.
To newer names better reflecting their types and yet sorting together
with folder.go. Doing it now without asking because there are no open
PRs that will get killed by it, and to avoid bikeshedding the names.
The actual pull method (which is really the only thing that differs
between them) is now an interface member which gets overridden by the
subclass.
"Subclass?!" Well, this is dynamic dispatch with overriding, I guess.
Instead of walking and unmarshalling the entire db and sorting the resulting
file infos by sequence, add store device keys by sequence number in the
database. Thus only the required file infos need be unmarshalled and are already
sorted by index.
Bumping the limit to 2 * the max block size (16 MiB) is a slight
increase compared to previously. Nonetheless I think it's good to allow
us to queue one request and have one on the way in, or conversely have
one large block on the way in and be able to ask for smaller blocks from
others at the same time.
Two small behavior changes: don't "charge" the data to the global rate
limit until it's been accepted by the device specific limiter, and fix
the send/recv direction in the log print on per device rate limits.
clearAddresses write locks the struct and then calls notify. notify in turn tries to obtain a read lock on the same mutex. The result was a deadlock. This change unlocks the struct before calling notify.
Given that we've taken on the resposibility of maintaining this forked
package I've added it to the Syncthing organization. We still vendor it
like an external package, because it's convenient to keep it as a fork
of upstream to easier merge and file pull requests towards them.
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.
Unignored files are marked as conflicting while scanning, which is then resolved
in the subsequent pull. Automatically reconciles needed items on send-only
folders, if they do not actually differ except for internal metadata.
This doesn't happen today, but it might in the future if the block size
were increased or made variable and we were talking to a client from the
future.
* lib/db: Don't panic on negative counts (fixes#4659)
So, negative counts should never happen and hence the original idea to
panic. However, this sucks as the panic will happen in a folder runner,
be automatically swallowed by suture, and the runner gets restarted but
now we are in a bad state. (Related: #4758)
At the time of writing the global list is somewhat in flux (we've
changed how ignored files are handled, invalid bits, etc.) and I think
that can cause unusual conditions here. Hence just fixing up the numbers
instead until the next full recount.
When scanner.Walk detects a change, it now returns the new file info as well as the old file info. It also finds deleted and ignored files while scanning.
Also directory deletions are now always committed to db after their children to prevent temporary failure on remote due to non-empty directory.
