When handling files that consist only of power-of-two-sized blocks of
zero we'd know we have nothing to write, and when using sparse files
we'd never even create the temp file. Hence the sync would fail.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Just to be entirely sure that if the migration succeeds the schema
version is always also updated. Currently if a migration succeeds but a
later migration doesn't, the changes of the migration apply but the
version stays - if the migration is breaking/non-idempotent, it will
fail when it tries to rerun it next time (otherwise it's just a
pointless re-execution).
Unfortunately with the current `db.runScripts` it wasn't that easy to
do, so I had to do quite a bit of refactoring. I am also ensuring the
right order of transactions now, though I assume that was already the
case lexicographically - can't hurt to be safe.
Currently, the input field has no step defined, meaning that it can be
increased with the arrow keys by the default value of "1". Considering
the fact that the default value is "3600" (seconds or one hour), it is
unlikely that the user wants to change it with such minimal steps.
For this reason, change the default step to "3600" (one hour). If the
user needs more granual control, they can still input the value
in seconds manually.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Currently, the bandwidth limit input fields have no step defined, and as
such they use the default value of "1". Taking into account the fact
that these fields use KiB as their measurements, it makes more sense to
use larger steps, such as "1024" (1 MiB), as in most cases, it is very
unlikely that the user needs to have byte-level control over the limits.
Note that these steps only apply to increasing the values by using the
arrow keys, and the user is still allowed to input any value they want
manually.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This just removes an unnecessary foreign key constraint, where we
already do the garbage collection manually in the database service.
However, as part of getting here I tried a couple of other variants
along the way:
- Changing the order of the primary key from `(hash, blocklist_hash,
idx)` to `(blocklist_hash, idx, hash)` so that inserts would be
naturally ordered. However this requires a new index `on blocks (hash)`
so that we can still look up blocks by hash, and turns out to be
strictly worse than what we already have.
- Removing the primary key entirely and the `WITHOUT ROWID` to make it a
rowid table without any required order, and an index as above. This is
faster when the table is small, but becomes slower when it's large (due
to dual indexes I guess).
These are the benchmark results from current `main`, the second
alternative below ("Index(hash)") and this proposal that retains the
combined primary key ("combined"). Overall it ends up being about 65%
faster.
<img width="764" height="452" alt="Screenshot 2025-08-29 at 14 36 28"
src="https://github.com/user-attachments/assets/bff3f9d1-916a-485f-91b7-b54b477f5aac"
/>
Ref #10264
Currently, the number of hashers is always set to 1 on interactive
operating systems, which are defined as Windows, macOS, iOS, and
Android. However, with modern multicore CPUs, it does not make much
sense to limit performance so much.
For example, without this fix, a CPU with 16 cores / 32 threads is
still limited to using just a single thread to hash files per folder,
which may severely affect its performance.
For this reason, instead of using a fixed value, calculate the number
dynamically, so that it equals one-fourth of the total number of CPU
cores. This way, the value of hashes will still end up being just 1 on
a slower 4-thread CPU, but it will be allowed to take larger values when
the number of threads is higher, increasing hashing performance in the
process.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Co-authored-by: Jakob Borg <jakob@kastelo.net>
Currently, the number of hashers, with the exception of some specific
operating systems or when defined manually, equals the number of CPU
cores divided by the overall number of folders, and it does not take
into account the value of MaxFolderConcurrency at all. This leads to
artificial performance limits even when MaxFolderConcurrency is set to
values lower than the number of cores.
For example, let's say that the number of folders is 50 and
MaxFolderConcurrency is set a value of 4 on a 16-core CPU. With the old
calculation, the number of hashers would still end up being just 1 due
to the large number of folders. However, with the new calculation, the
number of hashers in this case will be 4, leading to better hashing
performance per folder.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This adds a cleanup stage to remove database files for folders that no
longer exist on startup. Folder database files were already removed when
dropping a folder, assuming that the folder database had been opened at
that point. This won't be the case though when a folder is removed from
the config when Syncthing isn't running, or when a folder is dropped and
re-migrated in a restarted migration.
This adds a temporary GUI/API server during the database migration. It
responds with 200 OK and some log output for every request. This serves
two purposes:
- Primarily, for deployments that use the API as a health check, it
gives them something positive to accept during the migration, reducing
the risk of the migration getting killed halfway through and restarted,
thus never completing.
- Secondarily, it gives humans who happen to try to load the GUI some
sort of indication of what's going on.
Obviously, anything that expects a well-formed API response at this
stage is still going to fail. They were already failing though, as we
didn't even listen at this point before.
Two things:
- We could run into a write error, which would block the progress
forever without an error. This because the writer routine exited, while
the reader was just blocked on sending to it.
- After a failed migration, inserts could fail with unique index
constraint errors because we are reusing the sequence numbers from the
original database. Add a drop folder to the start of migration to handle
this.
Additionally, the drop folder will clear out broken database files due
to killed migrations.
### Purpose
This was lost / replaced when introducing the "version" command.
However, the documentation still lists the flag - actually under the
serve command, but that can be omitted. Common convention for CLI
programs is to accept it as a flag.
### Testing
```
$ bin/syncthing --help
Usage: syncthing <command> [flags]
Flags:
-h, --help Show context-sensitive help.
-C, --config=PATH Set configuration directory (config and keys) ($STCONFDIR)
-D, --data=PATH Set data directory (database and logs) ($STDATADIR)
-H, --home=PATH Set configuration and data directory ($STHOMEDIR)
--version Show current version, then exit
Commands:
serve Run Syncthing (default)
cli Command line interface for Syncthing
browser Open GUI in browser, then exit
decrypt Decrypt or verify an encrypted folder
device-id Show device ID, then exit
generate Generate key and config, then exit
paths Show configuration paths, then exit
upgrade Perform or check for upgrade, then exit
version Show current version, then exit
debug Various debugging commands
install-completions Print commands to install shell completions
Run "syncthing <command> --help" for more information on a command.
```
```
$ bin/syncthing --version
syncthing v2.0.3-dev.2.g0f47e944-restore-version-flag "Hafnium Hornet" (go1.24.0 linux-amd64) acolomb@riddo 2025-08-18 19:25:31 UTC
```
### Documentation
Already / *still* listed in the docs under Command Line Operation.
Avoid:
/_/GOROOT/src/os/user/cgo_lookup_cgo.go:45:(.text+0x54): warning: Using
'getgrgid_r' in statically linked applications requires at runtime the
shared libraries from the glibc version used for linking
and
/tmp/go-build/cgo-gcc-prolog:60:(.text+0x40): warning: Using
'getaddrinfo' in statically linked applications requires at runtime the
shared libraries from the glibc version used for linking
I added these tags as part of the big database PR, but I forget why. I
think it came from an attempt at a static binary using the Go-based
SQLite packages, but that's not the primary build anymore anyway. We can
remove this and go back to the standard resolvers, which gives better
support for primarily Windows and macOS special resolution methods...
chore(gui): remove redundant "authenticated" conditions from Actions
menu (#10235)
Due to previous code changes, the whole Actions menu is only available
when the user is logged in. As such, there is no reason to have the same
ng-if="authenticated" condition repeated in other items belonging to it.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This removes the `debugging` bool under GUI configuration, and two no
longer relevant development endpoints: `httpmetrics` (which I can't
imagine anyone using for anything -- if we need such metrics today, the
right place is the Prometheus exported metrics) and the `peerCompletion`
endpoint (previously used by integration tests).
The debugging bool initially enabled just those two endpoints, which are
not for end users. Then we added profiling and support bundles, which
are very useful indeed for end users to access, and they were hidden
behind the same debug flag. I don't see any reason for keeping that flag
now that these methods are more generally useful.
https://github.com/syncthing/docs/pull/949
This updates our logging framework from legacy freetext strings using
the `log` package to structured log entries using `log/slog`. I have
updated all INFO or higher level entries, but not yet DEBUG (😓)... So,
at a high level:
There is a slight change in log levels, effectively adding a new warning
level:
- DEBUG is still debug (ideally not for users but developers, though
this is something we need to work on)
- INFO is still info, though I've added more data here, effectively
making Syncthing more verbose by default (more on this below)
- WARNING is a new log level that is different from the _old_ WARNING
(more below)
- ERROR is what was WARNING before -- problems that must be dealt with,
and also bubbled as a popup in the GUI.
A new feature is that the logging level can be set per package to
something other than just debug or info, and hence I feel that we can
add a bit more things into INFO while moving some (in fact, most)
current INFO level warnings into WARNING. For example, I think it's
justified to get a log of synced files in INFO and sync failures in
WARNING. These are things that have historically been tricky to debug
properly, and having more information by default will be useful to many,
while still making it possible get close to told level of inscrutability
by setting the log level to WARNING. I'd like to get to a stage where
DEBUG is never necessary to just figure out what's going on, as opposed
to trying to narrow down a likely bug.
Code wise:
- Our logging object, generally known as `l` in each package, is now a
new adapter object that provides the old API on top of the newer one.
(This should go away once all old log entries are migrated.) This is
only for `l.Debugln` and `l.Debugf`.
- There is a new level tracker that keeps the log level for each
package.
- There is a nested setup of handlers, since the structure mandated by
`log/slog` is slightly convoluted (imho). We do this because we need to
do formatting at a "medium" level internally so we can buffer log lines
in text format but with separate timestamp and log level for the API/GUI
to consume.
- The `debug` API call becomes a `loglevels` API call, which can set the
log level to `DEBUG`, `INFO`, `WARNING` or `ERROR` per package. The GUI
is updated to handle this.
- Our custom `sync` package provided some debugging of mutexes quite
strongly integrated into the old logging framework, only turned on when
`STTRACE` was set to certain values at startup, etc. It's been a long
time since this has been useful; I removed it.
- The `STTRACE` env var remains and can be used the same way as before,
while additionally permitting specific log levels to be specified,
`STTRACE=model:WARN,scanner:DEBUG`.
- There is a new command line option `--log-level=INFO` to set the
default log level.
- The command line options `--log-flags` and `--verbose` go away, but
are currently retained as hidden & ignored options since we set them by
default in some of our startup examples and Syncthing would otherwise
fail to start.
Sample format messages:
```
2009-02-13 23:31:30 INF A basic info line (attr1="val with spaces" attr2=2 attr3="val\"quote" a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with grouped values (attr1=val1 foo.attr2=2 foo.bar.attr3=3 a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with grouped values via logger (foo.attr1=val1 foo.attr2=2 a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with nested grouped values via logger (bar.foo.attr1=val1 bar.foo.attr2=2 a=a log.pkg=slogutil)
2009-02-13 23:31:30 WRN A warning entry (a=a log.pkg=slogutil)
2009-02-13 23:31:30 ERR An error (a=a log.pkg=slogutil)
```
---------
Co-authored-by: Ross Smith II <ross@smithii.com>
Based on the discussion in
https://forum.syncthing.net/t/towards-syncthing-2-0/24072/35 This PR
adds the ability for Windows users to use the pipe character (|) to
escape the metacharacters *, ?, [, and { in .stignore files.
Additionally, this PR adds the ability for the user to set the escape
character to backslash, or any character they want, by adding a line in
the form:
#escape=X
(where X is any single rune), to the top of an .stignore file.
This would allow users to use the same .stignore file across platforms,
by simply adding
#escape=\
to the top of the file.
### Testing
All tests pass in CI.
### Documentation
See https://github.com/syncthing/docs/pull/919Fixes#10057: Support escaping in .stignore files on Windows
Fixes#7547: Ignore pattern with \[ and \] does not work
Prior to this fix, the folder would only get marked as "syncing" once we
started downloading data from the network. However in some cases there
will be a lot of data that can be reused locally and we spend
significant time copying blocks before downloading anything; in that
case, the folder would appear as "preparing to sync" while it was in
fact moving lots of data.
This fixes that, making it "syncing" as soon as it begins either copying
or downloading data.
### Purpose
Uses recommended pattern for slice pools to avoid copying the slice
struct, suggested by the linter and actually used in the go stdlib, for
example in `net/http/h2_bundle.go`.
Currently, the Revert Local Changes button for Receive Only folders, and
the Delete Unexpected Items button for Receive Encrypted folders buttons
are shown even when the folder is already performing other operations.
Because of the above, pressing the button seems to have no effect, as
its operation can only proceed after the previous operations have
completed. This confuses the user, who then may keep trying to press the
buttons again and again with no visible result.
Therefore, show the two buttons only when the folders are actually idle,
without performing other operations at the same time. This change makes
them behave similarly to the Override Changes button, which is also only
displayed for Send Only folders when they are idle.
Signed-off-by: Tomasz Wilczyński twilczynski@naver.com
Update the jQuery Fancytree Plugin to the newest version. Apart from
keeping it up-to-date out of principle, this may also help with further
investigation of issue #10155, which is related to the plugin.
Signed-off-by: Tomasz Wilczyński twilczynski@naver.com
chore(gui): Fix "Shut Down" spelling in Actions
Currently, the word is written as "Shutdown". However, similarly to
"Log Out", it is used as a verb here, thus it should be written as two
separate words, i.e. "Shut Down".
Signed-off-by: Tomasz Wilczyński twilczynski@naver.com
Add a wrapper that uses anet on Android, but net on other platforms.
### Purpose
Fixes
https://forum.syncthing.net/t/workaround-for-android-local-discovery/20403/12
### Testing
Run two Syncthing instances with Global Discovery disabled. Pair them
with each other, don't hardcode their addresses, and verify they
connect.
We've always, since the introduction of conflicts, had the policy that
deletes lose against any other change, for safety's sake. This is a
problem, however, because it means the sort order of versions is not a
total order.
That is, given two versions `A` and `B` that are currently in conflict,
we will sort them in a given order (let's say `A, B`, so `A < B` for
ordering purposes: we say "A wins over B" or "A is newer than B") and
consider the first in the list the winner. The loser (who has `B` on
disk) will process the conflict at some point and move the file to a
conflict copy and announce `A'` as the resolved conflict. The winner
(with `A` on disk) doesn't do anything.
However, if `A` is deleted the ordering changes. We still have `A < B`
and, of course, `Adel < A` (this is not even a conflict, just linear
order). In most sane systems this would imply the ordering `Adel < A <
B`, however in our case we in fact have `B < Adel` because any version
wins over a deleted one, so there is no logical ordering at all of the
files at this point. `Adel < A < B < Adel ???` In practice the deleted
version may end up at the head or the tail of the list, depending on the
order we do the compares.
Hence, at this point, "whatever" happens and it's not guaranteed to make
any sense. 😬
I propose that we resolve this my simply letting deletes be versions
like anything else and maintain a total ordering based on just version
vectors with the existing tie breakers like always. That means a delete
can win in a conflict situation, and the result should be that the file
is moved to a conflict copy on the losing device. I think this retains
the data safety to almost the same degree as previously, while removing
probably an entire class of strange out of sync bugs...
---
(A potential wrinkle here is that, ideally, we wouldn't even create the
conflict copy when the delete and the losing version represent the same
data -- same as when we handle normal modification conflicts. However,
the deleted FileInfo doesn't carry any information on what the contents
were, so we can't do that right now. A possible future extension would
be to carry the block list hash of the deleted data in the deleted
FileInfo and use that for this purpose, but I don't want to complicate
this PR with that. The block list hash itself also isn't a
protocol-defined thing at the moment, it's something implementation
dependent that we just use locally.)
### Purpose
As discussed on the forum:
https://forum.syncthing.net/t/reviving-nat-pmp-in-v2-x-on-android-14/24554
TL;DR
Android 14+ only lets java code get the gateway IPv4 address which is
used in SyncthingNative’s NAT-PMP feature.
So I’ve added the java code to the wrapper, got the router IP address
and feeded it to SyncthingNative by setting the env var
“FALLBACK_NET_GATEWAY_IPV4”.
This revives the NAT feature:
> [Z36WU] INFO: Detected 1 NAT service
### Testing
Local build and test via Android emulator (AVD 15).
This makes a couple of backwards compatible changes to the
ClusterConfig:
- Remove the `ignore_permissions` and `ignore_delete` booleans which
we've never read or used for anything
- Remove the `disable_temp_indexes` boolean and option entirely. We did
use this one, and about 1% of users have set the option. The only thing
it does is inhibits sending of periodical DownloadProgress messages
while downloading data, which is a minuscule bandwidth optimisation
given that we're already sending data at the time.
- Change the `read_only` boolean (which indicated send-only folders) to
an enum `FolderType`, where the values zero and one match the existing
usage. Again, we don't actually use this value, but I can see that we
might want to and then it makes more sense for it to be more
comprehensive.
- Change the `paused` boolean to an enum `StopReason`, where zero
indicates not stopped and one indicates paused, exactly the same wire
representation as previously but leaves space for additional stop
reasons (errors etc).
### Purpose
Filesystem watcher errors didnt have any whitespace between the share
name and the error message, making it hard to read. A simple colon and
whitespace solves this issue
I lately wanted some photos on my phone, and watched them sync
excrutiatingly slowly. I am used to android being slow, but not that
slow. This restriction caught my eye and I increased it beyond the
limit (didn't spot it at first), and I did see a clear improvement. Of
course as always with such a one-off test, I might also have
hallucinated it, but it seems plausible with the slow thing in android
being some layer between the actual filesystem and apps.
Also increase the max limit, mostly just because I don't see any reason
to restrict it that low - not that I have a particular reason to want
more.
I also changed the xml default to 0: The `prepare` code will change it
to the actual default - no need to change that anymore if we change the
default in the future.
Before:
- Local discovery on Android 10+ is broken. The phone receives local
discovery packets from other devices running Syncthing on the same
network, e.g. a computer. But it doesn't send its own local discovery
packets.
- Startup of the beacon/broadcast.go and beacon/multicast.go "services"
subsequently fail, see the log entries of "service.go" with "2 of 2
failures, backing off".
Root cause:
- Android 10+ restricts determining the network interfaces for privacy
reasons. The interfaces and IP addresses cannot be determined.
- There's a bug in the go "net" library. I can actually get the
interfaces, but the fix was not implemented by the go team.
Workaround:
- The "community" found a workaround by creating a light wrapper around
"net" called "anet" library.
- "anet" adjusts the behaviour on Android 10+ and gets the interfaces
plus their IP addresses, as required by Syncthing.
After:
- By using the "anet" lib, Syncthing is able to get the interface ip
addresses and put them into the "AllAddresses" string array.
- The "AllAddresses" string array is then announced on the local
discovery multicast and broadcast packets, if enabled in Syncthing's
config.
- By correctly getting the interfaces and IP addresses using "anet" in
"beacon/broadcast.go" and "beacon/multicast.go", the services start up
fine again.
Verification:
- I've built "libSyncthingNative.so" with this PR applied for Android
and put it into Syncthing-Fork v1.29.7.5 for testing. My two phones,
Android 10 and Android 15 (arm64-v8a) immediately discovered each other
using local discovery.
- I can see the "sent XX bytes" and "recv XX bytes" on both phones in
the log filtering for "SyncthingNativeCode" :-).
Personal note:
- Please go light on me, and, if it's not demanded too much of your
time, please help me on this. I am no go programmer. Most things you
think are easy or common sense aren't part of my knowledge set. I'd just
like to help and hope we somehow can drive this home together to fix the
problem.
----
ref: https://github.com/Catfriend1/syncthing-android/pull/1501
ref: https://github.com/Catfriend1/syncthing-android/issues/1500
ref: https://github.com/wlynxg/anet/blob/main/interface.go &
https://github.com/wlynxg/anet/blob/main/interface_android.go
With that fix, I can see the broadcast/multicast lines again and my
phone can be discovered by other phones running the Syncthing app which
wasn't possible before on Android 10+.
```
[ET76H] .346892 broadcast.go:107: DEBUG: sent 185 bytes to 192.168.x.255:21027
[ET76H] .347114 multicast.go:86: DEBUG: sent 185 bytes to [ff12::8384]:21027 on wlan0
```
---------
Co-authored-by: Marcus B Spencer <marcus@marcusspencer.us>
Fixes#7403.
Tested by enabling UPnP on the router, and checking on the router page
that the external ports of the UDP mappings match what is shown in the
logs and the internal ports matching the QUIC listening port.
Only Require either matching UID & GID OR matching Names.
If the 2 devices have a different Name => UID mapping, they can never be
totaly equal. Therefore when syncing we try matching the Name and fall
back to the UID. However when scanning for changes we currently require
both the Name & UID to match. This leads to forever having out of sync
files back and forth, or local additions when receive only.
This patch does not change the sending behavoir. It only change what we
decide is equal for exisiting files with mismapped Name => UID,
The added testcases show the change: Test 1,5,6 are the same as current.
Test 2,3 Are what change with this patch (from false to true). Test 4 is
a subset of test 2 they is currently special cased as true, which does
not chnage.
Co-authored-by: Jakob Borg <jakob@kastelo.net>
While it doesn't hurt, it's unnecessary since the big protobuf
modernisation, that also introduced types separate from the generated
ones for internal use. Those fields are already dropped when converting
to the wire in protocol.
This updates our key generation to use Ed25519 keys/certificates for
sync connections. Certificates for browser use remain ECDSA for wider
compatibility.
Ed25519 is more modern and has fewer concerns for the future than the
ECDSA curves we used previously. It is supported from Go 1.13 and
forwards, which is Syncthing 1.3.0 (October 2019).
### Purpose
Locally, on Windows 11, and on the windows-2025 GitHub runner (go 1.23
and 1.24), the `TestCopyRange` test is failing with `The request is not
supported.`
On windows-2022 and windows-2019:
```go
err == syscall.ENOTSUP
```
worked, but on Windows 11 and windows-2025, we need:
```go
errors.Is(err, errors.ErrUnsupported)
```
### Testing
Tested on Windows 11, windows-2019, windows-2022, and
[windows-2025](https://github.com/rasa/syncthing/actions/runs/15525123437/job/43703630634#step:7:2811).
* v2: (62 commits)
build: add dependency for next-version script
docs: add release note mention of platforms no longer built
build: streamline gathering of facts, checkouts (#10158)
build: more resilient pushes to releases
chore(etc): add option dash to upstart config
chore(fs): linter complaints
chore(model): the easier linter complaints
chore(internal): linter complaints
chore(sqlite): linter complaints
build: allow v2 into APT candidate channel
docs: link to Docker image, APT, in release notes
build: refactor builds for forks/PRs
build: use same GitHub token for releases as for translation etc pushes
refactor(sqlite): move deleted flag into logical order in schema
feat(config): enable multiple connections by default (#10151)
docs: mention subcommands in release notes, use for all 2.0 releases
docs: adjust release notes for v2.0.0
docs: add relnotes for v2.0.0
build: upgrade setup-zig action (#10134)
fix(versioner): correct fs creation in test
...
* main:
docs: link to Docker image, APT, in release notes
build: also create relaysrv and discosrv releases
fix(stupgrades): return latest stable & pre for each major
fix(syncthing): avoid writing panic log to nil fd (#10154)
### Purpose
This change fixes a logical bug in the panic log writing where we could
end up writing to a uninitialized file descriptor.
On the very first iteration, `panicFd` is nil. We enter the if `panicFd
== nil { … }` block, check for “panic:” or “fatal error:”, and if
neither matches, we skip instantiating `panicFd` altogether. However,
immediately after, still within `if panicFd == nil { … }`, we call
`panicFd.WriteString("Panic at ...")`. But `panicFd` would in this case
be `nil`, which will cause a run‐time panic.
It's not clear to me why panicFd is only initialized if the lines start
with "panic:" or "fatal error:" so I've left that logic untouched. With
this change we at least avoid the risk of writing to a nil
filedescriptor.
## Authorship
Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
* main:
feat(config): expose folder and device info as metrics (fixes#9519) (#10148)
chore: add issue types to GitHub issue templates
build: remove schedule from PR metadata job
chore(protocol): only allow enc. password changes on cluster config (#10145)
chore(protocol): don't start connection routines a second time (#10146)
Tihs makes it easier to use metrics based on device and folder labels,
names, and other attributes. Other metrics which are based on folder or
device ID can be joined with these info metrics to enrich their label
sets.
```
# HELP syncthing_config_device_info Provides additional information labels on devices
# TYPE syncthing_config_device_info gauge
syncthing_config_device_info{device="I6KAH76-66SLLLB-5PFXSOA-UFJCDZC-YAOMLEK-CP2GB32-BV5RQST-3PSROAU",introducer="false",name="s1",paused="false",untrusted="false"} 1
# HELP syncthing_config_folder_info Provides additional information labels on folders
# TYPE syncthing_config_folder_info gauge
syncthing_config_folder_info{folder="default",label="The default folder",path="s2",paused="false",type="sendreceive"} 1
```
With this you can e.g. query for
```
syncthing_connections_active * on(device) group_left syncthing_config_device_info
```
Fixes#9519Closes#10074Closes#10147
This changes the default number of connections from one to three (one
metadata + two data connections). This should give some advantages of
multiple connections, while also not being an overwhelming change for
larger installations. (Though those may need to tweak their settings
anyway, as always.)
In practice we already always call SetPassword and ClusterConfig
together. However it's not just "sensible" to do that, it's required: If
the passwords change, the remote device needs to know about that to
check that the enc. setup is valid/consistent (e.g. tokens match,
folder-type is appropriate, ...).
And with the passwords set later, there's no point in adding them as
part of creating a new connection.
This is a "followup" (if one can call it that 4 years later :) ) to
resp. fix for the following commit:
924b96856f
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This adds a file that will be prepended to release notes (tag messages,
GitHub releases, forum posts) for v1 releases. I'd like there to be
something there to flag that things are going to change.
* main:
refactor: use slices package for sorting (#10136)
build: handle multiple general release notes
build: no need to build on the branches that just trigger tags
* main:
build: use specific token for pushing release tags
fix(gui): update `uncamel()` to handle strings like 'IDs' (fixes#10128) (#10131)
refactor: use slices package for sort (#10132)
build: process for automatic release tags (#10133)
chore(gui, man, authors): update docs, translations, and contributors
> ⚠️ resubmission targeting `main` instead of `v2`
### Purpose
Updates `uncamel()` function in
[uncamelFilter.js](https://github.com/syncthing/syncthing/blob/v2/gui/default/syncthing/core/uncamelFilter.js)
to fix camelCase conversion edge cases, see #10128
This adds an array called `reservedStrings` which will be printed as-is,
e.g. `IDs`, `LAN` etc. I pre-populated this with what I believe makes
sense, but of course this is easily updated.
### Testing
I compiled all the config variables I could find in
`syncthing/lib/config/*configuration.go` and tested this new function
against them. Everything seemed to pass.
### Screenshot
The sort package is still used in places that were not trivial to
change. Since Go 1.21 slices package can be uswed for sort. See
https://go.dev/doc/go1.21#slices
### Purpose
Make some progress with the migration to a more up-to-date syntax.
* main:
fix(syncthing): ensure both config and data dirs exist at startup (fixes#10126) (#10127)
fix(versioner): fix perms of created folders (fixes#9626) (#10105)
refactor: use slices.Contains to simplify code (#10121)
As suggested in the linked issue, I've updated the versioner code to use
the permissions of the corresponding directory in the synced folder,
when creating the folder in the versions directory
### Testing
- Some tests are included with the PR. Happy to add more if you think
there are some edge-cases that we're missing.
- I've tested manually on linux to confirm the permissions of the
created directories.
- I haven't tested on Windows or OSX (I don't have access to these OS)
The copier routine refactor resulted in bad buffer pool handling,
putting a buffer back into the pool twice. This simplifies and removes
the danger prone Upgrade() method.
### Testing
Change the `auditEnabled` option and you should get a prompt in the Web
GUI.
Restart and change the `auditFile` option, and you should get that same
prompt.
The prompt you should get is shown in the screenshots below.
### Screenshots
Co-authored-by: Jakob Borg <jakob@kastelo.net>
### Purpose
Setting default configuration was not working properly since the
defaults struct is not deeply copied.
### Testing
Try running commands to change default configuration and either inspect
`config.xml` or `/rest/config` result to see the applied changed.
Example:
```
./syncthing cli config defaults folder versioning params set keep 5
```
* main:
chore(gui, man, authors): update docs, translations, and contributors
feat(gui): close a modal when pressing ESC after switching modal tabs (fixes#9489) (#10092)
Flattened the copier code more. Also removing and moving some
parameters/return values to simplify things. Generally rely less on
return values, e.g. by handling errors right away and using `state` to
do the right thing (e.g. abort on failure).
Supposed to be a refactor without any behaviour changes, except for
fixing a tiny regression on folder order: We used to try copying from
the same folder first, but lost that property at some point (also sent a
PR fixing only that, I'd merge that first making this refactor only).
### Purpose
As stated in #9489 after clicking on a tab link to switch tabs in a
modal you can no longer close the modal through clicking the ESC key
unless you click anywhere on the modal to focus on it again.
### Testing
- Click on a modal that has tabs like "Settings" or "Add Folder" and
switch tabs then click on ESC.
- Check if clicking outside of a modal in the backdrop should still
close the modal.
### Demo
https://github.com/user-attachments/assets/a010db9a-72f7-4160-a7db-ddfebffb4834
Where `folderFilesystems` and `folders` is built, there's a comment
spelling out the purpose: To have the same folder first, as that's the
most likely to get hits. Plus a copy is possibly more efficient than
from another folder, e.g. if that's on a different filesystem. We lost
that behaviour during some unrelated change.
(Also sneaking in a comment fix on yesterdays change.)
This is a draft because I haven't adjusted all the tests yet, I'd like
to get feedback on the change overall first, before spending time on
that.
In my opinion the main win of this change is in it's lower complexity
resp. fewer moving parts. It should also be faster as it only does one
query instead of two, but I have no idea if that's practically
relevant.
This also mirrors the v1 DB, where a block map key had the name
appended. Not that this is an argument for the change, it was mostly
reassuring me that I might not be missing something key here
conceptually (I might still be of course, please tell me :) ).
And the change isn't mainly intrinsically motivated, instead it came
up while fixing a bug in the copier. And the nested nature of that code
makes the fix harder, and "un-nesting" it required me to understand
what's happening. This change fell out of that.
After opening the database, we performed some checks, such as whether
the migration had already been successfully completed. If so, the
function returned immediately, and the database was not closed.
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
* main:
fix(strelaysrv): make the session limiter session-dependent (fixes#10072) (#10073)
build: artifact uploads destination OCI
chore(gui, man, authors): update docs, translations, and contributors
chore(gui): use go list --deps for dependency list (#10071)
### Purpose
Fix https://github.com/syncthing/syncthing/issues/9336
The `emitLoginAttempt` function now checks for the presence of an
`X-Forwarded-For` header. The IP from this header is only used if the
connecting host is either on loopback or on the same LAN.
In the case of a host pretending to be a proxy, we'd still have both IPs
in the logs, which should make this much less critical from a security
standpoint.
### Testing
1. directly via localhost
2. via proxy an localhost
#### Logs
```
[3JPXJ] 2025/04/11 15:00:40 INFO: Wrong credentials supplied during API authorization from 127.0.0.1
[3JPXJ] 2025/04/11 15:03:04 INFO: Wrong credentials supplied during API authorization from 192.168.178.5 proxied by 127.0.0.1
```
#### Event API
```
{
"id": 23,
"globalID": 23,
"time": "2025-04-11T15:00:40.578577402+02:00",
"type": "LoginAttempt",
"data": {
"remoteAddress": "127.0.0.1",
"success": false,
"username": "sdfsd"
}
},
{
"id": 24,
"globalID": 24,
"time": "2025-04-11T15:03:04.423403976+02:00",
"type": "LoginAttempt",
"data": {
"proxy": "127.0.0.1",
"remoteAddress": "192.168.178.5",
"success": false,
"username": "sdfsd"
}
}
```
### Documentation
https://github.com/syncthing/docs/pull/907
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This cleans up the option to allow old TLS 1.2 sync connections. The
flag existed for compatibility with old Syncthing versions that don't
support TLS 1.3, which is approximately Syncthing 1.2.2 (September 2019)
and older. ("Approximately" because it depends on the Go version it's
built with and that's when we switched to building with Go 1.13.)
Ref #10062 because it reminded me this exists.
* infrastructure:
feat(stdiscosrv): configurable desired not-found rate
chore(blobs): generalised blob storage
chore(stdiscosrv): path style s3
feat(ursv): add os/arch/distribution metric
chore(strelaypoolsrv): limit number of returned relays
build(infra): run in Docker environment for pushes
chore(stupgrades): expose latest release as a metric
* main:
build: push artifacts to Azure (#10044)
fix(syncthing): use separate lock file instead of locking the certificate (fixes#10053) (#10054)
fix(syncthing): use separate lock file instead of locking the certificate (fixes#10053) (#10054)
* release-1.29.5:
build: push artifacts to Azure (#10044)
fix(syncthing): use separate lock file instead of locking the certificate (fixes#10053) (#10054)
* main:
fix(gui): fix previous commit
fix(gui): mark unseen disconnected devices as inactive (#10048)
fix(strings): differentiate setup(n) and set(v) up (#10024)
chore(fs): changes to allow Filesystem to be implemented externally (#10040)
chore(config): resolve primary STUN servers via SRV record (fixes#10029) (#10031)
build: push artifacts to Azure (#10044)
chore(gui, man, authors): update docs, translations, and contributors
Currently, the "Disconnected (Inactive)" status is only given to devices
that have not been seen for 7 days or longer. However, this is not the
case when adding a new device, or after resetting the database. Those
devices are only marked as "Disconnected", and they will stay like that
even if a long time passes without any connectivity. Moreover, the lack
of an "Inactive" status may confuse the user to believe that their
disconnect is only temporary.
For this reason, always mark devices that have not been seen yet as
"Disconnected (Inactive)".
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
### Purpose
The `fs.Filesystem` interface contains two parts that cannot be
implemented externally because they are private:
* `filesystemWrapperType`: this PR changes `unwrapFilesystem` to
downcast to a specific concrete type
* `underlying`: this PR simply moves it to an unexported interface
### Testing
Regular tests pass.
This adds a simple delay to the process for starting the pull, by
default one second. In practice this means we're likely to wait for
initial index transfer, or multiple messages sent as part of a larger
change. This is better because we're more likely to have the whole
change for the purpose of handling renames etc, and also it's more
efficient to do one larger puller iteration instead of multiple while
also processing changes.
It does however introduce a certain amount of delay into the sync
process, so it can be tuned down or turned off entirely.
This changes the database structure to use one database per folder, with
a small main database to coordinate. Reverts the prior change to buffer
all files in memory when pulling, meaning there is now a phase where the
WAL file will grow significantly, at least for initial sync of folders
with many directories.
---------
Co-authored-by: bt90 <btom1990@googlemail.com>
* main:
fix(config): properly apply defaults when reading folder configuration (#10034)
chore(model): add metric for total number of conflicts (#10037)
build: replace underscore in Debian version (#10032)
The workflow building Debian packages chokes on branches containing
underscores:
```
{:timestamp=>"2025-04-03T10:31:46.749835+0000", :message=>"Invalid package configuration: The version looks invalid for Debian packages. Debian version field must contain only alphanumerics and . (period), + (plus), - (hyphen) or ~ (tilde). I have '1.29.5~dev.13.ga38df11f~srv_stun' which which isn't valid.", :level=>:error}
```
This replaces the offending `_` with a `~` which should yield a valid
version.
This reduces the number of file entries we carry in the database,
sometimes significantly. The downside is that if a file is deleted while
a device is offline, and that device comes back more than the cutoff
interval (six months) later, those files will get resurrected at some
point.
The current limit is far too low for our workloads. Perhaps we should
aim even higher than the 64MiB this patch proposes?
Citing the sqlite docs:
> [...] after committing a transaction the rollback journal file may
remain in the file-system. **This increases performance for subsequent
transactions since overwriting an existing file is faster than append to
a file**, but it also consumes file-system space.
tl;dr: if the limit is too low, we're shooting ourselves in the foot in
terms of performance.
(v2 change)
This cleans up the command line parsing a little:
- Remove the hack for supporting legacy single-dash long options (e.g.
`-home`), thus enabling actual short options
- Move legacy imperative flags from under the serve command into
separate commands, e.g. `syncthing serve --paths` to see the paths list
is now `syncthing paths`, `syncthing --upgrade-check` is now `syncthing
upgrade --check`
- Add environment variable support for all remaining flags for the
`serve` command (with one exception, left for the reader to discover),
as these are now all modifiers and not imperative
```
% syncthing --help
Usage: syncthing <command>
Flags:
-h, --help Show context-sensitive help.
Commands:
serve Run Syncthing (default)
cli Command line interface for Syncthing
browser Open GUI in browser, then exit
decrypt Decrypt or verify an encrypted folder
device-id Show device ID, then exit
generate Generate key and config, then exit
paths Show configuration paths, then exit
upgrade Perform or check for upgrade, then exit
version Show current version, then exit
debug Various debugging commands
install-completions Print commands to install shell completions
Run "syncthing <command> --help" for more information on a command.
```
```
% syncthing serve --help
Usage: syncthing serve [flags]
Run Syncthing (default)
Flags:
-h, --help Show context-sensitive help.
-C, --config=PATH Set configuration directory (config and keys) ($STCONFDIR)
-D, --data=PATH Set data directory (database and logs) ($STDATADIR)
-H, --home=PATH Set configuration and data directory ($STHOMEDIR)
--allow-newer-config Allow loading newer than current config version ($STALLOWNEWERCONFIG)
--audit Write events to audit file ($STAUDIT)
--auditfile=PATH Specify audit file (use "-" for stdout, "--" for stderr) ($STAUDITFILE)
--db-maintenance-interval=8h Database maintenance interval ($STDBMAINTINTERVAL)
--gui-address=URL Override GUI address (e.g. "http://192.0.2.42:8443") ($STGUIADDRESS)
--gui-apikey=API-KEY Override GUI API key ($STGUIAPIKEY)
--no-console Hide console window ($STHIDECONSOLE)
--logfile=PATH Log file name (see below) ($STLOGFILE)
--logflags=BITS Select information in log line prefix (see below) ($STLOGFLAGS)
--log-max-old-files=N Number of old files to keep (zero to keep only current) ($STNUMLOGFILES)
--log-max-size=BYTES Maximum size of any file (zero to disable log rotation) ($STLOGMAXSIZE)
--no-browser Do not start browser ($STNOBROWSER)
--no-default-folder Don't create the "default" folder on first startup ($STNODEFAULTFOLDER)
--no-port-probing Don't try to find free ports for GUI and listen addresses on first startup ($STNOPORTPROBING)
--no-restart Do not restart Syncthing when exiting due to API/GUI command, upgrade, or crash ($STNORESTART)
--no-upgrade Disable automatic upgrades ($STNOUPGRADE)
--paused Start with all devices and folders paused ($STPAUSED)
--unpaused Start with all devices and folders unpaused ($STUNPAUSED)
--verbose Print verbose log output ($STVERBOSE)
--debug-gui-assets-dir=PATH Directory to load GUI assets from ($STGUIASSETS)
--debug-perf-stats Write running performance statistics to perf-$pid.csv (Unix only) ($STPERFSTATS)
--debug-profile-block Write block profiles to block-$pid-$timestamp.pprof every 20 seconds ($STBLOCKPROFILE)
--debug-profile-cpu Write a CPU profile to cpu-$pid.pprof on exit ($STCPUPROFILE)
--debug-profile-heap Write heap profiles to heap-$pid-$timestamp.pprof each time heap usage increases ($STHEAPPROFILE)
--debug-profiler-listen=ADDR Network profiler listen address ($STPROFILER)
--debug-reset-delta-idxs Reset delta index IDs, forcing a full index exchange
...
```
Similarly to #10009, we will remove some discontinued STUN servers,
except instead of being the official primary server, it's some
unofficial secondary STUN servers.
### Testing
Use a STUN client (like [`pystun3`](https://pypi.org/project/pystun3))
to probe that the removed STUN servers are inactive.
### Documentation
syncthing/docs#902
The mechanism for primary STUN servers, is still intact, in case this
gets retried with a different domain.
### Purpose
As seen in [stun.syncthing.net doesn’t resolve
anymore](https://forum.syncthing.net/t/stun-syncthing-net-doesnt-resolve-anymore/24075/2?u=marbens)
on the forums, stun.syncthing.net has been shut down, so I think it's
probably a good idea to remove it.
### Testing
1. Have two or more devices
2. Disable Relaying
3. Have no Internet ports open on either end for incoming connections
trigger STUN)
4. Enable the `stun` debugging facility in the Actions -> Logs ->
Debugging Facilities
5. Verify that it doesn't output something like this within a few
seconds:
```
2025-03-30 05:51:32 Enabled debug data for "stun"
2025-03-30 05:51:47 Starting stun for Stun@udp://[::]:22000
2025-03-30 05:51:47 Running stun for Stun@udp://[::]:22000 via stun.syncthing.net:3478
2025-03-30 05:51:47 Stun@udp://[::]:22000 stun addr resolution on stun.syncthing.net:3478: lookup stun.syncthing.net: no such host
```
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
### Purpose
In the GUI, the device ID validation was case-sensitive and didn’t
account for dash variations, which allowed users to enter an existing
device ID without receiving proper feedback.
This fix ensures the ID is validated in its canonical form, thus
preventing the user from submitting the request if the device ID already
exists.
### Testing
To test this change, try adding a new device with an ID that matches an
existing device, but with a different case or dashes.
Switch the database from LevelDB to SQLite, for greater stability and
simpler code.
Co-authored-by: Tommy van der Vorst <tommy@pixelspark.nl>
Co-authored-by: bt90 <btom1990@googlemail.com>
We've had weak/rolling hashing in the code for quite a while. It was a
popular request for a while, based on the belief that rsync does this
and we should too. However, the benefit is quite small; we save on
average about 0.8% of transferred blocks over the population as a whole:
<img width="974" alt="Screenshot 2025-03-28 at 17 09 02"
src="https://github.com/user-attachments/assets/bbe10dea-f85e-4043-9823-7cef1220b4a2"
/>
This would be fine if the cost was comparably low, however the downside
of attempting rolling hash matching is that we (by default) do a
complete file read on the destination in order to look for matches
before we starting pulling blocks for the file. For any larger file this
means a sometimes long, I/O-intensive pause before the file starts
syncing, for usually no benefit.
I propose we simply rip off the bandaid and save the effort.
Currently, some options are automatically enabled or disabled depending
on the folder type. However, there is no explanation in the GUI on why
the options are like that. Thus, add short explanatory notes to each
case, where the option is either disabled or enabled according to the
current folder type.
### Purpose
This exposes four methods from `Model` through `Internals`. It allows
apps like Synctrain to obtain information about local/remote need and
sync progress.
### Testing
No testing seems necessary, functions are exported verbatim.
### Screenshots
N/a
### Documentation
Not public API, I am aware this interface may change at any time.
## Authorship
OK.
Co-authored-by: Ross Smith II <ross@smithii.com>
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This allows users to easily disable nightly builds in their forks,
simply by disabling the
build-nightly action.
### Testing
I tested it in my fork, and it works.
### Purpose
Path autocompletion wasn't working when using `~` as a shortcut for the
home directory. The issue occurred because the tilde was expanded to
/home/user, which caused the suggestion to no longer match the input
(thus preventing the autocompletion from appearing in the suggestion
list).
To fix this, I replaced the custom `parentAndBase` function, which
handled path splitting in a more complex way, with `filepath.Split` from
the standard `path/filepath` package. This prevents tilde expansion
while keeping the expected behavior for path splitting.
### Testing
The issue has been tested manually on Linux.
### Screenshots
This is extracted from PR #9175. This deduplicates `SetPassword` calls
and makes `postAdjustGui` a single place where PR #9175 can add
another adjustment step for sanitizing changes to WebAuthn
credentials.
This also adds tests to validate that the refactored logic was not
broken.
These refactorizations were made in [PR #9175][1] to accommodate a few
new variants of authentication method and body content. On request from
reviewers, this PR extracts it as a smaller refactorization to review in
isolation.
### Purpose
This extracts a shared `httpRequest` base function from `httpGet` and
`httpPost`, which will be used in PR #9175 for new helper functions
`httpGetCsrf` (hiding all optional parameters except the CSRF token),
`httpPostCsrf` (same) and `httpPostCsrfAuth` (hiding basic auth
parameters). A `getSessionCookie` function is also extracted from
`hasSessionCookie` and will be used to test that concurrent WebAuthn
authentications result in separate sessions (indicated by different
session cookies).
### Purpose
On iOS, the FSEvents API for watching files (also used on macOS) is not
available, but `kqueue` is. This PR ensures `kqueue` support is built on
iOS instead of the FSEvents based watcher implementation.
Before this PR, you could already use the `kqueue` build option to force
its usage. Unfortunately `gomobile`, the tool that I use to build
Syncthing for iOS and macOS for Synctrain, does not support setting
different build flags for iOS and macOS (unless I build separately for
each, which is a bit of a hassle because XCode nonsense). I am assuming
there are good reasons to support FSEvents even though `kqueue` is also
available on macOS (but I'm not sure why?). I do know FSEvents has been
working fine for me on macOS so it seems best to use FSEvents on macOS
and kqueue on iOS.
Note that this also requires https://github.com/syncthing/notify/pull/4
to be merged in `synchting/notify` (until that is done, this PR will
fail to build on iOS due to `notify` still trying to link to `fsevents`
stuff when the `kqueue` build flag is not set).
### Testing
I compiled both `syncthing/notify` and syncthing with this PR applied,
and used that to successfully build the Synctrain iOS app, which after
this PR works fine and should follow up file changes a bit quicker.
### Screenshots
n/a
### Documentation
n/a
## Authorship
Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
### Purpose
This is a [new function](https://pkg.go.dev/slices@go1.21.0#Contains)
added in the go1.21 standard library, which can make the code more
concise and easy to read.
### Testing
Describe what testing has been done, and how the reviewer can test the
change
if new tests are not included.
### Screenshots
If this is a GUI change, include screenshots of the change. If not,
please
feel free to just delete this section.
### Documentation
If this is a user visible change (including API and protocol changes),
add a link here
to the corresponding pull request on https://github.com/syncthing/docs
or describe
the documentation changes necessary.
## Authorship
Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.
Signed-off-by: dashangcun <907225865@qq.com>
Co-authored-by: Jakob Borg <jakob@kastelo.net>
The requirements for Windows code signing changed in 2023, so that newly
generated certificates can only be stored in hardware modules. Luckily,
I managed to snag a three year certificate before that so it hasn't
affected us so much. Now though, it does, because our cert is expiring
in March.
This changes the code signing process for Windows to use a cloud
service, Azure Trusted Signing. This appears to work equally well and
outsources the problem entirely, while also being cheaper than the
actual certificate was to begin with. 🤷
The signing entity will be Kastelo AB and not the Syncthing Foundation,
because the latter is almost impossible to get a certificate for as it's
not a normal corporate entity whose existence can be verified, etc. This
is also how it was prior to the latest certificate; it's not ideal, but
I think it's acceptable under the circumstances.
Currently, this just results in a very ambiguous `setting metadata: lookup
failed` while it could report what it's looking up and why it failed
(not found, etc).
Marshal() was called with the read lock held, and in turn called
Created() which also takes the read lock. This is fine by itself, but
there is a risk of deadlock if another call to lock the mutex happens
concurrently, as the lock call will block the inner rlock and the outer
rlock can never become unlocked.
It's an easy fix as marshalling is guaranteed to be called with a read
lock and does not need to call any methods that read lock themselves.
This was broken in the Protobuf refactor. The reason is that with the
previous library, generated types would have a JSON marshalling method
that would automatically return strings for enums. In the current
library you need to use the jsonpb marshaller for that, but these are
hand crafted structs so we can't do that. The easy solution is to just
use strings directly, since this is an API-only type anyway.
Commit dee920a840 was merged into
Weblate's repository, but later replaced on main by
2167ce9656. This led to a merge conflict,
which this commit fixes cleanly. After merging, we can unlock Weblate
again.
The GitHub checkout action does weird stuff with tags which breaks `git
describe`. This works around that so we get proper release builds on tag
pushes.
We must skip unix sockets, fifos, etc when scanning as these are not
filetypes we can handle. Currently we return a "bug" error, which
results in the walk being aborted and the rest of the tree being
essentially invisible to Syncthing. Instead, just ignore these files and
continue onwards.
This might well be #9859 as well but I can't confirm.
When creating an initial default config, we usually probe for a free
TCP port. But when a UNIX socket is specified via the `STGUIADDRESS=`
override or the `--gui-address=unix:///...` command line syntax, parsing
that option will fail during port probing.
The solution is to just skip the port probing when the address is
determined to specify something other than a TCP socket.
### Testing
Start with a fresh home directory each time.
1. Specify a UNIX socket for the GUI (works with this PR):
TMPHOME=$(mktemp -d); ./syncthing --home=$TMPHOME
--gui-address=unix://$TMPHOME/socket
2. Specify no GUI address (probes for a free port if default is taken,
as before):
TMPHOME=$(mktemp -d); ./syncthing --home=$TMPHOME
3. Specify a TCP GUI address (probes whether the given port is taken,
as before):
TMPHOME=$(mktemp -d); ./syncthing --home=$TMPHOME
--gui-address=127.0.0.1:8385
At a high level, this is what I've done and why:
- I'm moving the protobuf generation for the `protocol`, `discovery` and
`db` packages to the modern alternatives, and using `buf` to generate
because it's nice and simple.
- After trying various approaches on how to integrate the new types with
the existing code, I opted for splitting off our own data model types
from the on-the-wire generated types. This means we can have a
`FileInfo` type with nicer ergonomics and lots of methods, while the
protobuf generated type stays clean and close to the wire protocol. It
does mean copying between the two when required, which certainly adds a
small amount of inefficiency. If we want to walk this back in the future
and use the raw generated type throughout, that's possible, this however
makes the refactor smaller (!) as it doesn't change everything about the
type for everyone at the same time.
- I have simply removed in cold blood a significant number of old
database migrations. These depended on previous generations of generated
messages of various kinds and were annoying to support in the new
fashion. The oldest supported database version now is the one from
Syncthing 1.9.0 from Sep 7, 2020.
- I changed config structs to be regular manually defined structs.
For the sake of discussion, some things I tried that turned out not to
work...
### Embedding / wrapping
Embedding the protobuf generated structs in our existing types as a data
container and keeping our methods and stuff:
```
package protocol
type FileInfo struct {
*generated.FileInfo
}
```
This generates a lot of problems because the internal shape of the
generated struct is quite different (different names, different types,
more pointers), because initializing it doesn't work like you'd expect
(i.e., you end up with an embedded nil pointer and a panic), and because
the types of child types don't get wrapped. That is, even if we also
have a similar wrapper around a `Vector`, that's not the type you get
when accessing `someFileInfo.Version`, you get the `*generated.Vector`
that doesn't have methods, etc.
### Aliasing
```
package protocol
type FileInfo = generated.FileInfo
```
Doesn't help because you can't attach methods to it, plus all the above.
### Generating the types into the target package like we do now and
attaching methods
This fails because of the different shape of the generated type (as in
the embedding case above) plus the generated struct already has a bunch
of methods that we can't necessarily override properly (like `String()`
and a bunch of getters).
### Methods to functions
I considered just moving all the methods we attach to functions in a
specific package, so that for example
```
package protocol
func (f FileInfo) Equal(other FileInfo) bool
```
would become
```
package fileinfos
func Equal(a, b *generated.FileInfo) bool
```
and this would mostly work, but becomes quite verbose and cumbersome,
and somewhat limits discoverability (you can't see what methods are
available on the type in auto completions, etc). In the end I did this
in some cases, like in the database layer where a lot of things like
`func (fv *FileVersion) IsEmpty() bool` becomes `func fvIsEmpty(fv
*generated.FileVersion)` because they were anyway just internal methods.
Fixes#8247
Each section in the advanced settings dialog has similar code to insert
repeated input fields for each option. But only the first section (GUI
options) was adjusted in #9743 to avoid calling the `inputTypeFor()`
function repeatedly.
Apply the same caching to a locally scoped variable for each ng-repeat
entry by defining it in an ng-init directive.
I came accross this in another context and didn't investigate fully, but
literally ten lines above this code, in another method, we say that
filesets _must_ be created under the lock. It's either one or the other
and I'm taking the safer route here.
---------
Co-authored-by: Simon Frei <freisim93@gmail.com>
Encrypted-to-encrypted connections (i.e., ones where both sides set a
password) used to work but were broken in the 1.28.0 release. The
culprit is the 5342bec1b refactor which slightly changed how the request
was constructed, resulting in a bad block hash field.
Co-authored-by: Simon Frei <freisim93@gmail.com>
### Purpose
When generating a new `config.xml` file with default options, the GUI
address is populated with a hard-coded default value of
`127.0.0.1:8384`, except for a random free port if that default one is
occupied. This is independent from the GUI configuration default address
defined in the protobuf description. More importantly, it ignores any
`STGUIADDRESS` override given via environment variable or command-line
option, thus probing for the default port instead of the one specified
via override.
The `ProbeFreePorts()` function now respects the override, by reading
the `GUIConfiguration.Address()` method instead of using hard-coded
defaults.
When not calling `ProbeFreePorts()`, the override should still be
persisted rather than the default address. This happens only when
generating a fresh default `config.xml`, never on an existing one.
This adds `allow_contributor: true` which allows approvals by
contributors to the PR (but still not the author themself, which is a
different thing). This allows things like pushing minor fixups while
also approving.
The `ignore_update_merges: true` option makes it so that someone is not
considered a "contributor" just because they push the merge button to
update the branch. In principle this is not needed given the above, but
I like it for clarity.
### Purpose
This closes#9400 by always expanding tildes when parent/subdir checks
are done.
### Testing
I tested this by creating folders with paths to parent or subdirectories
of the default folder that include a tilde in their path as shown in the
attached screenshots.
With this change, overlap will be detected regardless of wether or not
tildes are used in other folder paths.
### Screenshots
Default Folder:
Newly created folder (parent directory in this case)
---------
Signed-off-by: tobifroe <froeltob@pm.me>
### Purpose
As discussed in #9686
Syncthing currently does not check folderstate on remote device before
pulling. If no devices have a valid folderstate (i.e all devices have
the folder paused) it will still attempt to pull. On large folders this
will cause a hanging "Syncing" status.
This checks whether at least one connected device has the file available
and has a valid folderstate.
### Testing
Tested locally on multiple devices.
We're new to Go (all our stuff is Python) so please bear with!
Interested if there may be a better place to slot this in.
Thanks,
Jon
---------
Co-authored-by: Simon Frei <freisim93@gmail.com>
Currently, the "Restart" and "Shutdown" buttons are displayed in the
middle of the Actions menu. On the other hand, the "Log Out" button is
displayed at the very bottom. However, in other cases, e.g. the menus in
operating systems like Windows or macOS, these kind of buttons are
usually grouped together.
Therefore, move the "Restart" and "Shutdown" buttons down, so that they
are listed together with the "Log Out" button. Also, change the order,
so that it goes from the least impactful ("Log Out") to the most
impactful ("Shutdown").
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
### Screenshots
#### Before
#### After
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
### Purpose
This fixes#9775. I also improved the comments as they were lacking.
My apologies for introducing this bug. In summary, the bug was
```
mode = mode ^ (ModeSymlink | ModeIrregular)
```
didn't correctly reset those bits. This correctly resets them:
```
mode = mode &^ ModeSymlink &^ ModeIrregular
```
Tested and working in Windows 11 version 10.0.22631.4317. I didn't test
in other versions, but I'm sure this is the only issue.
Currently we log on every single one of 10 retries deep in the upnp
stack. However we also return the failure as an error, which is bubbled
up a while until it's logged at debug level. Switch that around, such
that the repeat logging happens at debug level but the top-level happens
at info. There's some chance that this will newly log errors from
nat-pmp that were previously hidden in debug level - I hope those are
useful and not too numerous.
Also potentially this can even close#9324, my (very limited)
understanding of the reports/discussion there is that there's likely no
problem with syncthing beyond the excessive logging, it's some weird
router behaviour.
These CSS overrides address issues that are already present on wider
screens, so apply it there. Some experiments show we might even want to
up the limit more, but I am chicken and lazy, so I propose to use the
existing 470px media block.
Supersedes another PR after not getting any reaction to feedback there:
https://github.com/syncthing/syncthing/pull/9591#issuecomment-2212586134
Co-authored-by: Jakob Borg <jakob@kastelo.net>
As discussed in
https://github.com/syncthing/syncthing/pull/9175#discussion_r1730431703,
entries in advanced settings are unusable if they are comprised of a
list of objects. It just displays `[object Object], [object Object],
[object Object]`, e.g. for the devices a folder is shared with.
Filter out these config elements by detecting an array whose members are
not all strings or numbers, and setting them to `skip` type.
Fix some unnecessary repetition in calling `inputTypeFor()`, since it is
already cached in the `ng-init` directive.
### Purpose
Fixes#9776 by tweaking the text/background colours of disabled checkbox
panels when dark mode is enabled.
It was [noted on that
issue](https://github.com/syncthing/syncthing/issues/9776#issuecomment-2424828520)
that there's a bigger issue around the correctness of using the
`disabled` attribute on a `<div>` in the first place, but this PR does
not attempt to change that.
### Testing
I've hooked up the GUI files against a release build as suggested below.
### Screenshots
Using the dark theme, or the default theme with a system dark scheme:
Using the black theme:
These borrow the colours from dark theme text inputs and black theme
tabs for a consistent look (initially I tried the text colour of
disabled text inputs, but that produced some poor contrast).
Skipping these makes the sequence numbering inconcistent; we've received
a file and suppsedly added it to the database, but if you check the
sequence number afterwards it didn't increase, i.e., we trigger [this
failure
condition](47f48faed7/lib/model/indexhandler.go (L447-L459))
and, similarly, a future update will look like there was a hole in the
numbering.
I propose to at least temporarily remove this optimisation in order for
things to make more sense. Is there a reason to keep this beyond saving
some database operations?
This should prevent the panic that occurred in this test run:
https://github.com/syncthing/syncthing/actions/runs/11095876010/job/30825046810
```
2024-09-29T21:01:53.5425372Z === RUN TestIssue4357
2024-09-29T21:01:53.5505943Z panic: runtime error: integer divide by zero [recovered]
2024-09-29T21:01:53.5512200Z panic: runtime error: integer divide by zero
2024-09-29T21:01:53.5516633Z
2024-09-29T21:01:53.5523018Z goroutine 2655 [running]:
2024-09-29T21:01:53.5524157Z github.com/thejerf/suture/v4.(*Supervisor).runService.func2.2()
2024-09-29T21:01:53.5527176Z /home/runner/go/pkg/mod/github.com/thejerf/suture/v4@v4.0.5/supervisor.go:563 +0xd0
2024-09-29T21:01:53.5530556Z panic({0x1080d20?, 0x1851290?})
2024-09-29T21:01:53.5564723Z /home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.1.linux-amd64/src/runtime/panic.go:785 +0x132
2024-09-29T21:01:53.5566616Z github.com/syncthing/syncthing/lib/model.(*model).numHashers(0xc0006f6180, {0x117dc1a, 0x7})
2024-09-29T21:01:53.5568061Z /home/runner/work/syncthing/syncthing/lib/model/model.go:2581 +0x210
2024-09-29T21:01:53.5569912Z github.com/syncthing/syncthing/lib/model.(*folder).scanSubdirsChangedAndNew(0xc00c38c808, {0x0, 0x0, 0x0}, 0xc0003fc060)
2024-09-29T21:01:53.5571612Z /home/runner/work/syncthing/syncthing/lib/model/folder.go:653 +0x250
2024-09-29T21:01:53.5573010Z github.com/syncthing/syncthing/lib/model.(*folder).scanSubdirs(0xc00c38c808, {0x0, 0x0, 0x0})
2024-09-29T21:01:53.5574447Z /home/runner/work/syncthing/syncthing/lib/model/folder.go:512 +0xd0f
2024-09-29T21:01:53.5576011Z github.com/syncthing/syncthing/lib/model.(*folder).scanTimerFired(0xc00c38c808)
2024-09-29T21:01:53.5577367Z /home/runner/work/syncthing/syncthing/lib/model/folder.go:916 +0x46
2024-09-29T21:01:53.5579010Z github.com/syncthing/syncthing/lib/model.(*folder).Serve(0xc00c38c808, {0x1307650, 0xc0006a0910})
2024-09-29T21:01:53.5580428Z /home/runner/work/syncthing/syncthing/lib/model/folder.go:205 +0xd7e
2024-09-29T21:01:53.5581624Z github.com/thejerf/suture/v4.(*Supervisor).runService.func2()
2024-09-29T21:01:53.5582978Z /home/runner/go/pkg/mod/github.com/thejerf/suture/v4@v4.0.5/supervisor.go:567 +0x249
2024-09-29T21:01:53.5584400Z created by github.com/thejerf/suture/v4.(*Supervisor).runService in goroutine 2651
2024-09-29T21:01:53.5585872Z /home/runner/go/pkg/mod/github.com/thejerf/suture/v4@v4.0.5/supervisor.go:541 +0x32a
2024-09-29T21:01:53.5661413Z FAIL github.com/syncthing/syncthing/lib/model 5.510s
```
### Testing
I have not been able to reproduce the panic throughout a few minutes of
continuously running the test without this fix, but judging by the
traceback it seems to only happen if the test happens to delete the
folder from config at the same time `scanTimerFired` triggers.
### Purpose
Syncthing had a healthcheck API for a while, and the example Dockerfile
for it has it in the form of:
HEALTHCHECK --interval=1m --timeout=10s \
CMD curl -fkLsS -m 2 127.0.0.1:8384/rest/noauth/health | grep -o
--color=never OK || exit 1
Let's add it to the docker-compose as well
### Testing
I use this docker-compose.yml file to deploy via ansible (using
community.docker.docker_compose_v2) to my machine with success, using
`wait: true` in ansible for it to use `docker compose up --wait`.
```yml
- name: Enable syncthing docker
community.docker.docker_compose_v2:
project_src: /srv/syncthing
wait: true
wait_timeout: 90
```
In ignores, normalize the input when parsing it.
When scanning, normalize earlier such that the path is already
normalized when checking ignores. This requires splitting normalization
of the string from normalization of the file, as we don't want to
attempt the latter if the file is ignored.
Closes#9598
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
There was a bug that the unique ID was not set when reporting was
enabled, and thus the reports where rejected by the server. The unique
ID got set only on startup, so next time Syncthing restarted.
This makes sure to set the unique ID when blank.
I can see already in our Sentry data that there are a fair amount of
these warnings, and mostly the shape of it. Asking users to report them
will likely cause a lot of reporting effort to fairly little additional
value. We can do that when/if we have something more targeted to ask
for.
This codifies a review policy which is closer to what I always
envisioned, but which isn't expressible using the normal checks in the
GitHub GUI. It would move the commit approval check from GitHub into the
policy-bot check which is already present to enforce the
conventional-commits standard. Approvals in general would still work the
same -- it's just that the bot picks it up and toggles the status
accordingly. From a GitHub side when this is enabled we'd remove the
requires-review check from there and let the bot decide that part. We
would still require builds and tests to pass of course.
There are a couple of relexations from the current policy, details in
the code but briefly:
- Changes to translations or dependencies by a trusted person don't
require review
- Trivial changes by a trusted person, explicitly marked as such, don't
require review
This enables less bureaucracy for things like adding new translated
languages and updating dependencies, and enables the trivial-change
workflow to a larger audience than, like, me, who could always just
bypass the rules by way of being admin.
### Purpose
Since #8757, the Syncthing GUI now has an unauthenticated state. One
consequence of this is that `$scope.versionBase()` is not initialized
while unauthenticated, which causes the `docsURL` function to truncate
links to just `https://docs.syncthing.net`/, discarding the section
path. This currently affects at least the "Help > Introduction" link
reachable both while logged in and not. The issue is exacerbated in
https://github.com/syncthing/syncthing/pull/9175 where we sometimes want
to show additional contextual help links from the login page to
particular sections of the docs.
I don't think it's any worse to try to preserve the section path even
without an explicit version tag, than to fall back to just the host and
lose all context the link was attempting to provide.
### Testing
- On commit b1ed2802fb (before):
- Open the GUI, set a username and log out.
- Open the "Help" drop-down. The "Introduction" item links to:
https://docs.syncthing.net/
- Log in.
- Open the "Help" drop-down. The "Introduction" item links to:
https://docs.syncthing.net/v1.27.10/intro/gui
- On commit 44fef31780 (after):
- Open the GUI, set a username and log out.
- Open the "Help" drop-down. The "Introduction" item links to:
https://docs.syncthing.net/intro/gui
- Log in.
- Open the "Help" drop-down. The "Introduction" item links to:
https://docs.syncthing.net/v1.27.10/intro/gui
### Screenshots
This is a GUI change, but affecting only URLs in the markup with no
visual changes.
### Drawbacks
If a `docsURL` call generates a versionless link to a docs page that
doesn't exist on https://docs.syncthing.net - presumably because
Syncthing is not the latest version and links to a deleted page? - then
this will lead to GitHub's generic 404 page with no link to the
Syncthing docs root. Before, any versionless link would also be a
pathless link, leading to the Syncthing docs root instead of a 404 page.
### Purpose
The primary aim of this change is to minimize log clutter in production
environments. There are many lines in the logs coming from an expected
race condition when two devices connect `already connected to this
device`. These messages do not indicate errors and can overwhelm the log
files with unnecessary noise.
By lowering the logging level, we enhance the usability of the logs,
making it easier for users and developers to identify actual issues
without being distracted
### Testing
1. Build syncthing locally
2. Start two Syncthing instances
```bash
./syncthing -no-browser -home=~/.config/syncthing1
./syncthing -no-browser -home=~/.config/syncthing2
```
3. Enable the DEBUG logs from UI for `connections` package
4. Connect the synching instances by adding remote devices from the UI
5. Observe the logs for the message `XXXX already connected to this
device`
### Screenshots
## Authorship
Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.
Move infrastructure related commands to under `cmd/infra` and
development stuff to `cmd/dev`. The default build command builds the
regular user facing binaries: syncthing, stdiscosrv, and strelaysrv.
Reasoning in comments. The main motivation is to avoid all the case
checks when walking the filesystem.
"again" as we already tried once, but it caused a major issue ragarding
mtimefs layer. The root of this problem has been fixed in the meantime
in ac8b3342a
* infrastructure:
chore(stdiscosrv): ensure incoming addresses are sorted and unique
chore(stdiscosrv): use zero-allocation merge in the common case
chore(stdiscosrv): properly clean out old addresses from memory
chore(stdiscosrv): calculate IPv6 GUA
The read/write loops may keep going for a while on a closing connection
with lots of read/write activity, as it's random which select case is
chosen. And if the connection is slow or even broken, a single
read/write
can take a long time/until timeout. Add initial non-blocking selects
with only the cases relevant to closing, to prioritize those.
This would have addressed a recent issue that arose when re-ordering our
"filesystem layers". Specifically moving the caseFilesystem to the
outermost layer. The previous cache included the filesystem, and as such
all the layers below. This isn't desirable (to put it mildly), as you
can create different variants of filesystems with different layers for
the same path and options. Concretely this did happen with the mtime
layer, which isn't always present. A test for the mtime related breakage
was added in #9687, and I intend to redo the caseFilesystem reordering
after this.
Ref: #9677
Followup to: #9687
This is to add the generation of `compat.json` as a release artifact. It
describes the runtime requirements of the release in question. The next
step is to have the upgrade server use this information to filter
releases provided to clients. This is per the discussion in #9656
---------
Co-authored-by: Ross Smith II <ross@smithii.com>
This changes the two remaining instances where we use insecure HTTPS to
use standard HTTPS certificate verification.
When we introduced these things, almost a decade ago, HTTPS certificates
were expensive and annoying to get, much of the web was still HTTP, and
many devices seemed to not have up-to-date CA bundles.
Nowadays _all_ of the web is HTTPS and I'm skeptical that any device can
work well without understanding LetsEncrypt certificates in particular.
Our current discovery servers use hardcoded certificates which has
several issues:
- Not great for security if it leaks as there is no way to rotate it
- Not great for infrastructure flexibility as we can't use many load
balancer or TLS termination services
- The certificate is a very oddball ECDSA-SHA384 type certificate which
has higher CPU cost than a more regular certificate, which has real
effects on our infrastructure
Using normal TLS certificates here improves these things.
I expect there will be some very few devices out there for which this
doesn't work. For the foreseeable future they can simply change the
config to use the old URLs and parameters -- it'll be years before we
can retire those entirely.
For the upgrade client this simply seems like better hygiene. While our
releases are signed anyway, protecting the metadata exchange is _better_
and, again, I doubt many clients will fail this today.
The test is quite odd and specific, but it does reproduce the issue that
caused #9677, so I'd propose to add it to have a simple regression test
for the basic scenario. Also the option to the fakefs might come handy
for other scenarios where you want to quickly test some behaviour on a
filesystem without nanosecond precision, without actually needing access
to one.
gui: Actually filter scope ID out of IPv6 address when using Remote GUI
(ref #8084)
The current code does not work, because it uses a string in the
replace() method instead of regex. Thus, change it to a proper regex.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The preference for languages in the Accept-Language header field
should not be deduced from the listed order, but from the passed
"quality values", according to the HTTP specification:
https://httpwg.org/specs/rfc9110.html#field.accept-language
This implements the parsing of q=values and ordering within the API
backend, to not complicate things further in the GUI code. Entries
with invalid (unparseable) quality values are discarded completely.
* gui: Fix API endpoint in comment.
gui: Fix incorrect UI language auto detection (fixes#9668)
Currently, the code only checks whether the detected language partially
matches one of the available languages. This means that if the detected
language is "fi" but among the available languages there is only "fil"
and no "fi", then it will match "fi" with "fil", even though the two are
completely different languages.
With this change, the matching is only done when there is a hyphen in
the language code, e.g. "en" will match with "en-US".
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This adds a header with the operating system version, verbatim in
whatever format the operating system reports it, to the upgrade check.
The intention is that the upgrade server can use this information to
filter out (or maybe just mark) potentially unsupported upgrades.
This adds a header with the operating system version, verbatim in
whatever format the operating system reports it, to the upgrade check.
The intention is that the upgrade server can use this information to
filter out (or maybe just mark) potentially unsupported upgrades.
Based on user request from Weblate, user `@aindriu80`.
Looks promising based on the profile:
https://hosted.weblate.org/user/aindriu80/ Not sure whether almost
30.000 translations in about one month is realistic for a human though.
### Purpose
Wrap access to Model for users that use the syncthing Go package. See
discussion:
https://github.com/syncthing/syncthing/pull/9619#pullrequestreview-2212484910
### Testing
It works with the iOS app. Other than that, there are no current users
of this API (to my knowledge) as Model was only exposed recently form
the iOS app.
### Purpose
This PR contains the set of changes needed to make Syncthing work on iOS
for [my iOS app for
Syncthing](https://github.com/pixelspark/sushitrain).
Most changes originate from [the Mobius Sync
fork](http://github.com/MobiusSync/syncthing/tree/ios). I have removed
the changes from their fork that are not strictly needed for my app
(i.e. their changes to the GUI and command line utilities, for instance)
and squashed it all in a single commit.
In summary, the changes are:
* Resolve non-absolute paths to the 'Documents' folder (basically the
only one an app can/should write user data to by default on iOS)
* Tweaking of build flags/conditions for iOS (i.e. determine which
basicfs_watch, ignoreresult variant to build for iOS)
* Disable upgrade mechanism on iOS
* Make `RequestGlobal` and `PullerProgress` public symbols
* Expose syncthing.app's Model instance (app.M)
* Add no-op stub for SetLowPriority on iOS
I would very much appreciate these changes to be (eventually) merged to
mainline syncthing, as this would allow my iOS app to track the mainline
source code directly and removes the need (for me at least) for
maintaining a separate fork. Perhaps the Mobius folks can also benefit
from this (although as noted this branch does not contain their changes
to e.g. the GUI).
### Testing
This branch has been tested with the iOS app and appears to work fine.
The full set of MobiusSync changes has been used before with success.
### Screenshots
n/a
### Documentation
There should be no visible changes for users due to this set of changes.
---------
Co-authored-by: Simon Pickup <simon@pickupinfinity.com>
Tiny cleanup I noticed while trying to fix/test another issue
(https://github.com/syncthing/syncthing/pull/9600). I shortly tried to
figure out what it was used for in the past, but gave up without
results.
Previously we queried cache with backslashes, and stored entries with
slashes. As in no cache hits ever for non-toplevel files. I also
eventually remembered that cache is disabled by default, so this is a
bit pointless, but still right :P
### Purpose
Avoid the issue where the folder marker is deleted by overzealous
cleanup tools because it's just a useless, empty directory.
We create a small file containing a an admonishment to not delete the
directory, and some metadata that is just for human consumption at the
moment. (But it would parse as a valid yaml file if we wanted to read
this, at some point.)
This will only apply when _creating_ a folder marker, that is, existing
setups will not gain the file automatically. Obviously, when using a
custom folder marker none of this applies.
Also, slightly adjust the permission bits for the folder marker directory and file on Unixes, making sure the group & write bits are unset.
### Testing
I've created and deleted a few folders and it appears to behave as I
expect.
### Screenshots
```
jb@ok:~/somefolder % ls -la
total 0
drwxr-xr-x 3 jb staff 96 May 1 08:52 ./
drwx------ 12 jb staff 384 May 1 08:52 ../
drwxr-xr-x 3 jb staff 96 May 1 08:52 .stfolder/
jb@ok:~/somefolder % ls -l .stfolder
total 8
-rw-r--r-- 1 jb staff 122 May 1 08:52 syncthing-folder-39a4b0.txt
jb@ok:~/somefolder % cat .stfolder/syncthing-folder-39a4b0.txt
# This directory is a Syncthing folder marker.
# Do not delete.
folderID: xtdca-cudyf
created: 2024-05-01T08:52:49+02:00
```
Currently the maximum delay is always derived automatically from the
initial delay. This is fine in most cases, but for some use cases (large
files that take a long time to write) we need to be able to set a longer
max delay than the computed value (e.g., 15s delay with 10min timeout).
This adds a small package `geoip` which knows how to download and manage
the Maxmind GeoLite2 database we use. This removes the need for various
scripts to download and manage the geoip database, something that today
happens on Docker startup for the relay pool server and using various
hand written hacks for the usage reporting server.
The database is downloaded when needed and then refreshed on a
best-effort basis weekly.
Bumps [github.com/quic-go/quic-go](https://github.com/quic-go/quic-go)
from 0.42.0 to 0.43.0.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/quic-go/quic-go/releases">github.com/quic-go/quic-go's
releases</a>.</em></p>
<blockquote>
<h2>v0.43.0</h2>
<h2><em>quic-go.net</em>: Launching a new Documentation Site</h2>
<p>With this release, we're launching a new documentation site for the
quic-go projects (quic-go itself, HTTP/3, webtransport-go, and soon,
masque-go): <a href="https://quic-go.net">quic-go.net</a>.</p>
<p>The documentation site aims to explain QUIC concepts and how they are
made accessible using quic-go's API. This site replaces the wiki, and
the ever-growing README files.</p>
<p>A lot of work has gone into the documentation already, but we're by
no means done yet. The entire source is public in <a
href="https://github.com/quic-go/docs/">https://github.com/quic-go/docs/</a>,
and we're happy about community contributions.</p>
<h2>HTTP Datagrams (RFC 9297)</h2>
<p>This release adds support for HTTP Datagrams (<a
href="https://datatracker.ietf.org/doc/html/rfc9297">RFC 9297</a>), both
on the client and on the server side (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4452">#4452</a>).
HTTP Datagrams are used in WebTransport in CONNECT-UDP (<a
href="https://datatracker.ietf.org/doc/html/rfc9298">RFC 9298</a>),
among others.</p>
<p>The new API for HTTP Datagrams is described on the new documentation
page: <a href="https://quic-go.net/docs/http3/datagrams/">HTTP
Datagrams</a>. The integration of HTTP Datagram support necessitated a
comprehensive refactor of the HTTP/3 package, resulting in several
breaking API changes listed below.</p>
<h2>Breaking Changes</h2>
<ul>
<li>quicvarint: functions now return an <code>int</code> instead the
internal <code>protocol.ByteCount</code> (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4365">#4365</a>)</li>
<li>http3: <code>Server.SetQuicHeaders</code> was renamed to
<code>SetQUICHeaders</code> (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4377">#4377</a>)</li>
<li>http3: <code>Server.QuicConfig</code> was renamed to
<code>QUICConfig</code> (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4384">#4384</a>)</li>
<li>http3: <code>RoundTripper.QuicConfig</code> was renamed to
<code>QUICConfig</code> (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4385">#4385</a>)</li>
<li>http3: <code>RoundTripOpt.CheckSettings</code> was removed (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4416">#4416</a>).
Use the new<code>SingleDestinationRoundTripper</code> API instead.</li>
<li>http3: the <code>HTTPStreamer</code> interface is now implemented by
the <code>http.ResponseWriter</code> (and not the
<code>http.Request.Body</code>) (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4469">#4469</a>)</li>
<li>include the maximum payload size in the
<code>DatagramTooLargeError</code> (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4470">#4470</a>)</li>
</ul>
<h2>Other Notable Changes</h2>
<ul>
<li>GSO and ECN is disabled on kernel versions older than 5 (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4456">#4456</a>)</li>
<li>http3: logging can be controlled using an <code>slog.Logger</code>
(<a
href="https://redirect.github.com/quic-go/quic-go/issues/4449">#4449</a>)</li>
<li>http3: HEAD requests can now be sent in 0-RTT (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4378">#4378</a>)</li>
<li>http3: duplicate QPACK encoder and decoder streams are not rejected
as required by the RFC (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4388">#4388</a>)</li>
<li>http3: Extended CONNECT are blocked until the server's SETTINGS are
received, as required by the RFC (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4450">#4450</a>)</li>
<li>http3: HTTP/3 client connections aren't removed if
<code>RoundTrip</code> errors due to a cancelled context (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4448">#4448</a>).
Thanks to <a
href="https://github.com/GeorgeMac"><code>@GeorgeMac</code></a>!</li>
<li>http3: sniff Content-Type when flushing the ResponseWriter (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4412">#4412</a>).
Thanks to <a
href="https://github.com/WeidiDeng"><code>@WeidiDeng</code></a>!</li>
<li>The <code>Context</code> exposed on the <code>quic.Stream</code> is
now derived from the connection's context (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4414">#4414</a>)</li>
<li>The UDP send and receive buffer size was increased to 7 MiB (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4455">#4455</a>).
Thanks to <a
href="https://github.com/bt90"><code>@bt90</code></a>!</li>
</ul>
<h2>Clarifications on the QUIC Stream State Machine</h2>
<h3>Calling CancelWrite after Close</h3>
<p>After a long and fruitful discussion (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4404">#4404</a>),
we decided to clarify that calling <code>CancelWrite</code> after
<code>Close</code> on a <code>SendStream</code> (or a bidirectional
stream) should cause a state transition from the "Data Sent"
to the "Reset Sent" state, as described in <a
href="https://datatracker.ietf.org/doc/html/rfc9000#section-3.1">section
3.1 of RFC 9000</a>. This matches the current behavior of quic-go,
however, it didn't match the API documentation (fixed in <a
href="https://redirect.github.com/quic-go/quic-go/issues/4419">#4419</a>).</p>
<p>This means that stream data will not be delivered reliably if
<code>CancelWrite</code> is called, and that this applies even if
<code>Close</code> was called before.</p>
<h3>Garbage Collection of Streams</h3>
<p>This release also changes the way streams are garbage-collected (and
the peer is granted additional limit to open a new stream), once they're
not needed anymore, in a subtle way:</p>
<ul>
<li>for the send direction of streams: <a
href="https://redirect.github.com/quic-go/quic-go/issues/4445">#4445</a></li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="93c4785521"><code>93c4785</code></a>
http3: sniff Content-Type when flushing the ResponseWriter (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4412">#4412</a>)</li>
<li><a
href="c0250ce824"><code>c0250ce</code></a>
include the maximum payload size in the DatagramTooLargeError (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4470">#4470</a>)</li>
<li><a
href="34f4d1443f"><code>34f4d14</code></a>
http3: implement on the HTTPStreamer on the ResponseWriter, flush header
(<a
href="https://redirect.github.com/quic-go/quic-go/issues/4469">#4469</a>)</li>
<li><a
href="083ceb42f2"><code>083ceb4</code></a>
http3: rename Settings.EnableDatagram to EnableDatagrams (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4466">#4466</a>)</li>
<li><a
href="e1e5b6294d"><code>e1e5b62</code></a>
README: link to the new documentation site (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4464">#4464</a>)</li>
<li><a
href="2a37c53143"><code>2a37c53</code></a>
http3: add support for HTTP Datagrams (RFC 9297) (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4452">#4452</a>)</li>
<li><a
href="11b11594b2"><code>11b1159</code></a>
http3: fix race condition in client unit test (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4463">#4463</a>)</li>
<li><a
href="4b87539b1e"><code>4b87539</code></a>
delay completion of the receive stream until the reset error was read
(<a
href="https://redirect.github.com/quic-go/quic-go/issues/4460">#4460</a>)</li>
<li><a
href="bff131e546"><code>bff131e</code></a>
delay completion of the send stream until the reset error was delivered
(<a
href="https://redirect.github.com/quic-go/quic-go/issues/4445">#4445</a>)</li>
<li><a
href="12aa63824c"><code>12aa638</code></a>
disable GSO and ECN on kernels older than version 5 (<a
href="https://redirect.github.com/quic-go/quic-go/issues/4456">#4456</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/quic-go/quic-go/compare/v0.42.0...v0.43.0">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
### Purpose
Adds a new metric `syncthing_connections_active` which equals to the
amount of active connections per device.
Fixes#9527
<!--
Describe the purpose of this change. If there is an existing issue that
is
resolved by this pull request, ensure that the commit subject is on the
form
`Some short description (fixes#1234)` where 1234 is the issue number.
-->
### Testing
I've manually tested it by running syncthing with these changes locally
and examining the returned metrics from `/metrics`.
I've done the following things:
- Connect & disconnect a device
- Increase & decrease the number of connections and verify that the
value of the metric matches with the amount displayed in the GUI.
### Documentation
https://github.com/syncthing/docs/blob/main/includes/metrics-list.rst
needs to be regenerated with
[find-metrics.go](https://github.com/syncthing/docs/blob/main/_script/find-metrics/find-metrics.go)
## Authorship
Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
### Purpose
Firefox uses the last specified favicon link for bookmarks, but only if
it is available on initial page load.
Remove the second link and use ng-href to change the icon instead.
I'm not really familiar with AngularJS, feel free to offer suggestions
for improvements.
### Testing
Briefly tested on Firefox 124.0.2 and Chrome 123.0.6312.105.
### Purpose
Resend our indexes since we fixed that index-sending issue.
I made a new thing to only drop the non-local-device index IDs, i.e.,
those for other devices. This means we will see a mismatch and resend
all indexes, but they will not. This is somewhat cleaner as it avoids
resending everything twice when two devices are upgraded, and in any
case, we have no reason to force a resend of incoming indexes here.
### Testing
It happens on my computer...
The commit 7e4e65ebf5 added links to
devices listed in the Shared With list in the folder info. However, it
only added them to those that had no superscript next to them.
With this change, the links are added to all devices regardless of
whether they have the superscript next to their names or not. The commit
also simplifies the code by using anchors directly instead of wrapping
them in spans.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
### Purpose
Bountysource no longer exists and the readme link 404s. This removes the
Bountysource link and corresponding readme section.
Perhaps the section should instead be replaced by other instructions for
voting on features.
This is an extract from PR #9175, which can be reviewed in isolation to
reduce the volume of changes to review all at once in #9175. There are
about to be several services and API handlers that read and set cookies
and session state, so this abstraction will prove helpful.
In particular a motivating cause for this is that with the current
architecture in PR #9175, in `api.go` the [`webauthnService` needs to
access the
session](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dR309-R310)
for authentication purposes but needs to be instantiated before the
`configMuxBuilder` for config purposes, because the WebAuthn additions
to config management need to perform WebAuthn registration ceremonies,
but currently the session management is embedded in the
`basicAuthAndSessionMiddleware` which is [instantiated much
later](https://github.com/syncthing/syncthing/pull/9175/files#diff-e2e14f22d818b8e635572ef0ee7718dee875c365e07225d760a6faae8be7772dL371-R380)
and only if authentication is enabled in `guiCfg`. This refactorization
extracts the session management out from `basicAuthAndSessionMiddleware`
so that `basicAuthAndSessionMiddleware` and `webauthnService` can both
use the same shared session management service to perform session
management logic.
### Testing
This is a refactorization intended to not change any externally
observable behaviour, so existing tests (e.g., `api_auth_test.go`)
should cover this where appropriate. I have manually verified that:
- Appending `+ "foo"` to the cookie name in `createSession` causes
`TestHtmlFormLogin/invalid_URL_returns_403_before_auth_and_404_after_auth`
and `TestHtmlFormLogin/UTF-8_auth_works` to fail
- Inverting the return value of `hasValidSession` cases a whole bunch of
tests in `TestHTTPLogin` and `TestHtmlFormLogin` to fail
- (Fixed) Changing the cookie to `MaxAge: 1000` in `destroySession` does
NOT cause any tests to fail!
- Added tests `TestHtmlFormLogin/Logout_removes_the_session_cookie`,
`TestHTTPLogin/*/Logout_removes_the_session_cookie`,
`TestHtmlFormLogin/Session_cookie_is_invalid_after_logout` and
`TestHTTPLogin/200_path#01/Session_cookie_is_invalid_after_logout` to
cover this.
- Manually verified that these tests pass both before and after the
changes in this PR, and that changing the cookie to `MaxAge: 1000` or
not calling `m.tokens.Delete(cookie.Value)` in `destroySession` makes
the respective pair of tests fail.
This re-implements the stalled enhancement from #8808. Thanks @Craeckie
for the idea and first implementation draft!
If a folder is shared to a device with encryption, add a lock icon in
front of the device name under "Shared With" in the folder details
panel. Be careful not to add whitespace caused by line wraps in HTML
source code, which would defeat the purpose of keeping the icon glued to
the name by a non-breaking space.
Apply the same lock icon for the list of folders shared with a device.
This change was split off from #9355 as an independent clean-up / fix.
See that PR for review discussion, testing, and screenshots.
Improve the wrapping of folder labels / device names by going back to
word-wrapping, but making sure other spans, such as the trailing comma,
do not get separated from the label span.
* Avoid adding whitespace caused by line wraps in HTML source code.
The different cases within the ng-switch block are separated by
newlines for readability, but that gets parsed as whitespace. For
wrapping purposes, this should not happen, because then there is no
way to keep other HTML parts glued to the name / label in each list
entry.
* Simplify redundant conditional comma code.
The separating comma after a device name or folder label (all but the
last) should always stick to it. Use the HTML comment trick to avoid
whitespace and therefore a wrapping opportunity caused by the code
formatting newline. Thus the conditional comma only needs to be
defined once, not in each ng-switch case.
* Wrap at word boundaries and only break up words if necessary.
Use the overflow-wrap: break-word; style instead of word-break:
break-all;. While the latter is suitable for longish paths, breaking
device names or folder labels arbitrarily within words is ugly.
This also makes the the <sup> numbers actually stay glued to their
respective neighboring words.
Include legacy CSS alias "word-wrap" in the class definition.
* Fix indentation (unrelated).
Adds a bool flag to `scanIfItemChanged()` to indicate when the scan was initiated from a delete function, and if so, tell `IsEquivalentOptional()` to ignore Xattrs and Ownership regardless of the global setting.
I tested this with my sledgehammer and it seems to pass.
Go is not cgroup aware and by default will set GOMAXPROCS to the number
of available threads, regardless of whether it is within the allocated
quota. This behaviour causes high amount of CPU throttling and degraded
application performance.
Add support for setting umask value in the Docker `entrypoint.sh`
script. This is useful when
not syncing permissions and working with groups, and needing umask
values like `002` instead of `022`.
The changes to go.mod in latest Go 1.21/1.22 are not fully understood by
older Go that might be pre-installed on builds, so make sure we always
have a modern one in place even for running small release scripts etc.
Somewhere along the way, the non-parallel test became parallel, and at
that point, timeouts occurred. Parallel is better, so increase the
timeout on the offending call a bit...
The new test has a flakiness factor on slow platforms, where the close
on the sending connection races with the last index message, potentially
messing up the count. This adds a wait to ensure that all sent messages
are received, or the test will eventually fail with a timeout.
Currently `IsTraced("xyz")` will return true for
any inclusion of "xyz" in string.
This change splits `STTRACE` using `','`, `' '` and `';'`
as delimiters. That makes facilities separation
more clear.
This makes a couple of small improvements to the folder summary
mechanism:
- The folder summary includes the local and remote sequence numbers in
clear text, rather than some odd sum that I'm not sure what it was
intended to represent.
- The folder summary event is generated when appropriate, regardless of
whether there is an event listener. We did this before because
generating it was expensive, and we wanted to avoid doing it
unnecessarily. Nowadays, however, it's mostly just reading out
pre-calculated metadata, and anyway, it's nice if it shows up reliably
when running with -verbose.
The point of all this is to make it easier to use these events to judge
when devices are, in fact, in sync. As-is, if I'm looking at two
devices, it's very difficult to reliably determine if they are in sync
or not. The reason is that while we can ask device A if it thinks it's
in sync, we can't see if the answer is "yes" because it has processed
all changes from B, or if it just doesn't know about the changes from B
yet. With proper sequence numbers in the event we can compare the two
and determine the truth. This makes testing a lot easier.
This is a refactor of the protocol/model interface to take the actual
message as the parameter, instead of the broken-out fields:
```diff
type Model interface {
// An index was received from the peer device
- Index(conn Connection, folder string, files []FileInfo) error
+ Index(conn Connection, idx *Index) error
// An index update was received from the peer device
- IndexUpdate(conn Connection, folder string, files []FileInfo) error
+ IndexUpdate(conn Connection, idxUp *IndexUpdate) error
// A request was made by the peer device
- Request(conn Connection, folder, name string, blockNo, size int32, offset int64, hash []byte, weakHash uint32, fromTemporary bool) (RequestResponse, error)
+ Request(conn Connection, req *Request) (RequestResponse, error)
// A cluster configuration message was received
- ClusterConfig(conn Connection, config ClusterConfig) error
+ ClusterConfig(conn Connection, config *ClusterConfig) error
// The peer device closed the connection or an error occurred
Closed(conn Connection, err error)
// The peer device sent progress updates for the files it is currently downloading
- DownloadProgress(conn Connection, folder string, updates []FileDownloadProgressUpdate) error
+ DownloadProgress(conn Connection, p *DownloadProgress) error
}
```
(and changing the `ClusterConfig` to `*ClusterConfig` for symmetry;
we'll be forced to use all pointers everywhere at some point anyway...)
The reason for this is that I have another thing cooking which is a
small troubleshooting change to check index consistency during transfer.
This required adding a field or two to the index/indexupdate messages,
and plumbing the extra parameters in umpteen changes is almost as big a
diff as this is. I figured let's do it once and avoid having to do that
in the future again...
The rest of the diff falls out of the change above, much of it being in
test code where we run these methods manually...
This improves the ignore handling so that directories can be fully
ignored (skipped in the watcher) in more cases. Specifically, where the
previous rule was that any complex `!`-pattern would disable skipping
directories, the new rule is that only matches on patterns *after* such
a `!`-pattern disable skipping. That is, the following now does the
intuitive thing:
```
/foo
/bar
!whatever
*
```
- `/foo/**` and `/bar/**` are completely skipped, since there is no
chance anything underneath them could ever be not-ignored
- `!whatever` toggles the "can't skip directories any more" flag
- Anything that matches `*` can't skip directories, because it's
possible we can have `whatever` match something deeper.
To enable this, some refactoring was necessary:
- The "can skip dirs" flag is now a property of the match result, not of
the pattern set as a whole.
- That meant returning a boolean is not good enough, we need to actually
return the entire `Result` (or, like, two booleans but that seemed
uglier and more annoying to use)
- `ShouldIgnore(string) boolean` went away with
`Match(string).IsIgnored()` being the obvious replacement (API
simplification!)
- The watcher then needed to import the `ignore` package (for the
`Result` type), but `fs` imports the watcher and `ignore` imports `fs`.
That's a cycle, so I broke out `Result` into a package of its own so
that it can be safely imported everywhere in things like `type Matcher
interface { Match(string) result.Result }`. There's a fair amount of
stuttering in `result.Result` and maybe we should go with something like
`ignoreresult.R` or so, leaving this open for discussion.
Tests refactored to suit, I think this change is in fact quite well
covered by the existing ones...
Also some noise because a few of the changed files were quite old and
got the `gofumpt` treatment by my editor. Sorry not sorry.
---------
Co-authored-by: Simon Frei <freisim93@gmail.com>
### Purpose
Fix#9241 by expanding tildes in version paths.
When creating the versioner file system, first try to expand any leading
tildes to the user's home directory before handling relative paths. This
makes a version path `"~/p"` expand to `"$HOME/p"` instead of
`"/folder/~/p"`.
### Testing
Added a test to lib/versioner that exercises this code path. Also
manually tested with local syncthing instances.
### Purpose
This PR changes behaviour of syncthing related to `receive-only`
folders, which I believe to be a bug since I wouldn't expect the current
behaviour. With the current syncthing codebase, a file of a
`receive-only` folder that is only modified locally can cause the
creation of a `.sync-conflict` file.
### Testing
Consider this szenario: Setup two paired clients that sync a folder with
a given file (e.g. `Test.txt`). One of the clients configures the folder
to be `receive-only`. Now, change the contents of the file for the
receive-only client **_twice_**.
With the current syncthing codebase, this leads to the creation of a
`.sync-conflict` file that contains the modified contents, while the
regular `Test.txt` file is reset to the cluster's provided contents.
This is due to a `protocol.FileInfo#ShouldConflict` check, that is
succeeding on the locally modified file.
This PR changes this behaviour to not reset the file and not cause the
creation of a `.sync-conflict`. Instead, the second content update is
treated the same as the first content update.
This PR also contains a test that fails on the current codebase and
succeeds with the changes introduced in this PR.
### Screenshots
This is not a GUI change
### Documentation
This is not a user visible change.
## Authorship
Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.
#### Thanks to all the syncthing folks for this awesome piece of
software!
### Purpose
This implements CLI completion using the Kongplete module. As a side
effect a CLI structure for syncthing/cli was created for kongplete to be
able to parse and implement CLI completion.
### Testing
I've tested the autocompletion manually, and it had worked, but I hadn't
added any tests so as to test it automatically. Additionally, I ran `go
run build.go test` with all tests passing.
This adds a "token manager" which handles storing and checking expired
tokens, used for both sessions and CSRF tokens. It removes the old,
corresponding functionality for CSRFs which saved things in a file. The
result is less crap in the state directory, and active login sessions
now survive a Syncthing restart (this really annoyed me).
It also adds a boolean on login to create a longer-lived session cookie,
which is now possible and useful. Thus we can remain logged in over
browser restarts, which was also annoying... :)
<img width="1001" alt="Screenshot 2023-12-12 at 09 56 34"
src="https://github.com/syncthing/syncthing/assets/125426/55cb20c8-78fc-453e-825d-655b94c8623b">
Best viewed with whitespace-insensitive diff, as a bunch of the auth
functions became methods instead of closures which changed indentation.
```
% export GOTOOLCHAIN=go1.20.7
% go list -m all | cut -d ' ' -f 1 | xargs go get -u
% go mod tidy
```
Except:
- github.com/jackpal/gateway now requires Go 1.21
- github.com/shirou/gopsutil breaks linux-mips in latest version
Currently, with a large number of versioned files, there is a delay
between the Restore Versions modal showing up on the screen and
initialisation of the actual versions tree. This leads to a situation,
where the modal is initially empty, which confuses the user, making
them think that something is not working correctly.
To avoid the above, always show the loading data information. The string
is displayed using static HTML first, and then replaced with the exact
same content once the tree has been initialised. Both elements use the
same style and position, so there is no visual shift between the two.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Bumps
[actions/download-artifact](https://github.com/actions/download-artifact)
from 3 to 4.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/actions/download-artifact/releases">actions/download-artifact's
releases</a>.</em></p>
<blockquote>
<h2>v4.0.0</h2>
<h2>What's Changed</h2>
<p>The release of upload-artifact@v4 and download-artifact@v4 are major
changes to the backend architecture of Artifacts. They have numerous
performance and behavioral improvements.</p>
<p>For more information, see the <a
href="https://github.com/actions/toolkit/tree/main/packages/artifact"><code>@actions/artifact</code></a>
documentation.</p>
<h2>New Contributors</h2>
<ul>
<li><a href="https://github.com/bflad"><code>@bflad</code></a> made
their first contribution in <a
href="https://redirect.github.com/actions/download-artifact/pull/194">actions/download-artifact#194</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a
href="https://github.com/actions/download-artifact/compare/v3...v4.0.0">https://github.com/actions/download-artifact/compare/v3...v4.0.0</a></p>
<h2>v3.0.2</h2>
<ul>
<li>Bump <code>@actions/artifact</code> to v1.1.1 - <a
href="https://redirect.github.com/actions/download-artifact/pull/195">actions/download-artifact#195</a></li>
<li>Fixed a bug in Node16 where if an HTTP download finished too quickly
(<1ms, e.g. when it's mocked) we attempt to delete a temp file that
has not been created yet <a
href="hhttps://redirect.github.com/actions/toolkit/pull/1278">actions/toolkit#1278</a></li>
</ul>
<h2>v3.0.1</h2>
<ul>
<li><a
href="https://redirect.github.com/actions/download-artifact/pull/178">Bump
<code>@actions/core</code> to 1.10.0</a></li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="7a1cd3216c"><code>7a1cd32</code></a>
Merge pull request <a
href="https://redirect.github.com/actions/download-artifact/issues/246">#246</a>
from actions/v4-beta</li>
<li><a
href="8f32874a49"><code>8f32874</code></a>
licensed cache</li>
<li><a
href="b5ff8444b1"><code>b5ff844</code></a>
Merge pull request <a
href="https://redirect.github.com/actions/download-artifact/issues/245">#245</a>
from actions/robherley/v4-documentation</li>
<li><a
href="f07a0f73f5"><code>f07a0f7</code></a>
Update README.md</li>
<li><a
href="7226129829"><code>7226129</code></a>
update test workflow to use different artifact names for matrix</li>
<li><a
href="ada9446619"><code>ada9446</code></a>
update docs and bump <code>@actions/artifact</code></li>
<li><a
href="7eafc8b729"><code>7eafc8b</code></a>
Merge pull request <a
href="https://redirect.github.com/actions/download-artifact/issues/244">#244</a>
from actions/robherley/bump-toolkit</li>
<li><a
href="3132d12662"><code>3132d12</code></a>
consume latest toolkit</li>
<li><a
href="5be1d38671"><code>5be1d38</code></a>
Merge pull request <a
href="https://redirect.github.com/actions/download-artifact/issues/243">#243</a>
from actions/robherley/v4-beta-updates</li>
<li><a
href="465b526e63"><code>465b526</code></a>
consume latest <code>@actions/toolkit</code></li>
<li>Additional commits viewable in <a
href="https://github.com/actions/download-artifact/compare/v3...v4">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
If syncOwnership is enabled and the remote uses for example a dockerized
Syncthing it can't fetch the ownername and groupname of the local
instance. Without this patch this led to an endless cycle of detected
changes on the remote and failing re-sync attempts.
This patch skips comparing the ownername and groupname if they zare empty
on one side.
See https://github.com/syncthing/syncthing/issues/9039 for details.
### Testing
Proposed by @calmh in
https://github.com/syncthing/syncthing/issues/9039#issuecomment-1870584783
and tested locally in my setup,
Setup PC 1:
- Syncthing is run in Docker as user `root` and has none of the users
configured that synchronize their files
Setup PC 2:
- this PC has all users locally setup
- Syncthing runs as `systemd` service as user `syncthing` and has
multiple capabilities set to set the correct owner and permissions
Setup PC 3:
- same as PC 2
Handling:
- `PC 1` is send & receive and uses just the `UID` and `GID` identifiers
to store the files
- `PC 2` and `PC 3` synchronize their files over `PC 1` but not directly
to each other
Outcome:
- `PC 2` and `PC 3` should send and receive their files with the correct
ownership and groups from `PC 1`
### Purpose
Instead of hardcoding `SigningKey` as text use `go:embed`. Fixes#9247.
### Testing
* Building syncthing
* Trying to upgrade (signature verification)
On kqueue-systems, folders listen for folder summaries to (be able to)
warn for potential high resource usage. However, it listened for any
folder summary and not for the summary which matches the folder it's
about. This could cause that an unwatched folder causes a folder summary
containing more files than the threshold (10k), and the listening folder
(with the watcher enabled) triggers the warning.
This makes sure that only the folder summaries which are relevant to the
specific folder are being handled.
### Testing
- Fire up some kqueue-system (freebsd, I used).
- add folder A, disable the watcher, add 10001 files
- add folder B with the watcher enabled, no files are needed here
Before the change:
- add an item to folder A, trigger a rescan to speed up the process
- wait some seconds...warning triggered by folder B's
summarySubscription
After the change:
- Only a warning is triggered if the received folder summary matches the
folder which listens for the summaries
Cleanup after #9275.
This renames `fmut` -> `mut`, removes the deadlock detector and
associated plumbing, renames some things from `...PRLocked` to
`...RLocked` and similar, and updates comments.
Apart from the removal of the deadlock detection machinery, no
functional code changes... i.e. almost 100% diff noise, have fun
reviewing.
I'm tired of the fmut/pmut shenanigans. This consolidates both under one
lock; I'm not convinced there are any significant performance
differences with this approach since we're literally just protecting map
juggling...
- The locking goes away when we were already under an appropriate fmut
lock.
- Where we had fmut.RLock()+pmut.Lock() it gets upgraded to an
fmut.Lock().
- Otherwise s/pmut/fmut/.
In order to avoid diff noise for an important change I did not do the
following cleanups, which will be filed in a PR after this one, if
accepted:
- Renaming fmut to just mut
- Renaming methods that refer to being "PRLocked" and stuff like that
- Removing the no longer relevant deadlock detector
- Comments referring to pmut and locking sequences...
`syncthing cli` subcommand was using urfave/cli as the command parser.
This PR replace it with kong, which the main command uses.
Some help texts and error message format are changed. Other than that,
all the command usage and logic remains unchanged.
There's only one place which still uses urfave/cli, which is `syncthing
cli config`, because it uses some magic to dynamically build commands
from struct reflects. I used kong's `passthrough:""` tag to pass any
argument following `syncthing cli config` to urfave/cli parser.
This PR also fixes#9041
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This pull request allows syncthing to request an IPv6
[pinhole](https://en.wikipedia.org/wiki/Firewall_pinhole), addressing
issue #7406. This helps users who prefer to use IPv6 for hosting their
services or are forced to do so because of
[CGNAT](https://en.wikipedia.org/wiki/Carrier-grade_NAT). Otherwise,
such users would have to configure their firewall manually to allow
syncthing traffic to pass through while IPv4 users can use UPnP to take
care of network configuration already.
### Testing
I have tested this in a virtual machine setup with miniupnpd running on
the virtualized router. It successfully added an IPv6 pinhole when used
with IPv6 only, an IPv4 port mapping when used with IPv4 only and both
when dual-stack (IPv4 and IPv6) is used.
Automated tests could be added for SOAP responses from the router but
automatically testing this with a real network is likely infeasible.
### Documentation
https://docs.syncthing.net/users/firewall.html could be updated to
mention the fact that UPnP now works with IPv6, although this change is
more "behind the scenes".
---------
Co-authored-by: Simon Frei <freisim93@gmail.com>
Co-authored-by: bt90 <btom1990@googlemail.com>
Co-authored-by: André Colomb <github.com@andre.colomb.de>
gui: Show folder/device status on small screens
On larger screens, folder and device status is shown in a textual form
directly next to folder and device titles. However, on small screens,
only icons are currently shown, which may be ambiguous to new users, who
cannot possibly know what a specific icon means (see [1]). Thus, on
small screens only, display a new entry in folder/device info that
contains the same textual information that is shown in the title on
larger screens.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Co-authored-by: André Colomb <src@andre.colomb.de>
Before introducing the service map and using it for folder runners, the
entries in folderCfgs and folderRunners for the same key/folder were
removed under a single lock. Stopping the folder happens separately
before that with just the read lock. Now with the service map stopping
the folder and removing it from the map is a single operation. And that
still happens with just a read-lock. However even with a full lock it’s
still problematic: After the folder stopped, the runner isn’t present
anymore while the folder-config still is and sais the folder isn't
paused.
The index handler in turn looks at the folder config that is not paused,
thus assumes the runner has to be present -> nil deref on the runner.
A better solution might be to push most of these fmut maps into the
folder - they anyway are needed in there. Then there's just a single
map/source of info that's necessarily consistent. That's quite a bit of
work though, and probably/likely there will be corner cases there too.
This reverts commit e477777f49.
In principle, we could have stayed with `~1.21.1`, but `check-latest:
true` apparently checks some cache/manifest/something that is only
periodically refreshed and isn't aware of 1.21.5 yet. So update the
constraints to force an upgrade.
Also the infrastructure images weren't actually using the constraint
since there was no `setup-go` action...
This reduces allocations, in number and in size, while getting extended
attributes. This is mostly noticable when there is a large number of new
files to scan and we're running with the default scanProgressInterval --
then a queue of files is built in-memory, and this queue includes
extended attributes as part of file metadata. (Arguable it shouldn't,
but that's a more difficult and involved change.)
With 1M files to scan, each with one extended attribute, current peak
memory usage looks like this:
Showing nodes accounting for 1425.30MB, 98.19% of 1451.64MB total
Dropped 1435 nodes (cum <= 7.26MB)
Showing top 10 nodes out of 54
flat flat% sum% cum cum%
976.56MB 67.27% 67.27% 976.56MB 67.27%
github.com/syncthing/syncthing/lib/fs.getXattr
305.44MB 21.04% 88.31% 305.44MB 21.04%
github.com/syncthing/syncthing/lib/scanner.(*walker).walk.func1
45.78MB 3.15% 91.47% 1045.23MB 72.00%
github.com/syncthing/syncthing/lib/fs.(*BasicFilesystem).GetXattr
22.89MB 1.58% 93.04% 22.89MB 1.58%
github.com/syncthing/syncthing/lib/fs.listXattr
22.89MB 1.58% 94.62% 22.89MB 1.58%
github.com/syncthing/syncthing/lib/protocol.(*PlatformData).SetXattrs
16MB 1.10% 95.72% 16.01MB 1.10%
github.com/syndtr/goleveldb/leveldb/memdb.New
After the change, it's this:
Showing nodes accounting for 502.32MB, 95.70% of 524.88MB total
Dropped 1400 nodes (cum <= 2.62MB)
Showing top 10 nodes out of 91
flat flat% sum% cum cum%
305.43MB 58.19% 58.19% 305.43MB 58.19%
github.com/syncthing/syncthing/lib/scanner.(*walker).walk.func1
45.79MB 8.72% 66.91% 68.68MB 13.09%
github.com/syncthing/syncthing/lib/fs.(*BasicFilesystem).GetXattr
32MB 6.10% 73.01% 32.01MB 6.10%
github.com/syndtr/goleveldb/leveldb/memdb.New
22.89MB 4.36% 77.37% 22.89MB 4.36%
github.com/syncthing/syncthing/lib/fs.listXattr
22.89MB 4.36% 81.73% 22.89MB 4.36%
github.com/syncthing/syncthing/lib/protocol.(*PlatformData).SetXattrs
15.35MB 2.92% 84.66% 15.36MB 2.93%
github.com/syndtr/goleveldb/leveldb/util.(*BufferPool).Get
15.28MB 2.91% 87.57% 15.28MB 2.91% strings.(*Builder).grow
(The usage for xattrs is reduced from 976 MB to 68 MB)
LastSeen for a device was only updated when they connected. This now
updates it when they disconnect, so that we remember the last time we
actually saw them. When asking the API for current stats, currently
connected devices get a last seen value of the current time.
The string is currently hard-coded in English, so allow to translate it
into other languages. It appears mainly in the browser title bar before
logging in into the GUI or when the GUI is still loading.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This adds our short device ID to the basic auth realm. This has at least
two consequences:
- It is different from what's presented by another device on the same
address (e.g., if I use SSH forwards to different dives on the same
local address), preventing credentials for one from being sent to
another.
- It is different from what we did previously, meaning we avoid cached
credentials from old versions interfering with the new login flow.
I don't *think* there should be things that depend on our precise realm
string, so this shouldn't break any existing setups...
Sneakily this also changes the session cookie and CSRF name, because I
think `id.Short().String()` is nicer than `id.String()[:5]` and the
short ID is two characters longer. That's also not a problem...
### Purpose
The OCI image spec specifies well-defined
[annotations](https://github.com/opencontainers/image-spec/blob/main/annotations.md)
that can be added to images.
Theses annotations can then be used by other tools to gather more
information of an image.
This PR adds the `org.opencontainers.image.source` to allow tools such
as [renovate](https://github.com/renovatebot/renovate) to find the
release notes of a give version.
~~I've only done this change for `Dockerfile`. Should I also add the
label to the other dockerfiles?~~
I've now added the source annotations to all `Dockerfile`s & action
workflows.
### Testing
None, change was done by following the [renovate
documentation](https://docs.renovatebot.com/modules/datasource/docker/).
Following up on #9192, this makes use of the new mechanism for the theme
names.
The dummy string added for testing is removed again here. All
translations are updated to the new nested syntax, except Chinese
(zh-HK) where the string weren't actually translated.
Some translations, especially single words or other short
labels for buttons and the like, may not be transferable between
contexts even if they happen to be equal in English. In these cases,
setting an explicit translation ID is important for context separation.
Angular Translate also supports nested JSON in translation tables,
addressed using `.` as namespace separator; this enhancement makes use
of this when extracting translation with an explicit translation ID.
We use `env.GO_VERSION` as cache key for the build cache, but this is
nowadays typically something like `~1.21.1` which doesn't change when
1.21.2, 1.21.3 etc are released, making the cache fairly useless as
everything gets rebuilt. This re-sets the `GO_VERSION` variable after
installing Go so that it contains the actual installed version.
### Purpose
Treat X-Forwarded-For as a comma-separated string to prevent nil IP being returned by the Discovery Server
### Testing
Unit Tests implemented
Testing with a Discovery Client can be done as follows:
```
A simple example to replicate this entails running Discovery with HTTP, use Nginx as a reverse proxy and hardcode (as an example) a list of IPs in the X-Forwarded-For header.
1. Send an Announcement with tcp://0.0.0.0:<some-port>
2. Query the DeviceID
3. Observe the returned IP Address is no longer nil; i.e. `tcp://<nil>:<some-port>`
```
Add an id attribute to the submit button shown on the login form. This
allows my password manager's form filling function to interact with the
button after filling in username and password (which already have the id
attribute in place).
This makes the new default $XDG_STATE_HOME/syncthing or
~/.local/state/syncthing, while still looking in legacy locations first
for existing installs.
Note that this does not *move* existing installs, and nor should we.
Existing paths will continue to be used as-is, but the user can move the
dir into the new place if they want to use it (as they could prior to
this change as well, for that matter).
### Documentation
Needs update to the config docs about our default locations.
lib/fs: Fix conflicts on Android due to fluctuating inode change time
[1] added inode change time to file info in order to support syncing
extended attributes. However, in the case of Android, this inode change
time fluctuates, leading to unexpected conflicts even when the user has
not even touched the files on the Android device itself. Thus, in order
to prevent those conflicts from happening, do not write inode change
time on Android.
[1] 6cac308bcd
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
### Purpose
Deduplicated files are apparently considered 'irregular' under the hood,
this causes them to simply be ignored by Syncthing. This change is more
of a workaround than a proper fix, as the fix should probably happen in
the underlying libraries? - which may take some time. In the meanwhile,
this change should make deduplicated files be treated as regular files
and be indexed and synced as they should.
### Testing
Create some volume where deduplication is turned on (see the relevant
issue for details, including a proper description of how to reproduce
it). Prior to this change, the deduplicated files were simply ignored
(even by the indexer). After this change, the deduplicated files are
being index and synced properly.
This is motivated by the Android app:
https://github.com/syncthing/syncthing-android/pull/1982#issuecomment-1752042554
The planned fix in response to basic auth behaviour changing in #8757
was to add the `Authorization` header when opening the WebView, but it
turns out the function used only applies the header to the initial page
load, not any subsequent script loads or AJAX calls. The
`basicAuthAndSessionMiddleware` checks for no-auth exceptions before
checking the `Authorization` header, so the header has no effect on the
initial page load since the `/` path is a no-auth exception. Thus the
Android app fails to log in when opening the WebView.
This changes the order of checks in `basicAuthAndSessionMiddleware` so
that the `Authorization` header is always checked if present, and a
session cookie is set if it is valid. Only after that does the
middleware fall back to checking for no-auth exceptions.
`api_test.go` has been expanded with additional checks:
- Check that a session cookie is set whenever correct basic auth is
provided.
- Check that a session cookie is not set when basic auth is incorrect.
- Check that a session cookie is not set when authenticating with an API
token (either via `X-Api-Key` or `Authorization: Bearer`).
And an additional test case:
- Check that requests to `/` always succeed, but receive a session
cookie when correct basic auth is provided.
I have manually verified that
- The new assertions fail if the `createSession` call is removed in
`basicAuthAndSessionMiddleware`.
- The new test cases in e6e4df4d70 fail
before the change in 0e47d37e73 is
applied.
improve parsing of gui-address overrides
make checks for whether the gui-address is overridden consistent by
checking whether the environment variable is set and not an empty
string. the `Network()` function however checked for the inclusion of
a slash instead of the presence of any characters. If the config file's
gui address was set to a unix socket and the gui override to a tcp
address, then the function would have wrongly returned "unix".
the `URL()` function always returned the config file's gui address if a
unix socket was configured, even if an override was specified.
the `URL()` function wrongly formatted unix addresses. the http(s)
protocol was used as the sheme and the path was percent escaped. because
of the previous bug, this could only be triggered if the config file's
gui address was tcp and an unix socket override was given.
simplify the `useTLS()` function's codepath for overrides.
Co-authored-by: digital <didev@dinid.net>
Add a new entry to the unfolded device info to inform the user that the
device has been marked as "untrusted" and all folders shared with it
have to be password-protected or already Receive Encrypted.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Add a new entry to the unfolded device info to inform the user that the
device has Auto Accept enabled.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Opening and hiding multiple modals at the same time as well as opening a
modal before fully hiding the previous one can lead to the body padding
infinitely increasing by the scrollbar width each time, with the only
way to fix it being refreshing the GUI.
Therefore, always try to ensure to open and hide multiple modals one by
one, and also that the previous modal has fully been hidden before
proceeding to open the next one. The most common case when this problem
happens is when saving config changes which displays a GUI blocking
modal that overlaps, e.g. with folder or device modals that have not
been hidden yet.
Ref: https://github.com/twbs/bootstrap/issues/3902#issuecomment-1547187799
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Because $scope is missing, there are JavaScript errors when ticking and
unticking the "Untrusted" checkbox in the Advanced tab of the Edit
Device modal.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
I don't really understand under what circumstances, but sometimes these
calls panic with a "panic: counter cannot decrease in value" because the
value passed to Add() was negative.
Currently, the UI is always blocked from modifications when changes are
being saved, even if the save process takes very little time. This leads
to a situation where showing and closing the blocking modal can take
more time than is actually required to perform the whole operation. The
modal opening and closing very quickly can also cause the screen to
flash for a brief moment, leading to visual discomfort.
Because of this, wait for at least 200 ms and only show the blocking
modal if the changes have not been saved until then yet. The value of
200 ms is loosely based on [1] which states that 'a delay of 0.2–1.0
seconds does mean that users notice the delay and thus feel the computer
is "working" on the command, as opposed to having the command be a
direct effect of the users' actions.' Additionally, the delay must not
be too long, because the main purpose of the blocking modal is to
prevent the user from making further changes, and a longer delay would
possibly allow to do so in that brief amount of time as long as the user
is quick enough with their input.
[1] https://nngroup.com/articles/response-times-3-important-limits
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
In principle a connection can close while it's in progress with
starting, and then it's undefined if we wait for goroutines to exit etc.
With this change, we will wait for start to complete before starting to
stop everything.
gui: Remove unused hard-coded styles from globalChangesModalView modal
Currently, the globalChangesModalView modal has hardcoded th and td
styles. However, they are not even used in the modal itself, because
Bootstrap overrides them with its own styles for these elements in the
same modal. Yet, when hard-coded like that, these styles can conflict
with other table elements in the GUI. Thus, remove them completely.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This adds the ability to have multiple concurrent connections to a single device. This is primarily useful when the network has multiple physical links for aggregated bandwidth. A single connection will never see a higher rate than a single link can give, but multiple connections are load-balanced over multiple links.
It is also incidentally useful for older multi-core CPUs, where bandwidth could be limited by the TLS performance of a single CPU core -- using multiple connections achieves concurrency in the required crypto calculations...
Co-authored-by: Simon Frei <freisim93@gmail.com>
Co-authored-by: tomasz1986 <twilczynski@naver.com>
Co-authored-by: bt90 <btom1990@googlemail.com>
Instead of separately tracking the token.
Also changes serviceMap to have a channel version of RemoveAndWait, so
that it's possible to do the removal under a lock but wait outside of
the lock. And changed where we do that in connection close, reversing
the change that happened when I added the serviceMap in 40b3b9ad1.
Currently, some of the information for folders and devices displayed in
the GUI relies on arbitrary values that come pre-set as defaults on a
fresh Syncthing installation, i.e. if the value matches the default, it
is hidden, and if does not, then it is displayed.
With this change, the GUI always displays all information regardless
of their value, making the overall experience more consistent and
predictable.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Currently, different icons are used for File Versioning when displayed
in the unfolded folder info in the main part of the GUI, and the icon
used in the Edit Folder modal. This changes the main GUI icon to match
the icon used in the modal.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The Logs icon was changed in [1] in the header, however the icon used in
the modal was left out. This changes it, so that the header and the
modal icons match.
[1] 2abba1dfb0
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Summarize platforms that fail to build, without overloading the build
log with errors that we anyway ignore. (Currently freebsd/riscv64 fails
to build.)
* gui: Remove footer and move links to header (fixes#5607)
Currently, the footer is always present and takes space at the bottom of
the GUI. However, the links listed there are not part of everyday user
interaction, and as such, they unnecessarily clutter the page, reducing
the usable screen space. Thus, transform the current Help link in the
header into a Help dropdown menu, and move the links from the footer
into it.
Also apply the following tweaks:
1. Move the About dialog from Actions to Help.
2. Add an Introduction (to the GUI) link to Help.
3. Change the Support icon from a question mark to a group of people.
4. Change the Changelog and About icons to a filled version to match the
other icons better.
5. Use a source code icon for Source Code instead of a wrench icon, and
move the wrench icon to Logs. This is done to prevent Changelog and
Logs from using the same icon.
6. Update all dropdown icons' Fork Awesome styles to "fa fa-fw <icon>".
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
* a few more Fork Awesome style updates
---------
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The CSS method to select device IDs on click was added in [1]. However,
it was later mistakenly overwritten by [2]. This commit fixes the
regression and also applies the same behaviour to the Edit Device modal
which was omitted in the original commit.
[1] 5baf5fedb5
[2] 5e384c9185
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
By creating the http.Transport and tls.Configuration ourselves we
override some default behavior and end up with a client that speaks only
HTTP/1.1.
This adds a call to http.ConfigureTransport to do the relevant magic to
enable HTTP/2.
Also tweaks the keepalive settings to be a little kinder to the
server(s).
The allowed IPv4 ranges are the same as before. But we now also accept IPv6 addresses in the ULA range FC00::/7. These addresses don't require an interface identifier and are roughly equivalent to the IPv4 private ranges.
Typical usecases:
VPN interface IPs: Wireguard, OpenVPN, Tailscale, ...
fixed IPv6 LAN addressing while the provider assigns a dynamic prefix. e.g used by pihole
https://cs.opensource.google/go/go/+/refs/tags/go1.21.0:src/net/ip.go;l=146
* lib/versioner: Factor out DefaultPath constant.
Replace several instances where .stversions is named literally to all
use the same definition in the versioner package. Exceptions are the
packages where a cyclic dependency on versioner is impossible, or some
tests which combine the versions base path with other components.
* lib/versioner: Fix comment about trash can in simple versioner.
* lib/versioner: Fix wrong versioning type string in error message.
The error message shows the folder type instead of the versioning
type, although the correct field is used in the comparison.
Safety check added in v1.23.6 introduced bug. Bug unshares folders with untrusted devices if folder does not have an encryption password set, regardless of whether the folder is shared with the untrusted device as encrypted or not. Prevents sharing with untrusted devices in some cases where sharing would be encrypted.
Patch preserves safety check but permits sharing folders with untrusted devices if they are shared as encrypted.
Signed-off-by: kewiha <keithh@protonmail.com>
Currently, the versions filter is case-sensitive regardless of the
underlying OS. With this change, the filter becomes case-insensitive
everywhere, which is more user-friendly and makes it easier to search
for files whose exact case the user may not remember.
In addition, forward and backslashes are no longer distinguished,
whether used as path separators or as part of a file / directory
name (which is unlikely but possible on some platforms).
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
As per Bootstrap recommendation, buttons with tooltips inside button
groups require to have container: 'body' set. This prevents tooltips
from causing the buttons to jump on hover and also allows the tooltips
to be wider instead of wrapping on every space.
Ref: https://getbootstrap.com/docs/3.3/components/#btn-groups
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Currently, historically, we look for the `X-API-Key` header to
authenticate with an API key. There's nothing wrong with this, but in
some scenarios it's easier to produce an `Authorization` header with a
`Bearer $token` content, which is nowadays more common. This change adds
support for both, so that we will accept an API key either in our custom
header or as a bearer token.
Currently, because of devices with unset RTC clock, the 100% percentile
for Uptime on [1] is calculated since the Unix epoch which is useless as
far as usage statistics are concerned. Thus, if the Syncthing start time
is set to a past date, assume that the clock is wrong and do not even
try to report the uptime.
[1] https://data.syncthing.net
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Co-authored-by: Jakob Borg <jakob@kastelo.net>
refactor: replace empty slice literal with `var`
An empty slice can be represented by `nil` or an empty slice literal. They are
functionally equivalent — their `len` and `cap` are both zero — but the `nil`
slice is the preferred style. For more information about empty slices,
see [Declaring Empty Slices](https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices).
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
refactor: fix unused method receiver
Methods with unused receivers can be a symptom of unfinished refactoring or a bug. To keep
the same method signature, omit the receiver name or '_' as it is unused.
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
refactor: unused parameter should be replaced by underscore
Unused parameters in functions or methods should be replaced with `_`
(underscore) or removed.
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
This adds an environment variable STVERSIONEXTRA that, when set, gets
added to the version information in the API and GUI.
The purpose of all this is to be able to communicate something about the
bundling or packaging, through the log & GUI and the end user, to the
potential person supporting it -- i.e., us. :) A wrapper can set this
variable to indicate that Syncthing is being run via `SyncTrayzor`,
`Syncthing-macOS`, etc., and thus indicate to the end user that the GUI
they are looking at is perhaps not the only source of truth and
management for this instance.
With this change, error messages include the offending characters or
name parts. Examples:
nul.txt: name is invalid, contains Windows reserved name: "nul"
foo>bar.txt: name is invalid, contains Windows reserved character: ">"
foo \bar.txt: name is invalid, must not end in space or period on Windows
This adds builds for the discovery and relay servers in the same manner
as Syncthing, so that Docker images are kept up to date with releases.
Inspired by and closes#8834.
Co-authored-by: Migelo <miha@filetki.si>
This adds builds for the discovery and relay servers in the same manner
as Syncthing, so that Docker images are kept up to date with releases.
Inspired by and closes#8834.
Co-authored-by: Migelo <miha@filetki.si>
This removes the tilde expansion we had in the GUI. I'm not sure why we
had it, but there are valid reasons to have a folder path with tilde in
it in the config, and it was previously impossible to enter one of
those.
This allows environment overrides for our directories. This is
advantageous because, apart from the obvious, it means we can set it in
the Docker file and not add command line options there. Having the
command line option as we did meant that it was impossible to use the
Docker image for other commands than `serve` (because that is implied
when we see other options on the command line).
This prevents combining untrusted with introducer and auto-accept, and
also verifies that folders shared with untrusted devices have passwords
at config loading time.
Co-authored-by: Simon Frei <freisim93@gmail.com>
We usually want to ensure that our own device is present. However if the
given device ID is the empty ID, we shouldn't do that. This is a
legimate (though way too non-obvious) use-case when opening the config
without knowing/caring about the device ID.
* Platform data (ownership, xattrs, etc.) is now set correctly for newly-received folders, even if the received folder has the NoPermissions flag.
* Call setPlatformData on receivers that have ignorePerms set to true.
This fixes various test issues with Go 1.20.
- Most tests rewritten to use fakefs where possible
- Some tests that were already skipped, or dubious (invasive,
unmaintainable, unclear what they even tested) have been removed
- Some actual code rewritten to better support testing in fakefs
Co-authored-by: Eric P <eric@kastelo.net>
In the sequence of loading ignores, the error File Does Not Exist is not being considered a fatal error, since the .stignore file is allowed to not exist. However, included ignore files also tossed that same error in case those do not exist while in those cases it's considered an error and it should lead to the folder stopping. Changing the error when opening an included ignore file to something other than the regular does fix this issue, as in it now works again as described in the Documentation.
This makes the various protocol priorities configurable among the other
options. With this, it's possible to prefer QUIC over TCP for WAN
connections, for example. Both sides need to be similarly configured for
this to work properly.
The default priority order remains the same as previously (TCP, QUIC,
Relay, with LAN better than WAN).
To make this happen I made each dialer & listener more priority aware,
and moved the check for whether a connection is LAN or not into the
dialer / listener -- this is the new "lanChecker" type that's passed
around.
In the original fix in #8563 I simply forgot this. Which meant #8556
wasn't actually fixed, as the trialer size would have been 0 (default),
and thus we would have still sent the inflated size to encrypted peers.
lib/model: Fix file size inconsisency due to enc. trailer
Fixes a regression due to PR #8563, while arguable the bug was actually
introduced in a much older PR #7155, but didn't have any bad effects so
far:
We account for the encryption trailer in the db updater routine,
calculating the file-info size there. However there's no guarantee that
the file-info at this point is still the exact same as when it was
written. It was before, but isn't anymore since introducing the new
EncryptedTrailerSize field.
Fix: Adjust the size in the info at the same place where the trailer is
written, i.e. we definitely have the actual size on disk.
The problem was that a statistics/cleanup run is triggered when the
database started and runs concurrently with the test. That cleanup run
removes old entries without valid addresses, and one of the test objects
matched this. The test object would thus randomly be removed in the
middle of the test, causing a failure. This fixes it so the object looks
recent when the cleaner-upper looks, and also uses a RAM database
(faster).
* gui: Import latest state from Weblate.
Downloaded 2023-03-23 at 20:38:53.
* gui: Remove untranslated strings from JSON files.
The GUI code will fall back to the English string translation anyway,
so keeping it identical inside the other language files does not
help. It makes the filter handling on Weblate easier though.
Current state retreived with these filter settings:
state:>=translated OR (state:needs-editing AND NOT check:same)
The layout of the request differs based on whether it comes from an
untrusted device or a trusted device with encrypted enabled. Handle
both.
Closes#8819.
Allow the watcher delay to take fractional values, effectively allowing
for much shorter delays. The minimum value is limited at 0.01, which
effectively translates to 10ms. This is required in order to guarantee
that there is still enough time to aggregate multiple single change
events.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This adds a cache to the expensive key generation operations. It's fixes
size LRU/MRU stuff to keep memory usage bounded under absurd conditions.
Also closes#8600.
Currently, the name and date filters in the Restore Versions modal are
always enabled, even if there are no versioned files present. With this
change, they are enabled only when there are no errors and versioned
files actually exist.
In addition to disabling the filters, also completely skip date picker
generation, as it serves no function while still displaying a non-sense
date range, which starts today and ends in the past.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This adds the BlocksHash field from the FileInfo to our API output. It
can be useful for debugging, or for external tools. I'm intentionally
leaving it as an opaque base64 string because no meaning should be
derived from it: it's just a string.
The reference comes from fd0a6225aa,
but the file itself was removed in the process of working on the pull
request, yet the reference to it was still left in the index.html.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
gui: Remove duplicate Spanish (Spain) translation.
Currently not among the set of "valid" (sufficiently complete)
languages and no longer synced with the tranlation platform, Weblate.
* Normalize EOL before EOF.
* Import current Transifex state, only translated.
Skip duplicated and completely empty languages.
* Re-add untranslated strings to JSON files.
These are now treated correctly as untranslated, but present with the
original (English) translation key.
* Update valid language codes and names.
* Refer to Weblate instead of Transifex for translations.
* lang-es: Copy untranslated strings from es-ES.
* Integrate translation changes from Weblate.
Translated using Weblate (Polish)
Currently translated at 100.0% (512 of 512 strings)
Translation: Syncthing/GUI strings
Translate-URL: https://hosted.weblate.org/projects/syncthing/gui/pl/
Translated using Weblate (Danish)
Currently translated at 98.6% (505 of 512 strings)
Translation: Syncthing/GUI strings
Translate-URL: https://hosted.weblate.org/projects/syncthing/gui/da/
Translated using Weblate (Lithuanian)
Currently translated at 85.4% (441 of 516 strings)
Translation: Syncthing/GUI strings
Translate-URL: https://hosted.weblate.org/projects/syncthing/gui/lt/
Translated using Weblate (French)
Currently translated at 100.0% (516 of 516 strings)
Translation: Syncthing/GUI strings
Translate-URL: https://hosted.weblate.org/projects/syncthing/gui/fr/
Translated using Weblate (Danish)
Currently translated at 98.6% (509 of 516 strings)
Translation: Syncthing/GUI strings
Translate-URL: https://hosted.weblate.org/projects/syncthing/gui/da/
Translated using Weblate (Spanish)
Currently translated at 100.0% (516 of 516 strings)
Translation: Syncthing/GUI strings
Translate-URL: https://hosted.weblate.org/projects/syncthing/gui/es/
* translation update uses weblate
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This makes sure the service manager doesn't interpret timeout errors, or any other error, as a signal to stop the service instead of restarting it.
I added it directly to our service utility function, as it may help catch other instances of the same problem... We would typically want timeouts etc to be a retryable error, unless it is the top level context that has timed out and we check for that specifically.
Currently, action and type are both displayed only in English. This
commit makes it possible to translate both of them.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The current code assumes that lastSeenDays is always set which is not
the case when device reports its lastSeen as equal to the Unix epoch.
Thus, make sure that lastSeenDays is set before proceeding with the
disconnected-inactive status check to avoid JS errors in the browser.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This adds a word to the version string when running containerized. The
purpose is mostly to facilitate troubleshooting via screenshot by
"leaking" this rather important aspect of the setup. Additionally, the
version row gets "no-overflow-ellipsis" treatment so that the whole
thing is actually visible in the GUI and the (now useless) tooltip is
removed. In production releases this won't make a difference as the
whole thing will typically fit, but in odd setups it provides more info
up front.
The authorship script didn't pick up people who were only ever
"co-authors" of a commit, such as when they wrote stuff which was later
included in a PR by someone else, or added code during code review.
This modified the script to look closer in the commit bodies for
"Co-authored-by:"-lines and adds those found to the set of authors.
gui: Remove unmaintained language variant nl-BE.
This has long been removed from Transifex and from the valid-langs.js
list, but somehow the translation file stayed in the repo. There is
already a generic Dutch variant (nl) available as replacement.
On systems with safe umasks (`umask 077`), the entrypoint as copied from
the host may not be executable by other users. Ensure that it is set to
be within the Dockerfile.
* lib/connections: Cache isLAN decision for later external access.
The check whether a remote device's address is on a local network
currently happens when handling the Hello message, to configure the
limiters. Save the result to the ConnectionInfo and pass it out as
part of the model's ConnectionInfo struct in ConnectionStats().
* gui: Use provided connection attribute to distinguish LAN / WAN.
Replace the dumb IP address check which didn't catch common cases and
actually could contradict what the backend decided. That could have
been confusing if the GUI says WAN, but the limiter is not actually
applied because the backend thinks it's a LAN.
Add strings for QUIC and relay connections to also differentiate
between LAN and WAN.
* gui: Redefine reception level icons for all connection types.
Move the mapping to the JS code, as it is much easier to handle
multiple switch cases by fall-through there.
QUIC is regarded no less than TCP anymore. LAN and WAN make the
difference between levels 4 / 3 and 2 / 1:
{TCP,QUIC} LAN --> {TCP,QUIC} WAN --> Relay LAN --> Relay WAN -->
Disconnected.
Without this, we tag the build as made by some random user account on some random host name which is not useful.
(And minor bug in the cache key which has no effect on the build itself.)
We had some unholy mix of our own logger and the stdlib logger, probably
because for historical reasons we wanted the device ID to stdout and the
rest to stderr? But that's not the case any more, and the mix of formats
is weird. Ideally I think the generate command should be silent and just
print the device ID and nothing else, but that's tricky to accomplish
since we have other methods do logging on their own. Hence this just
harmonizes it so that we at least use the same logger with the same
format and target...
gui: Add copy to clipboard, share by email, and share by SMS buttons to device IDs (fixes#2771, ref #3868)
Add buttons to allow for simpler sharing device IDs with others. The
first one copies the ID to clipboard (trying to use three different
methods, depending on the browser). The second one triggers a mailto
link with prefilled subject and body. The third one triggers an sms link
with prefilled body. The short description of Syncthing included in the
latter part of the body is a direct copy from the description at the
official website https://syncthing.net.
Issue #3868 is referred here, because the copy to clipboard button
offers an alternative method for IE11 users to actually be able to copy
device IDs without having to select it manually (which doesn't work).
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This is sort of a proof of concept, but since our current Windows
builder is down this might solve the problem. It includes a change for
easier code signing (taking the certificate in a secret/env var rather
than existing already on disk), but otherwise mirrors precisely what we
already do in the build server.
The connection type icon comes from Bootstrap. As such, it does not
follow the same dimensions as the other GUI icons, which come from Fork
Awesome. Thus, add left and right margin to make its width roughly the
same as the other GUI icons, which fixes its alignment in relation to
text.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Currently, a disconnected device is marked as "Disconnected" without any
consideration whether the state is just temporary or rather it has been
like that for a long time, potentially requiring user intervention.
This commit adds a new state called "Disconnected (Inactive)" which is
shown for devices that have been disconnected for longer than a week. It
also changes the "Last seen" information, so that "Never" is used only
when the device hasn't connected at all, and the exact date is displayed
otherwise.
Additionally, when the date is older than 1 week, a note about that is
displayed below the date. Furthermore, when the date becomes older than
1 month, the note text colour changes to orange, and when it exceeds 1
year, the colour changes again to red. This is done to provide the user
with a visual clue that something may potentially be wrong and requires
a manual fix.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Co-authored-by: André Colomb <src@andre.colomb.de>
Right now, the Out of Sync Items list for a specific remote device is
available only when the device is online and currently synchronising.
However, the list itself is being created by Syncthing all the time, so
there is no need to hide it from the user even when the remote device is
offline or paused. This way they can always easily check which files
exactly have not been synchronised yet.
In addition, the Out of Sync Items entry already includes file size next
to it, so there is no need to display it again next to the Sync Status
entry. Thus, remove out-of-sync file size from Sync Status, leaving only
percentage next to it.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This is sort of a proof of concept, but since our current Windows
builder is down this might solve the problem. It includes a change for
easier code signing (taking the certificate in a secret/env var rather
than existing already on disk), but otherwise mirrors precisely what we
already do in the build server.
In large setups, it is currently impossible to know the exact number of
folders and remote devices without counting them manually, either in the
GUI or in config.xml. Thus, to provide this information to the user, add
a specific number right next to both Folders and Remote Devices headers
in the Web GUI. The numbers are only displayed when two or more folders
or devices are present.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Previous debug input didn't really give enough info to show what was
happening, while it also printed full block lists which are enormously
verbose. Now it consistently prints 1. what it sees on disk, 2. what it
got from CurrentFile (without blocks), 3. the action taken on that file.
Some WebKit browsers select more than needed when using double click to
select device IDs, e.g. new lines and white space. This commit adds a
prefixed version of user-select in CSS in order to add support for those
browsers and allow them to select just device IDs automatically.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
There are some situations where an upgrade wouldn't be supported, even though the noUpgrade bool isn't set. So when handling the errors that are caused by this, when attempting an upgrade, it shouldn't lead to some sort of offline-message/restart/warning/etc...
I added some checks on specific errors related to this and return a 501 (Not Implemented) response instead, in case of an "UpgradeUnsupported"-error. Additionally, on the GUI-side, the 501-response is now not to be considered an error to act upon.
* implement authentication via token for relaysrv
Make replaysrv check for a token before allowing clients to
join. The token can be set via the replay-uri.
* fix formatting
* key composite literal
* do not error out if auth material is provided but not needed
* remove unused method receiver
* clean up unused parameter in functions
* cleaner token handling, disable joining the pool if token is set.
* Keep backwards compatibility with older clients.
In prior versions of the protocol JoinRelayRequest did not have a
token field. Trying to unmarshal such a request will result in
an error. Return an empty JoinRelayRequest, that is a request
without token, instead.
Co-authored-by: entity0xfe <entity0xfe@my.domain>
Currently, the link to remote GUI uses device addresses as they are
advertised from the router. Because of this, they may end up having a
scope ID attached to them. The problem is that browsers do not support
such addresses, leaving the user with a non-working URL.
Because of the above, use regex to simply filter out the scope ID from
the address before using it for Remote GUI.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Currently, the QR code image has no size specified in the HTML. This
causes other elements of the modal to jump around when opening it for
the first time and waiting for the QR image to generate. The jumping is
especially noticeable when the GUI is accessed remotely and there is a
delay while loading the elements due to slow connection.
In addition, capitalise "QR" in the alt text. This is necessary for
accessibility reasons, e.g. when using a screen reader, which may read
upper- and lowercase characters differently.
Lastly, allow to translate the alt text itself.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Currently, a custom JS script is used to select the whole device ID on
click. However, the current script isn't compatible with all browsers
(and in IE in particular), making it impossible to select the ID in them
at all. Additionally, the same functionality is already available in CSS
with no such drawbacks, as the whole selection process is handled by the
Web browser natively, which is lightweight and does not require custom
code.
Thus, remove the currently used JS script completely, replacing it with
a new CSS class that can be added to an element when required. If the
browser does not support the CSS, the user can still select the element
manually, which makes it safer than the current behaviour that can block
the user from being able to select the element at all.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The restore function of Trash Can ran a rename at the end regardless of whether there was anything to rename. In this case, when the file-to-be-restored did not exist in the destination folder, this resulted in an error. I added a simple check, keeping track of whether the file existed prior to restoring it in the destination folder and depending on this value it will now return nil after the restoration to prevent the renaming function to kick off. Added a test for this specific edge-case as well.
For consistency with syncOwnership explanation right above this one, the
sentence should talk about "sending ownership information", and not just
"sending ownership".
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Duplicate the regular expression for single and double quotes.
Support additional arguments (string substitution) in both variants.
Simplify the translation string group matching by using a lazy
quantifier instead of excluding the quote itself.
This adds support for syncing extended attributes on supported
filesystem on Linux, macOS, FreeBSD and NetBSD. Windows is currently
excluded because the APIs seem onerous and annoying and frankly the uses
cases seem few and far between. On Unixes this also covers ACLs as those
are stored as extended attributes.
Similar to ownership syncing this will optional & opt-in, which two
settings controlling the main behavior: one to "sync" xattrs (read &
write) and another one to "scan" xattrs (only read them so other devices
can "sync" them, but not apply any locally).
Co-authored-by: Tomasz Wilczyński <twilczynski@naver.com>
The cleaning logic in util.go was used by Simple and Trashcan but only
really suited Trashcan since it works based on mtimes which Simple does
not use. The cleaning logic in util.go was moved to trashcan.go.
Staggered and Simple seemed to be able to benefit from the same base so
util.go now has the base for those two with an added parameter which
takes a function so it can still handle versioner-specific logic to
decide which files to clean up. Simple now also correctly cleans files
based on their time-stamp in the title together with a specific maximum
amount to keep. The Archive function in Simple.go was changed to get rid
of duplicated code.
Additionally the trashcan testcase which was used by Trashcan as well as
Simple was moved from versioner_test.go to trashcan_test.go to keep it
clean, there was no need to keep it in a separate test file
Currently, the code contains a "mobile phone" fix to allow wrapping of
long lines in table heading and cells. However, the fix is applied to
all screen sizes equal or below 768 px wide, which causes the layout to
break on tablet-sized screens.
The commit moves the "mobile" fix to the actual mobile media query,
which is applied to screens up to 419 px wide. It is only really needed
there, where it synergises with the existing fix that changes table cell
display to "block". There is no need to wrap the text on larger screens,
as there is more than enough space to display the lines in full on them.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Add a commented entry to the systemd service file templates to point
the user in the right direction when using syncOwnership and starting
via systemd. Which is more upgrade-friendly than setting caps on the
executable directly, as mentioned in the docs.
The current code checks whether the same-named item exists in the tree,
and when it does, it re-uses it when adding new children to it. However,
the code doesn't check whether the existing item is a folder or a file.
It rather assumes that it is always a folder, which is not necessarily
the case.
This commit adds a new check to the code, so that the existing element
is reused only when it is a folder, and ignored otherwise.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Currently, the error message is often quite long and thus it appears
truncated with no possibility for the user to view the full string.
Thus, add a tooltip that displays the message in full on hover. This
follows the convention used in other parts of the GUI in similar cases.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This replaces old style errors.Wrap with modern fmt.Errorf and removes
the (direct) dependency on github.com/pkg/errors. A couple of cases are
adjusted by hand as previously errors.Wrap(nil, ...) would return nil,
which is not what fmt.Errorf does.
Locally paused folders will fail on checkFolderRunningLocked() and
therefore abort the loop. Avoid this by skipping paused folders
directly.
Co-authored-by: Jakob Borg <jakob@kastelo.net>
These strings no longer exist in the GUI, hence there is no need to keep
them in the translation json files either.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Co-authored-by: Jakob Borg <jakob@kastelo.net>
* lib/locations: Fix enum values camelCase.
* lib/locations: Remove unused FailuresFile.
* cmd/syncthing: Turn around role of locations storage.
Previously the locations package was used to provide default paths,
possibly with an overridden home directory. Extra paths supplied on
the command line were handled and passed around in the options object.
To make the changed paths available to any other interested package,
override the location setting from the option if supplied, instead of
vice versa when not supplied. Adapt code using this to read from the
locations package instead of passing through the options object.
* lib/locations: Refactor showPaths to locations package.
Generate a reusable string in locations.PrettyPrintPaths().
Enumerating all possible locations in different packages is error
prone, so add a new public function to generate the listing as a
string in the locations package. Adapt cmd/syncthing --paths to use
that instead of its own console output.
* lib/locations: Include CSRF token in pretty printed paths.
* lib/api: New endpoint /rest/system/paths.
The paths should be available for troubleshooting from a running
instance. Using the --paths CLI option is not easy in some
environments, so expose the locations mapping to a JSON endpoint.
Add utility function ListExpandedPaths() that also filters out any
entries which still contain variable placeholders.
* gui: List runtime paths in separate log viewer tab.
* Wrap paths.
* lib/syncthing: Utilize locations.Get() instead of passing an arg.
* Include base directories, move label to table caption.
* gui: Switch to hard-coded paths instead of iterating over all.
* gui: Break aboutModalView into tabs.
Use tabs to separate authors from included third-party software.
* gui: Move paths from log viewer to about modal.
* lib/locations: Adjust pretty print output order to match GUI.
* gui, authors: Remove additional bot names and fix indent.
The indentation changed because of the tabbed about dialog, fix the
authors script to respect that.
Skip Syncthing*Automation in authors list as well.
* Update AUTHORS list to remove bot names.
* Revert AUTHORS email order change.
* Do not emphasize DB and log file locations.
* Review line wrapping.
* review part 1: strings.Builder, naming
* Rename and extend locations.Set() with error handling.
Remodel the Override() function along the existing SetBaseDir() and
rename it to simply Set(). Make sure to use absolute paths when given
log file or GUI assets override options. Add proper error reporting
if that goes wrong.
* Remove obsolete comment about empty logfile option.
* Don't filter out unexpanded baseDir placeholders, only ${timestamp}.
* Restore behavior regarding special "-" logfile argument.
If the option is given, but with empty value, assume the no log
file (same as "-"). Don't try to convert the special value to an
absolute path though and document this fact in a comment for the Set()
function.
* Use template to check for location key validity.
* Don't filter out timestamp placeholders.
* lib/api: Remove paths from /rest/system/status.
* lib/ur: Properly initialize map in failure data (fixes#8479)
Co-authored-by: Jakob Borg <jakob@kastelo.net>
Currently the `Run()` function of the CLI always uses `os.Args` directly.
This change adds an additional `RunWithArgs()` function that allows passing
arguments as `[]string` instead. This is useful when linking against
Syncthing as a library to be able to also expose the CLI.
Also adds idle time and keepalive parameters because how this is
configured has changed in the new package version. The values are those
that seems like might already be default, if keep-alives were enabled,
which is not obvious from the doc comments.
Also, Go 1.19 gofmt reformatting of comments.
all: Add package runtimeos for runtime.GOOS comparisons
I grew tired of hand written string comparisons. This adds generated
constants for the GOOS values, and predefined Is$OS constants that can
be iffed on. In a couple of places I rewrote trivial switch:es to if:s,
and added Illumos where we checked for Solaris (because they are
effectively the same, and if we're going to target one of them that
would be Illumos...).
Currently, there are two issues with the detailed staggered versioning
information displayed in the folder info. Firstly, the maxAge value of
365d is displayed despite matching the default. Secondly, there is no
consideration for the special case of maxAge equal to 0, meaning that
versions are kept forever.
This commit fixes both of those issues, so that the default maxAge is
not displayed and its value of 0 is displayed as "forever".
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This adds support for syncing ownership on Unixes and on Windows. The
scanner always picks up ownership information, but it is not applied
unless the new folder option "Sync Ownership" is set.
Ownership data is stored in a new FileInfo field called "platform data". This
is intended to hold further platform-specific data in the future
(specifically, extended attributes), which is why the whole design is a
bit overkill for just ownership.
* lib/model: Override scan config for auto-accepted encrypted folders.
Encrypted folders should not have the fs watcher enabled and rarely
benefit from a scheduled rescan. The GUI adjusts the suggested
settings (watcher disabled, one day rescan interval) when accepting a
receive-encrypted folder. Mirror that behavior to the auto-accept
case where the GUI is not involved.
Versioning also does not work well for encrypted folders, same
treatment.
Currently, the filesystem watcher explanation in the Advanced tab in the
Edit Folder modal window is split into two parts. The first is location
in a tooltip that is visible only when hovering the mouse over the small
checkbox, which makes it not easily accessible, e.g. for new or touch
screen users.
No other settings in the Edit Folder window have their explanation
presentend like that. When needed, it is always displayed in their
respective help blocks, which are always visible on the screen. Thus,
move the filesystem watcher explanation from the tooltip to the help
block, merging it with the other part of the explanation in the process.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
* gui: Use discovered IDs from cache when adding a new remote device.
The GUI already does refreshDiscoveryCache() when it comes online and
on every 10 second refresh cycle. Querying the same information
freshly in addDevice() seems unnecessary and adds some more round-trip
delay on a possibly slow network link. Instead use the IDs from the
existing discoveryCache property to populate the suggested ID list.
* Increase maximum suggested device ID count to 100.
For the auto-completion list, which is hidden by default and filtered
by the browser, we can offer more discovered device IDs without
causing much confusion. The list of suggested "nearby" devices is
still limited to the first five.
* Rename $scope.discoveryUnknown.
The old name "discovery" was pretty ambiguous..
This commit updates Fork Awesome from version 1.1.2 from 2018 to version
1.2.0 from 2021. The changes are few and non-breaking, consisting mainly
of new icon additions.
It is worth to note that the new version includes also a Syncthing icon,
which was not present before.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
An off-by-one error could cause tokens to be forgotten. Suppose
tokens := []string{"foo", "bar", "baz", "quux"}
i := 2
token := tokens[i] // token == "baz"
Then, after
copy(tokens[1:], tokens[:i+1])
tokens[0] = token
we have
tokens == []string{"baz", "foo", "bar", "baz"}
The short test actually relied on this bug.
Similar to the "remote has not accepted sharing" message, add a
footnote number 2 to indicate a folder which will not sync with a
certain device because the remote has paused it. Applies to the edit
folder / device modals' sharing tab, as well as the "shared with"
listing and tooltips under device and folder details.
Use the proper encoding function in the relay server when constructing
the URL. In the pool server, parse and re-encode the query values to
sanitize whatever the client sent.
Currently, the "Latest Change" translation string is hard-coded to use
the English-like word order of V ("deleted") + N ("file"). As such, it
is incompatible with languages that require to use a different word
order, e.g. Korean or Japanese. In other words, a proper translation of
the string to those languages is currently impossible.
This commit changes the translation string, so that it includes the file
variable, and thanks to this, it can be easily adopted to languages with
different word order than English.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This commit replaces `os.MkdirTemp` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `os.MkdirTemp`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Showing all folders from disconnected or paused remote devices as
unaccepted would be a lot of false positives. As we cannot know
whether the remote has accepted while it doesn't have an active
connection, let's better report false negatives, as in assuming the
folders are accepted.
* lib/api: Note ItemStarted and ItemFinished for default filtering.
The reasoning why LocalChangeDetected and RemoteChangeDetected events
are not included in the event stream by default (without explicit
filter mask requested) also holds for the ItemStarted and ItemFinished
events. They should be excluded as well when we start to break the
API compatibility for some reason.
* gui: Enumerate unused event types in the eventService.
Define constants for the unused event types as well, for completeness'
sake. They are intentionally not handled in the GUI currently.
* cmd/syncthing: Harmonize uppercase CLI argument placeholders.
Use ALL-UPPERCASE and connecting dashes to distinguish argument
placeholders from literal argument options (e.g. "cpu" or "heap" for
profiling). The dash makes it clear which words form a single
argument and where a new argument starts.
This style is already used for the "syncthing cli debug file" command.
* lib/model: Simplify event data structure.
Using map[string]interface{} is not necessary when all values are
known to be strings.
Having a separate mutex for the three or four instructions needed to
fetch and increment nextID means the overhead exceeds the cost of this
operation. nextID is now handled inside the critical section for
awaiting instead, while the more expensive channel creation has been
moved outside it.
This is mostly a simplification, though it may have minor performance
benefits in some situations. The single-threaded sender benchmark shows
no significant difference:
name old speed new speed delta
RequestsRawTCP-8 55.3MB/s ± 7% 56.6MB/s ± 6% ~ (p=0.190 n=10+10)
RequestsTLSoTCP-8 20.5MB/s ±20% 20.8MB/s ± 8% ~ (p=0.604 n=10+9)
Add a link next to each setting's label to its explanation in the Docs.
This way, it is easy to quickly check what the setting is about without
going to the Docs site separately and searching for it manually.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Right now, the Trash Can versioning info text is displayed for both the
Trash Can and Simple versioning. However, the Simple versioning has its
own info text that is also displayed, which results in two very similar
sentences being shown on the screen.
This commit limits the Trash Can info text to be displayed only when the
Trash Can versioning is selected, and moves the Simple versioning info
text up to the same location as the other one.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Even though technically possible, CJK languages normally don't use
italic text at all, as not only does it make the characters/letters look
unnatural, but also, in the case of complex characters, unreadable too.
For these reasons, it is usually recommended not to use the italic font
style at all [1][2].
This commit changes the default font-style of the i element for Chinese,
Japanese, and Korean langauge to "normal" instead of "italic". In order
to do so, the HTML lang attribute is also changed following each change
of the GUI language.
[1] https://bobtung.medium.com/best-practice-in-chinese-layout-f933aff1728f
[2] https://devblogs.microsoft.com/oldnewthing/20060914-02/?p=29743
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Staggered File Versioning used to have its own cleanInterval that
controlled how often file versions were cleaned. Nowadays, there is a
seperate setting called cleanupIntervalS responsible for the cleanup,
which applies to all File Versioning (except External). Thus, remove the
unneeded code and don't set the param up on new folders anymore.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
* gui: Allow to translate and fix incorrect Versions date filter ranges
Translate the previously English-only ranges used in Versions date
filter. In the process, fix the currently incorrect range calculation.
For instance, let us say it is 08:05. Selecting "today" should set the
range to start at 00:00 and end at 08:05. However, what really happens
is that both start and end are set to 08:05, and as a result "today" is
never shown to the user. The case is the same for "yesterday", which in
contrary to "today" is shown, but its range is fixed at 08:05 on the
previous day.
This commit fixes the above by always calculating "today" starting at
00:00 up to the current moment, and calculating "yesterday" from 00:00
on the previous to 00:00 on the current day.
When it comes to "last x days", the commit fixes the calculation so that
it actually covers the whole day, which is done by moving the start date
to 00:00 on the first day.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The ignoreDelete warning code originally comprised of two sentences,
which in total were much longer than the current one. Because of that,
some additional fixes were added to make it present itself better, e.g.
white-space was set to normal to allow for text wrap on narrow screens,
and the help link was moved to a new line to make sure both the anchor
and the question mark always stayed together.
However, the code fixes remained despite the final text being much
shorter than the original. Therefore, remove the unnecessary white-space
formatting, and also move the help link right next to the warning. This
makes it clear that the link is related to this particular warning when
both it and the ignore patterns warning are displayed at the same time.
Ref: https://github.com/syncthing/syncthing/pull/8054#issuecomment-978148117
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
* gui: Translate fancytree messages in Versions modal
Currently, the default fancytree info/error messages are used. This
means that a) they are English-only, and b) they are very generic. With
this commit, the messages are added to the translatable strings, and
they are also more specifically related to file versioning.
On a side note, the "moreData" string has been left out on purpose, as
it is not used in the current code. It can be added later if needed.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
* gui: Make ID read-only and hide nearby devices when adding pending devices (fixes#8083)
Currently, there is no distinction between adding "new" and "pending"
devices. For this reason, the user is always presented with a list of
"nearby devices" to choose from. This commit adds such distinction to
the code, and in the case of "pending" device, both the device ID is
made read-only and the nearby devices list is hidden.
As a by-product of the function rename and clean-up, this commit also
hides the non-functional "remove" button that is shown when editing
device defaults.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The intention was that if no peers are given, we shouldn't start the
listener. We did that anyway, because:
- splitting an empty string on comma returns a slice with one empty
string in it
- parsing the empty string as a device ID returns the empty device ID
so we end up with a valid replication peer which is the empty device ID.
* lib/protocol: Require at least 3.125% savings from compression
The new lz4 library doesn't need its output buffer to be the maximum
size, unlike the old one (which would allocate if it weren't). It can
take a buffer that is of a smaller size and will report if compressed
data can fit inside the buffer (with a small chance of reporting a false
negative). Use that property to our advantage by requiring compressed
data to be at most n-n/32 = .96875*n bytes long for n input bytes.
* lib/protocol: Remove unused receivers
To make DeepSource happy.
* lib/protocol: Micro-optimize lz4Compress
Only write the length if compression was successful. This is a memory
write, so the compiler can't reorder it.
Only check the return value of lz4.CompressBlock. Length-zero inputs
are always expanded by LZ4 compression (the library documents this),
so the check on len(src) isn't needed.
When GOBIN is set, 'go install' cannot install cross-compilied binaries.
To satisfy cross-compilation, it's necessary to add the '-o' to build
target, otherwise 'go build' will discarding the resulting objects when
compiling multiple packages.
Signed-off-by: bekcpear <i@bitbili.net>
* lib/model: Remove bogus fields from connections API endpoint.
Switch the returned data type for the /rest/system/connections element
"total" to use only the Statistics struct. The other fields of the
ConnectionInfo struct are not populated and misleading.
* Lowercase JSON field names.
* lib/model: Get rid of ConnectionInfo.MarshalJSON().
It was missing the StartedAt field from the embedded Statistics
struct. Just lowercasing the JSON attribute names can be done just as
easily with annotations.
* lib/model: Remove bogus startedAt field from totals.
Instead of using the Statistics type with one field empty, just switch
to a free-form map with the three needed fields.
* cmd/syncthing: Remove unnecessary function arguments.
The openGUI() function does not need a device ID to work, and there is
only one caller anyway which uses EmptyDeviceID.
The loadOrDefaultConfig() function is always called with the same
dummy values.
* cmd/syncthing: Avoid misleading info messages from monitor process.
In order to check whether panic reporting is enabled, the monitor
process utilizes the loadOrDefaultConfig() function. In case there is
no config file yet, info messages may be logged during creation if the
config Wrapper, which is discarded immediately after.
Stop using the DefaultConfig() utility function from lib/syncthing and
directly generate a minimal config instead to avoid these.
Add comments to loadOrDefaultConfig() explaining its limited purpose.
* cmd/syncthing/generate: Always write updated config file.
Previously, an existing config file was left untouched unless either
of the --gui-user or --gui-password options was given. Remove that
condition and simplify the checking code.
* lib/config: Factor out ProbeFreePorts().
* cmd/syncthing: Add option --skip-port-probing.
Applies to both the "generate" and "serve" subcommands, as well as the
deprecated --generate option, just as the --no-default-folder flag.
* cmd/syncthing: Remove unnecessary function arguments.
The openGUI() function does not need a device ID to work, and there is
only one caller anyway which uses EmptyDeviceID.
The loadOrDefaultConfig() function is always called with the same
dummy values.
* cmd/syncthing: Avoid misleading info messages from monitor process.
In order to check whether panic reporting is enabled, the monitor
process utilizes the loadOrDefaultConfig() function. In case there is
no config file yet, info messages may be logged during creation if the
config Wrapper, which is discarded immediately after.
Stop using the DefaultConfig() utility function from lib/syncthing and
directly generate a minimal config instead to avoid these.
Add comments to loadOrDefaultConfig() explaining its limited purpose.
* cmd/syncthing/generate: Always write updated config file.
Previously, an existing config file was left untouched unless either
of the --gui-user or --gui-password options was given. Remove that
condition and simplify the checking code.
Use indexOf instead of startsWith to make the now translatable theme
names appear correctly in IE11. This also prevents console log error
spam in the browser. The same problem was previously reported in #6940.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
When pathSep is a constant, the compiler precomputes pathSep+pathSep and
".."+pathSep instead of emitting function calls to compute "//" and
"../". Benchmark results in lib/osutil:
name old time/op new time/op delta
TraversesSymlink-8 8.86µs ± 3% 8.53µs ± 4% -3.79% (p=0.000 n=18+20)
name old alloc/op new alloc/op delta
TraversesSymlink-8 1.06kB ± 0% 1.06kB ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
TraversesSymlink-8 15.0 ± 0% 15.0 ± 0% ~ (all equal)
The locking protocol in nat.Mapping was racy:
* Mapping.addressMap RLock'd, but then returned a map shared between
caller and Mapping, so the lock didn't do anything.
* Operations inside Service.{verifyExistingMappings,acquireNewMappings}
would lock the map for every update, but that means callers to
Mapping.ExternalAddresses can be looping over the map while the
Service methods are concurrently modifying it. When the Go runtime
detects that happening, it panics.
* Mapping.expires was read and updated without locking.
The Service methods now lock the map once and release the lock only when
done.
Also, subscribers no longer get the added and removed addresses, because
none of them were using the information. This was changed for a previous
attempt to retain the fine-grained locking and not reverted because it
simplifies the code.
Accept a subcommand as an alternative to the --generate option. It
accepts a custom config directory through either the --home or
--config options, using the default location if neither is given.
Add the options --gui-user and --gui-password to "generate", but not
the "serve --generate" option form. If either is given, an existing
config will not abort the command, but rather load, modify and save it
with the new credentials. The password can be read from standard
input by passing only a single dash as argument.
Config modification is skipped if the value matches what's already in
the config.
* cmd/syncthing: Utilize lib/locations package in generate().
Instead of manually joining paths with "magic" file names, get them
from the centralized locations helper lib.
* cmd/syncthing: Simplify logging for --generate option.
Visible change: No more timestamp prefixes.
What hash is used to store the password should ideally be an
implementation detail, so that every user of the GUIConfiguration
object automatically agrees on how to handle it. That is currently
distribututed over the confighandler.go and api_auth.go files, plus
tests.
Add the SetHasedPassword() / CompareHashedPassword() API to keep the
hashing method encapsulated. Add a separate test for it and adjust
other users and tests. Remove all deprecated imports of the bcrypt
package.
LoadOrGenerateCertificate() takes two file path arguments, but then
uses the locations package to determine the actual path. Fix that
with a minimally invasive change, by using the arguments instead.
Factor out GenerateCertificate().
The only caller of this function is cmd/syncthing, which passes the
same values, so this is technically a no-op.
* lib/tlsutil: Make storing generated certificate optional. Avoid
temporary cert and key files in tests, keep cert in memory.
Consistently use double dashes and fix typos -conf, -data-dir and
-verify.
Applies also to tests running the syncthing binary for consistency.
* Fix mismatched option name --conf in cli subcommand.
According to the source code comments, the cli option flags should
mirror those from the serve subcommand where applicable. That one is
actually called --config though.
* cli: Fix help text option placeholders.
The urfave/cli package uses the Value field of StringFlag to provide a
default value, not to name the placeholder. That is instead done with
backticks around some part of the Usage field.
* cli: Add missing --data flag in subcommand help text.
The urfave/cli based option parsing uses a fake flags collection to
generate help texts matching the used global options. But the --data
option was omitted from it, although it is definitely required when
using --config as well. Note that it cannot just be ignored, as some
debug stuff actually uses the DB:
syncthing cli --data=/bar --config=/foo debug index dump
By truncating time.Time to an int64 nanosecond count, we lose the
ability to precisely order timestamps before 1678 or after 2262, but we
gain (linux/amd64, Go 1.17.1):
name old time/op new time/op delta
JobQueuePushPopDone10k-8 2.85ms ± 5% 2.29ms ± 2% -19.80% (p=0.000 n=20+18)
JobQueueBump-8 34.0µs ± 1% 29.8µs ± 1% -12.35% (p=0.000 n=19+19)
name old alloc/op new alloc/op delta
JobQueuePushPopDone10k-8 2.56MB ± 0% 1.76MB ± 0% -31.31% (p=0.000 n=18+13)
name old allocs/op new allocs/op delta
JobQueuePushPopDone10k-8 23.0 ± 0% 23.0 ± 0% ~ (all equal)
Results for BenchmarkJobQueueBump are with the fixed version, which no
longer depends on b.N for the amount of work performed. rand.Rand.Intn
is cheap at ~10ns per iteration.
Like Windows and Mac, Android is also an interactive operating system.
On top of that, it usually runs on much slower hardware than the other
two. Because of that, it makes sense to limit the number of hashes used
by default there too.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Add each subdirectory of the guiDir as a translation candidate string.
The key is prefixed with "theme-name-" and the default English
translation corresponds to the directory name turned to title case.
Disable the automatic name mangling in the GUI JS code in favor of
just looking up the translation.
Registry.Get used a full sort to get the minimum of a list, and the sort
was broken because util.AddressUnspecifiedLess assumed it could find out
whether an address is IPv4 or IPv6 from its Network method. However,
net.(TCP|UDP)Addr.Network always returns "tcp"/"udp".
Currently, the default sorting of the file list in the Restore Versions
modal in both case sensitive, and it also does not distinguish between
files and folders. With this change, which uses a slightly modified code
from the fancytree itself (with added folder prioritisation), the sort
becomes case insensitive and also always places folders on top. This is
to make it behave more similar to what file managers do in the commonly
used operating systems (including popular Linux desktop environments).
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This reverts commit cca17f5306.
Using textarea instead of input makes it possible to enter new lines,
which then are added to config.xml as "
". This breaks the whole
script, because these characters are passed to the command line as "\n".
Therefore, the script should rather remain a single-lined input field.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The current detection is flawed, because it looks for a few specific
file systems like "msdos" or "fat" to set the mtime window, while in
reality Android seems to report names like "fuseblk", which can stand
for fat, ext4, or even f2fs.
At the moment, we set the mtime window only for a few known names used
for the fat filesystem. With this change, we take a safer approach of
always setting the time window unless we explicitly detect file systems
like ext2/ext3/ex4, which are known not to experience issues with moving
timestamps on Android.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The list of unshared devices is built from deviceList(), which sorts
the result. But the shared devices are listed in undefined order from
the unsorted currentFolder.devices array. Apply the same sorting
after building the array used in the dialog.
Currently, the dismiss button is displayed as the first of the three
buttons. However, the most common action that the user wants to do when
sharing a new folder is to add it on a different device. Because of
this, the add button should be displayed first as the most prominent of
the three. The ignore button is the opposite of the add button, and also
results in a parmenent action, hence it makes sense to lump the two
together. Thus, the dismiss button should be moved to the last place as
an alternative to the two main actions, when the user is yet unsure what
they want to do with the notification.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
1. Change each modal title text to match the action that is being
executed (i.e. "Revert" to "Revert Local Additions", "Override" to
"Override Changes", "Delete" to "Delete Unexpected Items").
2. Change the icons to match the icons used by each action (i.e. arrow-
circle-down for Revert, arrow-circle-up for Override). Replace the
broken lock icon for Delete with minus-circle.
3. Rearrange the order in the modal HTML code to simplify it a little.
Before this patch, IPv4-compatible addresses (::ffff:aaa.bbb.ccc.ddd)
may be used if a quic6://some.domain:port is specified and both IPv4 and
IPv6 addresses exist for that domain name.
This changes the GC mechanism so that the first pass (which reads all
FileInfos to populate bloom filters with block & version hashes) can
happen concurrently with normal database operations.
The big gcMut still exists, and we grab it temporarily to block all
other modifications while we set up the bloom filters. We then release
the lock and let other things happen, with those other things also
updating the bloom filters as required. Once the first phase is done we
again grab the gcMut, knowing that we are the sole modifier of the
database, and do the cleanup.
I also removed the final compaction step.
* cmd/strelaypoolsrv: Fix minor grammar, use https in links
Add a few minor grammatical/stylistic fixes. Use `https` instead of
`http` in the MaxMind link in the footer.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
* wip
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Disable the Versions button when the folder is paused, because it does
not work, i.e. the versioned files are not loaded. The folder needs to
be unpaused to actually be able to view the versioned file list.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
* Trigger connection loop on config device addition (fixes#7600)
* Also check for device address equality
* Move EqualStrings from api_test to utils, and use in connections/service.go
* Make sure CommitConfiguration cannot block due on the deviceAddressesChanged channel
* Update lib/connections/service.go
Co-authored-by: Jakob Borg <jakob@kastelo.net>
* cmd/syncthing: Don't fail early on api setup error (fixes 7558)
* switch to factory pattern
* refactor config command to show help on nothing
* wip
* wip
* already abort in before
This makes us use TLS 1.3+ on sync connections by default. A new option
`insecureAllowOldTLSVersions` exists to allow communication with TLS
1.2-only clients (roughly Syncthing 1.2.2 and older). Even with that
option set you get a slightly simplified setup, with the cipher suite
order fixed instead of auto detected.
* cmd/syncthing: Don't fail early on api setup error (fixes 7558)
* switch to factory pattern
* refactor config command to show help on nothing
* wip
* wip
* already abort in before
No longer hide the web UI controls for the new untrusted/encrypted
device feature. Testing hasn't been very widespread, but there has been
some and quite a few bugs have been caught and fixed. I believe its time
to not hide it anymore, and cautiously recommend usage. E.g. mention
that the feature hasn't been widely used yet and anyone using it is an
early adopter, but drop the bit about not using it with production data.
We can maybe stress the need for backups in general and especially
using this.
Remove the animation due to its excessive CPU usage, especially when a
large number of files is being downloaded and listed at the same time.
Also, remove the stripes, as they serve no purpose in the now-static
progress bar.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
There was a logic mistake, so the limit in question wasn't used. On my
macOS this doesn't seem to matter, the hard limit returned is 2^63-1 and
setting the soft limit to that works. However I'm assuming that's not
the case for older macOSes since it was so nicely documented, so we
should still have this working. (10240 FDs should be enough for
anybody.)
This is a mostly pointless change to make security scanners and static
analysis tools happy, as they all hate seeing md5. None of our md5 uses
were security relevant, but still. Only visible effect of this change is
that our temp file names for very long file names become slightly longer
than they were previously...
The current text gives an impression that we are currently using a
Receive Encrypted folder, even if we are not. Thus, make the current
text displayed only when the folder is in fact Receive Encrypted, and
add a new string to be displayed when using different folder types.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This adds a couple of dummy asset files protected by the "noassets"
build tag. The purpose is that it should be possible for, for example,
CI tools and static analysis things to compile and analyze the source
tree without our custom asset generation step. Also makes `go test -tags
noassets ./...` work without building assets first.
Disabled options are currently barely distinguishable from enabled
ones. This changes their background to grey, following the Bootstrap
defaults already used for disabled <select>.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
This truncates times meant for API consumption to second precision,
where fractions won't typically matter or add any value. Exception to
this is timestamps on logs and events, and of course I'm not touching
things like file metadata.
I'm not 100% certain this is an exhaustive change, but it's the things I
found by grepping and following the breadcrumbs from lib/api...
I also considered general-but-ugly solutions, like having the API
serializer itself do reflection magic or even regexps on returned
objects, but decided against it because aurgh...
This adds a button in the top right that changes the config back to the
default theme.
Code wise, it takes the header that was previously a part of the
dashboard component and moves it to the app component, and then adds the
button there. Possibly the header should be a component of it's own, but
that's more of refactor that can happen separately I think.
The config change uses the new config API to just patch the relevant
setting.
I'm not doing an automatic reload because 1) I don't want to figure out
how to do it correctly and 2) this doesn't work reliably anyway, as
for example the current gen GUI does a reload and ends up with
connection refused as the API service is still reloading...
* Provide a sysctl config to raise max UDP buffer size
* Add sysctl config to deb
* Check if `deb-systemd-invoke` is available
Co-authored-by: otbutz <tbutz@optitool.de>
This loosens the ‘is this localhost?’ check to include *.localhost host
names.
This allows for clearer (hence better) names to be used in browsers,
e.g. when accessing a remote syncthing instance ‘foo’ using a ssh port
forward, one can use foo.localhost to remind oneself which one is which.
💡 Without these changes, Syncthing shows a ‘Host check error’ when
pointing a browser at http://foo.localhost/, and with these changes, the
interface loads as usual.
The .localhost top level domain is a reserved top-level domain (RFC 2606):
> The ".localhost" TLD has traditionally been statically defined in
> host DNS implementations as having an A record pointing to the
> loop back IP address and is reserved for such use. Any other use
> would conflict with widely deployed code which assumes this use.
> – https://tools.ietf.org/html/rfc2606
As Wikipedia puts it:
> This allows the use of these names for either documentation purposes
or in local testing scenarios. – https://en.wikipedia.org/wiki/.localhost
On Linux systems, systemd-resolved resolves *.localhost, on purpose:
https://www.freedesktop.org/software/systemd/man/systemd-resolved.service.html
See also #4815, #4816.
* release:
all: Fix Microsoft documentation links in code comments (#7387)
gui: Handle info labels that are longer than available space (fixes#944) (#7386)
gui: Show "Last seen" at the top when device is disconnected (ref #7166) (#7373)
gui: Remove cleanupIntervalS leftovers from External Versioning (ref #7360) (#7378)
gui: Allow setting custom path for all versioning except external (#7377)
lib/fs: Consider options in case-fs caching (fixes#7371) (#7381)
lib/scanner: Pass on errors while hashing (#7380)
build: Update pfilter (#7376)
gui: Hide the Rescan All button when no folders exist (#7367)
lib/connections: Allow QUIC with Go 1.16 (#7372)
gui: Hide non-functional Versions button when using External Versioning (#7365)
gui, man, authors: Update docs, translations, and contributors
gui: Fix setting external versioning command (fixes#7361) (#7362)
lib/versioner: Improve error messages (fixes#7354) (#7357)
gui: Disable Cleanup Interval with External File Versioning as unsupported (#7360)
Apply to table headers the same code as already used for table data.
This way, the headers will be either pushed to the next line, or cut
with an ellipsis if the single word is too long.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Move the "Last seen" field to the very top in the device information.
This way, if a device has disconnected unexpectly, we can quickly check
the time when it was last available. Right now, due to the very long
address field, it is usually necessary to scroll down in order to view
the "Last seen" field.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
If there are no folders present, show only the "Add Folder" button, and
hide the "Rescan All" button. Only show the latter when at least one
folder exists.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Apply to table headers the same code as already used for table data.
This way, the headers will be either pushed to the next line, or cut
with an ellipsis if the single word is too long.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Move the "Last seen" field to the very top in the device information.
This way, if a device has disconnected unexpectly, we can quickly check
the time when it was last available. Right now, due to the very long
address field, it is usually necessary to scroll down in order to view
the "Last seen" field.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
If there are no folders present, show only the "Add Folder" button, and
hide the "Rescan All" button. Only show the latter when at least one
folder exists.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The button does nothing when the External Versioning is being used, so
it should not be displayed at all to avoid confusing the users.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
As for right now, the External File Versioning does not support Cleanup
Interval. Therefore, the option should no be available at all when using
it.
Ref: https://forum.syncthing.net/t/16346
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
The button does nothing when the External Versioning is being used, so
it should not be displayed at all to avoid confusing the users.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
As for right now, the External File Versioning does not support Cleanup
Interval. Therefore, the option should no be available at all when using
it.
Ref: https://forum.syncthing.net/t/16346
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
An untrusted device will receive padded info for small blocks, and hence
sometimes request a larger block than actually exists on disk.
Previously we let this pass because we didn't have a hash to compare to
in that case and we ignored the EOF error based on that.
Now the untrusted device does pass an encrypted hash that we decrypt and
verify. This means we can't check for len(hash)==0 any more, but on the
other hand we do have a valid hash we can apply to the data we actually
read. If it matches then we don't need to worry about the read
supposedly being a bit short.
Benchmark results on Linux/amd64, using updated benchmark for old and
new:
name old time/op new time/op delta
HashFile-8 88.6ms ± 1% 88.3ms ± 1% -0.33% (p=0.046 n=19+19)
name old speed new speed delta
HashFile-8 201MB/s ± 1% 202MB/s ± 1% +0.33% (p=0.044 n=19+19)
name old alloc/op new alloc/op delta
HashFile-8 59.4kB ± 0% 46.1kB ± 0% -22.47% (p=0.000 n=14+20)
name old allocs/op new allocs/op delta
HashFile-8 29.0 ± 0% 27.0 ± 0% -6.90% (p=0.000 n=20+20)
Co-authored-by: greatroar <@>
* lib/db: Add ExpirePendingFolders().
Use-case is to drop any no-longer-pending folders for a specific
device when parsing its ClusterConfig message where previously offered
folders are not mentioned any more.
The timestamp in ObservedFolder is stored with only second precision,
so round to seconds here as well. This allows calling the function
within the same second of adding or updating entries.
* lib/model: Weed out pending folders when receiving ClusterConfig.
Filter the entries by timestamp, which must be newer than or equal to
the reception time of the ClusterConfig. For just mentioned ones,
this assumption will hold as AddOrUpdatePendingFolder() updates the
timestamp.
* lib/model, gui: Notify when one or more pending folders expired.
Introduce new event type FolderOfferCancelled and use it to trigger a
complete refreshCluster() cycle. Listing individual entries would be
much more code and probably just as much work to answer the API
request.
* lib/model: Add comment and rename ExpirePendingFolders().
* lib/events: Rename FolderOfferCancelled to ClusterPendingChanged.
* lib/model: Reuse ClusterPendingChanged event for cleanPending()
Changing the config does not necessarily mean that the
/resut/cluster/pending endpoints need to be refreshed, but only if
something was actually removed. Detect this and indicate it through
the ClusterPendingChanged event, which is already hooked up to requery
respective endpoints within the GUI.
No more need for a separate refreshCluster() in reaction to
ConfigSaved event or calling refreshConfig().
* lib/model: Gofmt.
* lib/db: Warn instead of info log for failed removal.
* gui: Fix pending notifications not loading on GUI start.
* lib/db: Use short device ID in log message.
* lib/db: Return list of expired folder IDs after deleting them.
* lib/model: Refactor Pending...Changed events.
* lib/model: Adjust format of removed pending folders enumeration.
Use an array of objects with device / folder ID properties, matching
the other places where it's used.
* lib/db: Drop invalid entries in RemovePendingFoldersBeforeTime().
* lib/model: Gofmt.
My local gofmt did not complain here, strangely...
* gui: Handle PendingDevicesChanged event.
Even though it currently only holds one device at a time, wrap the
contents in an array under the "added" property name.
* lib/model: Fix null values in PendingFoldersChanged removed member.
* gui: Handle PendingFoldersChanged event.
* lib/model: Simplify construction of expiredPendingList.
* lib/model: Reduce code duplication in cleanPending().
Use goto and a label for the common parts of calling the DB removal
function and building the event data part.
* lib/events, gui: Mark ...Rejected events deprecated.
Extend comments explaining the conditions when the replacement event
types are emitted.
* lib/model: Wrap removed devices in array of objects as well.
* lib/db: Use iter.Value() instead of needless db.Get(iter.Key())
* lib/db: Add comment explaining RemovePendingFoldersBeforeTime().
* lib/model: Rename fields folderID and deviceID in event data.
* lib/db: Only list actually expired IDs as removed.
Skip entries where Delete() failed as well as invalid entries that got
removed automatically.
* lib/model: Gofmt
This splits the ignore getting to two methods, one that loads from disk
(the old one) and one that just returns whatever is already loaded (the
new one). The folder summary service which is just interested in stats
now uses the latter method. This means that it, and API calls that call
it, does not get blocked by folder I/O.
This adds two new configuration options:
// The number of connections at which we stop trying to connect to more
// devices, zero meaning no limit. Does not affect incoming connections.
ConnectionLimitEnough int
// The maximum number of connections which we will allow in total, zero
// meaning no limit. Affects incoming connections and prevents
// attempting outgoing connections.
ConnectionLimitMax int
These can be used to limit the number of concurrent connections in
various ways.
This adds a statistic to track the last connection duration per device.
It isn't used for much in this PR, but it's available for #7223 to use
in deciding how to order device connection attempts (deprioritizing
devices that just dropped our connection the last time).
The test would fail if the umask on UNIX is greater than 0022, because
the OS transparently subtracts it from the mode passed to Mkdir(), as
the Go documentation confirms.
Our goal here is not to test os.Mkdir(), so just make sure the desired
mode is actually set by forcing it afterwards.
This breaks out some methods from the connection loop to make it simpler
to manage and understand.
Some slight simplifications to remove the `seen` variable (we can filter
`nextDial` based on times are in the future or not, so we don't need to
track `seen`) and adding a minimum loop interval (5s) in case some
dialer goes haywire and requests a 0s redial interval or such.
Otherwise no significant behavioral changes.
This does two things:
- Exclude QUIC from go1.16 builds, automatically, for now, since it
doesn't work and just panics.
- Provide some fake listeners and dialers when QUIC is disabled.
These fake listeners and dialers indicate that they are disabled and
unsupported, which silences "Dialing $address: unknown address scheme:
quic" type of stuff which is not super helpful to the user.
This fixes a regression by restoring two functions removed in commit
a20d85d451, but still used from the HTML
template. Adjust the restored versions to access the new flag storage
structure.
The replacement functions selectAllSharedFolders() and
selectAllUnrelatedFolders() are not functional, so this is just a
quick fix to restore a working state before making the new functions
actually work sensibly.
Just like the respective functions for sharing device selections, give
it a boolean argument instead of writing two almost identical
functions.
In line with the other selectAll...() functions, avoid calling
angular.forEach() and iterate over the folders object directly.
This fixes a regression by restoring two functions removed in commit
a20d85d451, but still used from the HTML
template. Adjust the restored versions to access the new flag storage
structure.
The replacement functions selectAllSharedFolders() and
selectAllUnrelatedFolders() are not functional, so this is just a
quick fix to restore a working state before making the new functions
actually work sensibly.
Just like the respective functions for sharing device selections, give
it a boolean argument instead of writing two almost identical
functions.
In line with the other selectAll...() functions, avoid calling
angular.forEach() and iterate over the folders object directly.
When using a Web browser with JavaScript either disabled or unavailable,
show a warning to let the user know that the Web GUI requires JS in
order to operate.
To achieve this, add a <div> that wraps both the navbar and the main
content, and then move the CSS class ng-cloak from the <html> element to
that <div>. This way, only the JavaScript-dependent part is hidden when
JS is unavailable, and not the whole website, as it is the case right
now. Then, add a <noscript> element right at the start of the <body>
element, so that the warning is also shown right away in text-based Web
browsers. The <noscript> element includes a stripped down version of the
navbar showing only the Syncthing logo, and then a container with the
warning itself. Lastly, leave the footer untouched and always visible,
because it does not rely on JavaScript at all.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This removes the switch for using a Badger database, because it has bugs
that it seems there is no interest in fixing, and no actual bug tracker
to track them in.
It retains the actual implementation for the sole purpose of being able
to do the conversion back to LevelDB if anyone is actually running with
USE_BADGER. At some point in a couple of versions we can remove the
implementation as well.
Add specific errors for the failures, resulting in this rather than just
the generic "invalid filename":
[MRIW7] 08:50:50 INFO: Puller (folder default, item "NUL"): syncing: filename is invalid: name is reserved
[MRIW7] 08:50:50 INFO: Puller (folder default, item "fail."): syncing: filename is invalid: name ends with space or period
[MRIW7] 08:50:50 INFO: Puller (folder default, item "sup:yo"): syncing: filename is invalid: name contains reserved character
[MRIW7] 08:50:50 INFO: default: Failed to sync 3 items
Use the IoctlFileClone and IoctlFileCloneRange ioctl wrappers and the
FileCloneRange type provided by golang.org/x/sys/unix instead of
locally implementing them. This also allows to re-enable the code for
ppc/ppc64/ppc64le again (see commit 758a1a6a37 ("lib/fs: Disable ioctl
on ppc (fixes#6898) (#6901)")) since golang.org/x/sys/unix internally
uses the correct FICLONE and FICLONERANGE values depending on $GOARCH.
Most notably, it now detects all-lowercase files and returns these
as-is. The tests have been expanded with two cases and are now used
as a benchmark (admittedly a rather trivial one).
name old time/op new time/op delta
UnicodeLowercaseMaybeChange-8 4.59µs ± 2% 4.57µs ± 1% ~ (p=0.197 n=10+10)
UnicodeLowercaseNoChange-8 3.26µs ± 1% 3.09µs ± 1% -5.27% (p=0.000 n=9+10)
This changes the cache to cache less things, yet retain the required
efficiency for our walk usecase. This uses less memory.
Specifically, instead of keeping result and child caches for each path
level, only keep a single cached child. In practice our operations are
depth-first, or almost depth-first, and then we retain the same hit
ratio for a smaller cache size.
I improved the benchmark so that it counts the Lstat and DirNames
operations performed, and they do not change significantly. The amount
of allocated memory is reduced by 20% and the walk itself is actually
slightly faster.
This also removes the clear based on number of cached names (as that is
not a thing any more) and the timer based clear (which was unused). This
means we'll retain the last cache state forever until it's cleared by a
write operation, but we did that before too and that state is now a lot
smaller...
The overhead compared to not using a casefs, for our typical "double
walk" (walk the tree then stat everything again) is 2x the dirnames we
would otherwise call, and no overhead on the stats (unchanged from old
implementation)
```
name old time/op new time/op delta
WalkCaseFakeFS100k/rawfs-8 306ms ± 1% 305ms ± 2% ~ (p=0.182 n=9+10)
WalkCaseFakeFS100k/casefs-8 579ms ± 5% 557ms ± 1% -3.77% (p=0.000 n=10+10)
name old B/entry new B/entry delta
WalkCaseFakeFS100k/rawfs-8 590 ± 0% 590 ± 0% ~ (all equal)
WalkCaseFakeFS100k/casefs-8 1.09k ± 0% 0.87k ± 0% -19.98% (p=0.000 n=10+10)
name old DirNames/entry new DirNames/entry delta
WalkCaseFakeFS100k/rawfs-8 0.51 ± 0% 0.51 ± 0% ~ (all equal)
WalkCaseFakeFS100k/casefs-8 1.02 ± 0% 1.02 ± 0% ~ (all equal)
name old DirNames/op new DirNames/op delta
WalkCaseFakeFS100k/rawfs-8 51.2k ± 0% 51.2k ± 0% ~ (all equal)
WalkCaseFakeFS100k/casefs-8 102k ± 0% 102k ± 0% ~ (all equal)
name old Lstat/entry new Lstat/entry delta
WalkCaseFakeFS100k/rawfs-8 3.02 ± 0% 3.02 ± 0% ~ (all equal)
WalkCaseFakeFS100k/casefs-8 3.02 ± 0% 3.02 ± 0% ~ (all equal)
name old Lstat/op new Lstat/op delta
WalkCaseFakeFS100k/rawfs-8 302k ± 0% 302k ± 0% ~ (all equal)
WalkCaseFakeFS100k/casefs-8 302k ± 0% 302k ± 0% ~ (all equal)
name old allocs/entry new allocs/entry delta
WalkCaseFakeFS100k/rawfs-8 15.7 ± 0% 15.7 ± 0% ~ (all equal)
WalkCaseFakeFS100k/casefs-8 27.5 ± 0% 26.1 ± 0% -5.09% (p=0.000 n=10+10)
name old ns/entry new ns/entry delta
WalkCaseFakeFS100k/rawfs-8 2.02k ± 1% 2.02k ± 2% ~ (p=0.163 n=9+10)
WalkCaseFakeFS100k/casefs-8 3.83k ± 5% 3.68k ± 1% -3.77% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
WalkCaseFakeFS100k/rawfs-8 89.2MB ± 0% 89.2MB ± 0% ~ (p=0.364 n=9+10)
WalkCaseFakeFS100k/casefs-8 164MB ± 0% 131MB ± 0% -19.97% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
WalkCaseFakeFS100k/rawfs-8 2.38M ± 0% 2.38M ± 0% ~ (all equal)
WalkCaseFakeFS100k/casefs-8 4.16M ± 0% 3.95M ± 0% -5.05% (p=0.000 n=10+10)
```
Since iterators must be released before committing or discarding a
transaction we have the pattern of both deferring a release plus doing
it manually. But we can't release twice because we track this with a
WaitGroup that will panic when there are more Done()s than Add()s. This
just adds a boolean to let an iterator keep track.
Use indexOf() == 0 instead of startsWith() to maintain compatibility
and prevent JavaScript error console spam in the Web GUI when used in
Internet Explorer 11 under Windows 7.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
We created a new fileset before stopping the folder during restart. When
we create that fileset it loads the current metadata and sequence
numbers from the database. But the folder may have time to update those
before stopping, leaving the new fileset with bad data.
This would cause wrong accounting (forgotten files) and potentially
sequence reuse causing files not sent to other devices.
This change reuses the fileset on restart, skipping the issue entirely.
It also moves the creation of the fileset back under the lock so there
should be no chance of concurrency issues here.
We created a new fileset before stopping the folder during restart. When
we create that fileset it loads the current metadata and sequence
numbers from the database. But the folder may have time to update those
before stopping, leaving the new fileset with bad data.
This would cause wrong accounting (forgotten files) and potentially
sequence reuse causing files not sent to other devices.
This change reuses the fileset on restart, skipping the issue entirely.
It also moves the creation of the fileset back under the lock so there
should be no chance of concurrency issues here.
The FileSet.Drop operation in there needs to potentially update a whole lot of global lists, which can take a while (longer than the deadlock interval apparently)
When cap(permanentRelays) >= len(permanentRelays) + len(knownRelays),
append(permanentRelays, knownRelays...)
returns a slice of the array underlying permanentRelays. The subsequent
rand.Shuffle then mixes the permanent and known relays. Sequential
requests may cause strelaypoolsrv to forget its permanent relays. Worse,
concurrent requests may cause shuffling of the same slice on multiple
processors concurrently.
Co-authored-by: greatroar <@>
* Add clean up for Simple File Versioning pt.1
created test
* Add clean up for Simple File Versioning pt.2
Passing the test
* stuck on how javascript communicates with backend
* Add trash clean up for Simple File Versioning
Add trash clean up functionality of to allow the user to delete backups
after specified amount of days.
* Fixed html and js style
* Refactored cleanup test cases
Refactored cleanup test cases to one file and deleted duplicated code.
* Added copyright to test file
* Refactor folder cleanout to utility function
* change utility function to package private
* refactored utility function; fixed build errors
* Updated copyright year.
* refactor test and logging
* refactor html and js
* revert style change in html
* reverted changes in html and some js
* checkout origin head version edit...html
* checkout upstream master and correct file
Quoting the manual:
-trimpath
remove all file system paths from the resulting executable.
Instead of absolute file system paths, the recorded file names
will begin with either "go" (for the standard library),
or a module path@version (when using modules),
or a plain import path (when using GOPATH).
That is, when we panic, instead of:
goroutine 1 [running]:
main.main()
/Users/jb/dev/syncthing/syncthing/cmd/syncthing/main.go:272 +0x116
we get:
goroutine 1 [running]:
main.main()
github.com/syncthing/syncthing@/cmd/syncthing/main.go:272 +0x116
(Module path and file path within module.)
Our authentication is based on device ID (certificate fingerprint) but
we also check the certificate name for ... historical extra security
reasons. (I don't think this adds anything but it is what it is.) Since
that check breaks in Go 1.15 this change does two things:
- Adds a manual check for the peer certificate CommonName, and if they
are equal we are happy and don't call the more advanced
VerifyHostname() function. This allows our old style certificates to
still pass the check.
- Adds the cert name "syncthing" as a DNS SAN when generating the
certificate. This is the correct way nowadays and makes VerifyHostname()
happy in Go 1.15 as well, even without the above patch.
Apparently our Tags field depended on having specific files react to
tags and add themselves there. This, instead, works for all tags.
Also, pass tags to the test command line.
The QUIC package is notorious for being incompatible with either too
old or too new Go releases. Currently it doesn't build with Go 1.15 RC
and I want to test the rest with Go 1.15. With this I can do `go run
build.go --tags noquic` to do that.
With this change we emulate a case sensitive filesystem on top of
insensitive filesystems. This means we correctly pick up case-only renames
and throw a case conflict error when there would be multiple files differing
only in case.
This safety check has a small performance hit (about 20% more filesystem
operations when scanning for changes). The new advanced folder option
`caseSensitiveFS` can be used to disable the safety checks, retaining the
previous behavior on systems known to be fully case sensitive.
Co-authored-by: Jakob Borg <jakob@kastelo.net>
Prompted by https://forum.syncthing.net/t/infinite-filesystem-recursion-detected/15285. In my opinion the filesystem shouldn't throw warnings but pass on errors for the caller to decide what's to be happening with it. Right now in this PR an infinite recursion is a normal scan error, i.e. folder is in failed state and displays failed items, but no warning. I think that's appropriate but if deemed appropriate an additional warning can be thrown in the scanner.
This fixes the change in #6674 where the weak hash became a deciding
factor. Now we again just use it to accept a block, but don't take a
negative as meaning the block is bad.
This fixes the change in #6674 where the weak hash became a deciding
factor. Now we again just use it to accept a block, but don't take a
negative as meaning the block is bad.
If the GC finds a key k that it wants to keep, it records that in a
Bloom filter. If a key k' can be removed but its hash collides with k,
it will be kept. Since the old Bloom filter code was completely
deterministic, the next run would encounter the same collision, assuming
k must still be kept.
A randomized hash function that uses all the SHA-256 bits solves this
problem: the second run has a non-zero probability of removing k', as
long as the Bloom filter is not completely full.
* Fix ui, hide report date
* Undo Goland madness
* UR now web scale
* Fix migration
* Fix marshaling, force tick on start
* Fix tests
* Darwin build
* Split "all" build target, add package name as a tag
* Remove pq and sql dep from syncthing, split build targets
* Empty line
* Revert "Empty line"
This reverts commit f74af2b067.
* Revert "Remove pq and sql dep from syncthing, split build targets"
This reverts commit 8fc295ad00.
* Revert "Split "all" build target, add package name as a tag"
This reverts commit f4dc889951.
* Normalise contract types
* Fix build add more logging
This is shorter, skips two allocations, makes the function inlineable
and is safer, since the compiler now check whether
DeviceIDLength == sha256.Size.
Currently a random dev version has a version string like this:
v1.7.0-rc.1+23-gef3441bd6
That is, the tag name followed by a plus sign and the git describe
metadata (number of commits and hash) plus -dirty or -branchname in some
cases.
We introduced the plus sign in #473, where a dev version would
previously be called v0.9.0-42-gwhatever which is considered older than
v0.9.0 and hence caused a downgrade. The problem with the plus is that
per semver everything after the plus is ignored as build metadata, which
means we won't upgrade from v1.7.0-rc.1 to v1.7.0-rc.1+22-g946170f3f.
With this change the we instead either just add a dev suffix (if we're
already on a prerelease version) or we wind the patch version and add a
dev suffix.
v1.7.0-rc.1+23-gef3441bd6 => v1.7.0-rc.1.dev.23.gef3441bd6
v1.6.1+80-gef3441bd6 => v1.6.2-dev.80.gef3441bd6
This should preserve the ordering and keep versions semver-ish.
This changes the error handling in loading ignores slightly:
- There is a new ParseError type that is returned as the error
(somewhere in the chain) when the problem was not an I/O error loading
the file, but some issue with the contents.
- If the file was read successfully but not parsed successfully we still
return the lines read (in addition to nil patterns and a ParseError).
- In the API, if the error IsParseError then we return a successful
HTTP response with the lines and the actual error included in the JSON
object.
- In the GUI, as long as the HTTP call to load the ignores was
successful we can edit the ignores. If there was an error we show this
as a validation error on the dialog.
Also some cleanup on the Javascript side as it for some reason used
jQuery instead of Angular for this editor...
crypto/rand output is cryptographically secure by the Go library
documentation's promise. That, rather than strength (= passes randomness
tests) is the property that Syncthing needs).
This makes Go 1.15 test/vet happy, avoiding "conversion from untyped int
to string yields a string of one rune" warning where we do
string(KeyTypeWhatever) in namespaced.go.
It also clarifies and enforces the currently allowed range of these
numbers so I think it's fine.
This matches the convention of the stdlib and avoids ambiguity: when
customErr{} and &customErr{} both implement error, client code needs to
check for both.
Memory use should remain the same, since storing a non-pointer type in
an interface value still copies the value to the heap.
This extracts the extra tags from any `[foo]` stuff at the end of the
version and sends them to Sentry for indexing.
If I need to modify that regexp again I'll probably write a from scratch
tokenizer and parser for our version string instead...
This adds some env vars to the long version string as if they were build
tags. The purpose is to better understand what code was running or not
in the version output, usage reporting and crash reports. In order to
prevent possible privacy issues the actual value of the variable is not
reported, just the fact that it was set to something non-empty.
Example:
% ./bin/syncthing --version
syncthing v1.6.1+47-g1eb104f3-buildtags "Fermium Flea" (go1.14.3 darwin-amd64) jb@kvin.kastelo.net 2020-06-03 07:25:46 UTC [stnoupgrade, use_badger]
Group the global list of files by version, instead of having one flat list for all devices. This removes lots of duplicate protocol.Vectors.
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This reduces the size of our write batches before we flush them. This
has two effects: reducing the amount of data lost if we crash when
updating the database, and reducing the amount of memory used when we do
large updates without checkpoint (e.g., deleting a folder).
I ran our SyncManyFiles benchmark as it is the one doing most
transactions, however there was no relevant change in any metric (it's
limited by our fsync I expect). This is good as any visible change would
just be a decrease in performance.
I don't have a benchmark on deleting a large folder, taking that part on
trust for now...
* cmd/stindex: Unify access to key from cached variable.
Avoid calling the Key() method from the iterator each time the value
is needed. Just reuse the cache variable already assigned before the
switch block.
* cmd/stindex: Display the prefix byte value for unknown key types.
Make it easier to diagnose corrupt / unknown key type entries by
showing their decimal value, correlating with the definitions in
keyer.go.
* cmd/stindex: Add missing KeyType values in stindex dump code.
Recently added DB key prefixes KeyTypeBlockListMap and KeyTypeVersion
were unknown to the stindex dumping tool. Add basic parsing to dump
their key structure.
If we fail to take the rename shortcut we may crash on a later loop,
because we do trickiness with the indexes but the original buckets[key]
in "range buckets[key]" isn't re-evaluated so i exceeds the max index.
This adds indirection of large version vectors in the same manner as we
already to block lists. The effect is the same: less duplicated data in
some situations.
To mitigate the impact for when this indirection
wouldn't be needed I've added an indirection cutoff for both blocks and
the new version vector stuff: we don't do the indirection at all for
small block lists or small version vectors, instead storing it directly
like we used to do. This is faster for small files and small setups.
Storing assets as []byte requires every compiled-in asset to be copied
into writable memory at program startup. That currently takes up 1.6MB
per syncthing process. Strings stay in the RODATA section and should be
shared between processes running the same binary.
This makes version vector values clock based instead of just incremented
from zero. The effect is that a vector that is created from scratch
(after database reset) will have a higher value for the local device
than what it could have been previously, causing a conflict. That is, if
we are A and we had
{A: 42, B: 12}
in the old scheme, a reset and rescan would give us
{A: 1}
which is a strict ancestor of the older file (this might be wrong). With
the new scheme we would instead have
{A: someClockTime, b: otherClockTime}
and the new version after reset would become
{A: someClockTime+delta}
which is in conflict with the previous entry (better).
In case the clocks are wrong (current time is less than the value in the
vector) we fall back to just simple increment like today.
This scheme is ineffective if we suffer a database reset while at the
same time setting the clock back far into the past. It's however no
worse than what we already do.
This loses the ability to emit the "added" event, as we can't look for
the magic 1 entry any more. That event was however already broken
(#5541).
Another place where we infer meaning from the vector itself is in
receive only folders, but there the only criteria is that the vector is
one item long and includes just ourselves, which remains the case with
this change.
* wip
This changes the build script to build all the things in one go
invocation, instead of one invocation per cmd. This is a lot faster
because it means more things get compiled concurrently. It's especially
a lot faster when things *don't* need to be rebuilt, possibly because it
only needs to build the dependency map and such once instead of once per
binary.
In order for this to work we need to be able to pass the same ldflags to
all the binaries. This means we can't set the program name with an
ldflag.
When it needs to rebuild everything (go clean -cache):
( ./old-build -gocmd go1.14.2 build all 2> /dev/null; ) 65.82s user 11.28s system 574% cpu 13.409 total
( ./new-build -gocmd go1.14.2 build all 2> /dev/null; ) 63.26s user 7.12s system 1220% cpu 5.766 total
On a subsequent run (nothing to build, just link the binaries):
( ./old-build -gocmd go1.14.2 build all 2> /dev/null; ) 26.58s user 7.53s system 582% cpu 5.853 total
( ./new-build -gocmd go1.14.2 build all 2> /dev/null; ) 18.66s user 2.45s system 1090% cpu 1.935 total
This makes the GC runner a service that will stop fairly quickly when
told to.
As a bonus, STTRACE=app will print the service tree on the way out,
including any errors they've flagged.
* release:
lib/db: Don't get blocklists on drop and missing continue (ref #6457) (#6502)
Revert "cmd/syncthing: Do auto-upgrade before startup (fixes#6384) (#6385)"
lib/ur: Correct freaky error handling (fixes#6499) (#6500)
So, in a funny plot twist, it turns out that WriteFile in Go 1.13
doesn't actually set the read only bit on Windows when called with
permissions 0444 so my test was broken. With an improved test it turns
out that Rename does not, in fact, overwrite a read-only file on
Windows. This adds a fix for that.
(Rename might get improved in Go 1.15...)
Successful LRU cache lookups modify the cache's recency list, so
RWMutex.RLock isn't enough protection.
Secondarily, multiple concurrent lookups with the same key should not
create separate rate limiters, so release the lock only when presence
of the key in the cache has been ascertained.
Co-authored-by: greatroar <@>
This adds the functionality to run a user search with a filter for LDAP
authentication. The search is done after successful bind, as the binding
user. The typical use case is to limit authentication to users who are
member of a group or under a certain OU. For example, to only match
users in the "Syncthing" group in otherwise default Active Directory
set up for example.com:
<searchBaseDN>CN=Users,DC=example,DC=com</searchBaseDN>
<searchFilter>(&(sAMAccountName=%s)(memberOf=CN=Syncthing,CN=Users,DC=example,DC=com))</searchFilter>
The search filter is an "and" of two criteria (with the ampersand being
XML quoted),
- "(sAMAccountName=%s)" matches the user logging in
- "(memberOf=CN=Syncthing,CN=Users,DC=example,DC=com)" matches members
of the group in question.
Authentication will only proceed if the search filter matches precisely
one user.
The previous implementation was very generic; its tests didn't cover the
actual alphabet for device IDs.
Benchmark results on amd64:
name old time/op new time/op delta
Luhnify-8 1.00µs ± 1% 0.28µs ± 4% -72.38% (p=0.000 n=9+10)
Unluhnify-8 992ns ± 2% 274ns ± 1% -72.39% (p=0.000 n=10+9)
We set the STRESTART environment when starting the inner process after
the first time, but this didn't persist when restarting the monitor
process. Now it does.
As of the latest database checker we are again putting files without
blocks. I'm not 100% convinced that's a great idea, but we also do it
for ignored files apparently so it looks like we probably should support
it. This adds an escape hatch that must be manually enabled...
* release:
lib/db, lib/syncthing: Repair db once on upgrade (ref #6425, #6427) (#6429)
lib/db: Fix removeFromGlobal and no filenames in error (fixes#6427) (#6428)
lib/db: Remove emptied global list in checkGlobals (fixes#6425) (#6426)
### DO NOT REPORT SECURITY ISSUES IN THIS ISSUE TRACKER
Instead, contact security@syncthing.net directly - see
https://syncthing.net/security.html for more information.
### DO NOT POST SUPPORT REQUESTS OR GENERAL QUESTIONS IN THIS ISSUE TRACKER
Please use the forum at https://forum.syncthing.net/ where a large number of
helpful people hang out. This issue tracker is for reporting bugs or feature
requests directly to the developers. Worst case you might get a short
"that's a bug, please report it on GitHub" response on the forum, in which
case we thank you for your patience and following our advice. :)
### Please use the correct issue tracker
If your problem relates to a Syncthing wrapper or [sub-project](https://github.com/syncthing) such as [Syncthing for Android](https://github.com/syncthing/syncthing-android/issues), [SyncTrayzor](https://github.com/canton7/synctrayzor) or the [documentation](https://github.com/syncthing/docs/issues), please use their respective issue trackers.
### Does your log mention database corruption?
If your Syncthing log reports panics because of database corruption it is most likely a fault with your system's storage or memory. Affected log entries will contain lines starting with `panic: leveldb`. You will need to delete the index database to clear this, by running `syncthing -reset-database`.
### Please do post actual bug reports and feature requests.
If your issue is a bug report, replace this boilerplate with a description
of the problem, being sure to include at least:
- what happened,
- what you expected to happen instead, and
- any steps to reproduce the problem.
Also fill out the version information below and add log output or
screenshots as appropriate.
If your issue is a feature request, simply replace this template text in
description:Please describe the behavior you'd like to see.
validations:
required:true
- type:textarea
id:problem-usecase
attributes:
label:Problem or use case
description:Please explain which problem this would solve, or what the use case is for the feature. Keep in mind that it's more likely to be implemented if it's generally useful for a larger number of users.
validations:
required:true
- type:textarea
id:alternatives
attributes:
label:Alternatives or workarounds
description:Please describe any alternatives or workarounds you have considered and, possibly, rejected.
description:If you're actually looking for support instead, see "I need help / I have a question".
labels:["bug","needs-triage"]
type:Bug
body:
- type:markdown
attributes:
value:|
:no_entry_sign: If you want to report a security issue, please see [our Security Policy](https://syncthing.net/security/) and do not report the issue here.
:interrobang: If you are not sure if there is a bug, but something isn't working right and you need help, please [use the forum](https://forum.syncthing.net/).
- type:textarea
id:what-happened
attributes:
label:What happened?
description:Also tell us, what did you expect to happen, and any steps we might use to reproduce the problem.
placeholder:Tell us what you see!
validations:
required:true
- type:input
id:version
attributes:
label:Syncthing version
description:What version of Syncthing are you running?
placeholder:v1.27.4
validations:
required:true
- type:input
id:platform
attributes:
label:Platform & operating system
description:Onwhat platform(s) are you seeing the problem?
placeholder:Linux arm64
validations:
required:true
- type:input
id:browser
attributes:
label:Browser version
description:If the problem is related to the GUI, describe your browser and version.
placeholder:Safari 17.3.1
- type:textarea
id:logs
attributes:
label:Relevant log output
description:Please copy and paste any relevant log output or crash backtrace. This will be automatically formatted into code, so no need for backticks.
If you believe that you've found a Syncthing-related security vulnerability, please report it by sending email to the address security@syncthing.net. The PGP key for security@syncthing.net (B683AD7B76CAB013) below can be used to send encrypted mail or to verify responses received from that address.
If you believe that you've found a Syncthing-related security vulnerability,
please report it by sending email to the address security@syncthing.net. The
[PGP key for security@syncthing.net
(B683AD7B76CAB013)](https://syncthing.net/security-key.txt) can be used to
send encrypted mail or to verify responses received from that address.
[](https://build.syncthing.net/viewType.html?buildTypeId=Syncthing_BuildLinuxCross&guest=1)
[](https://build.syncthing.net/viewType.html?buildTypeId=Syncthing_BuildWindows&guest=1)
[](https://build.syncthing.net/viewType.html?buildTypeId=Syncthing_BuildMac&guest=1)
message:`panic: filling Blocks: read C:\Users\Serv-Resp-Tizayuca\AppData\Local\Syncthing\index-v0.14.0.db\006561.ldb: Error de datos (comprobación de redundancia cíclica).`,
exp:`panic: filling Blocks: read x: Error de datos (comprobación de redundancia cíclica).`,
fmt.Fprintf(tw,"0x%02x:\t%d items,\t%d KB keys +\t%d KB data,\t%d B +\t%d B avg,\t%d B max\t\n",t,counts[t],ksizes[t]/1000,dsizes[t]/1000,ksizes[t]/counts[t],dsizes[t]/counts[t],max[t])
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
