RESOLVED: Dropping index entry for … contains invalid path separator


(Audrius Butkevicius) #21

This smells, something is not right here. I suspect we end up with gaps in the index seq numbers


(Jakob Borg) #22

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.


(Audrius Butkevicius) #23

I don’t think we should ever drop anything, but just mark it as invalid or something and never pull it.


(Jakob Borg) #24

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.


#25

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.


(Jakob Borg) #26

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.


(Simon) #27

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.


(Jakob Borg) #28

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.


#29

How do you handle other invalid characters on windows?

BTW the “:” is problably as much trouble as the “\” in these paths.


(Jakob Borg) #30

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…


#31

I agree, once is ok. But currently it’s not once.


(Simon) #32

Can you check two things:

  1. In current state, the warning only comes up after a new connection.
  2. 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.

#33

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.