This smells, something is not right here. I suspect we end up with gaps in the index seq numbers
That doesn’t really matter though. Someone will send entries 1, 2 and 3. Then “2” gets dropped because it’s invalid. We’ll continue from 4 on next exchange.
I admit not following the full issue. But
fooC:\bar is an acceptable unixish file name that will cause issues Windows-side… So once we let that crap escape, it could fester on some other device.
I don’t think we should ever drop anything, but just mark it as invalid or something and never pull it.
The thing here is that internally we use native path separators, while the wire protocol and database are unix normalized. If we accept a name like
foo/bar\baz from the outside it’ll get normalized to
foo\bar\bar and then
foo/bar/baz in various places. We can’t really tag it with a flag because there might be another file at
foo/bar/baz that would be affected.
Unless you can somehow make the data format to differentiate clearly between folders and file-names … maybe. eg. by escaping the special characters used inside a single file or folder name.
We could use something like the null character as path separator, or use an array of strings to represent a path. But I doubt it’s worth the effort for this specific case.
Shouldn’t there first be an indication that dropping invalid indexes is a problem before thinking about any complicated measures? I am still of the opinion that we should just “downgrade” this log line from warning to debug and be done with it.
It should be treated with the same severity as other invalid characters in Windows, whatever that is. That we need to specially handle it earlier and throw away the index entry is an (imho harmless) implentation detail.
How do you handle other invalid characters on windows?
BTW the “:” is problably as much trouble as the “\” in these paths.
It just becomes a sync error for that file, which makes the folder out of sync and shows up in the GUI. Which is nicer, but tricky to do for the backslashes for the technical reasons mentioned above.
There’s myriad of forbidden characters and names on Windows…
@imsodin I think debug (aka “invisible”) is too low a severity, this should happen seldom enough that a warning once should be OK… I think…
I agree, once is ok. But currently it’s not once.
Can you check two things:
- In current state, the warning only comes up after a new connection.
- If you do an unrelated change in the same syncthing folder on the sending side (i.e. where the file with backslashes was created), the warning doesn’t come up anymore at all.
It did not happen for a while, even if i changed files.
But now it came back after I updated / restarted all devices.
Specifically: It showed up on my windows device after I updated my linux device.
2018-06-09 10:11:13: Dropping index entry for Skype/C:\Users\Jan\AppData\Local\Packages\Microsoft.SkypeApp_kzf8qxf38zg5c\LocalState\1fda1808-31e5-4b2a-95ce-9ee2a63c2c34_20180411_170042865.jpg, contains invalid path separator 2018-06-09 10:11:13: Dropping index entry for Skype/C:\Users\User\AppData\Local\Packages\Microsoft.SkypeApp_kzf8qxf38zg5c\LocalState\94db313a-c5fd-4aac-8ddf-5b02cfb3c76f_20180406_113030260.jpg, contains invalid path separator 2018-06-09 10:11:13: Dropping index entry for Skype/C:\Users\amuel\AppData\Local\Packages\Microsoft.SkypeApp_kzf8qxf38zg5c\LocalState\3695e692-3464-4482-92be-1a74cdbe9440_20180409_161725895.jpg, contains invalid path separator
I assume (from what you said) it will only show up once (per restart).
I guess restarting the linux device will trigger the warning again. A while ago my linux device was constantly crashing and restarting due to OOM errors and (i guss) thus created this warnings again and again.
I’ll keep you updated if i notice anything else.