Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/protocol: handle empty names in unixOwnershipEqual (fixes #9039) #9306

Merged
merged 1 commit into from Dec 29, 2023

Conversation

sven
Copy link
Contributor

@sven sven commented Dec 28, 2023

Purpose

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 are empty on one side.

See #9039 for details.

Testing

Proposed by @calmh in #9039 (comment) 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

Copy link
Member

@calmh calmh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable, except Go does not use explicit semicolons.

…ng#9039)

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 are empty
on one side.

See syncthing#9039 for details.
@sven
Copy link
Contributor Author

sven commented Dec 28, 2023

Reasonable, except Go does not use explicit semicolons.

Sorry, should be fixed now.

@calmh calmh merged commit 1ce2af1 into syncthing:main Dec 29, 2023
21 checks passed
calmh added a commit to calmh/syncthing that referenced this pull request Jan 4, 2024
* main:
  Update dependencies (syncthing#9321)
  gui: Always inform about loading data in Restore Versions modal (syncthing#9317)
  lib/build: Allow semver build in version regex (fixes syncthing#9267) (syncthing#9316)
  gui: Keep short deviceID length consistent + xrefs (fixes syncthing#9313) (syncthing#9314)
  build(deps): bump actions/download-artifact from 3 to 4 (syncthing#9294)
  build(deps): bump actions/upload-artifact from 3 to 4 (syncthing#9293)
  gui, man, authors: Update docs, translations, and contributors
  gui, lib/scanner: Improve scan progress indication (ref syncthing#8331) (syncthing#9308)
  lib/protocol: handle empty names in unixOwnershipEqual (fixes syncthing#9039) (syncthing#9306)
  gui, man, authors: Update docs, translations, and contributors
  etc/linux-desktop: use double dash for long options (syncthing#9301)
  lib/connections: Skip allocation in check for missing port (syncthing#9297)
  lib/upgrade: Extract signing key to embedded file (fixes syncthing#9247) (syncthing#9296)
  gui, man, authors: Update docs, translations, and contributors
  build: Update quic-go (fixes syncthing#9287)
  lib/model: Only handle relevant folder summaries (kqueue) (fixes syncthing#9183) (syncthing#9288)
calmh added a commit to danpadcz/syncthing that referenced this pull request Jan 4, 2024
* main:
  Update dependencies (syncthing#9321)
  gui: Always inform about loading data in Restore Versions modal (syncthing#9317)
  lib/build: Allow semver build in version regex (fixes syncthing#9267) (syncthing#9316)
  gui: Keep short deviceID length consistent + xrefs (fixes syncthing#9313) (syncthing#9314)
  build(deps): bump actions/download-artifact from 3 to 4 (syncthing#9294)
  build(deps): bump actions/upload-artifact from 3 to 4 (syncthing#9293)
  gui, man, authors: Update docs, translations, and contributors
  gui, lib/scanner: Improve scan progress indication (ref syncthing#8331) (syncthing#9308)
  lib/protocol: handle empty names in unixOwnershipEqual (fixes syncthing#9039) (syncthing#9306)
  gui, man, authors: Update docs, translations, and contributors
  etc/linux-desktop: use double dash for long options (syncthing#9301)
  lib/connections: Skip allocation in check for missing port (syncthing#9297)
  lib/upgrade: Extract signing key to embedded file (fixes syncthing#9247) (syncthing#9296)
  gui, man, authors: Update docs, translations, and contributors
  build: Update quic-go (fixes syncthing#9287)
  lib/model: Only handle relevant folder summaries (kqueue) (fixes syncthing#9183) (syncthing#9288)
@calmh calmh added this to the v1.27.3 milestone Jan 4, 2024
calmh added a commit to calmh/syncthing that referenced this pull request Jan 14, 2024
* main: (24 commits)
  lib/ignore: Refactor out result type (syncthing#9343)
  build: Testing infra images for infra-* branches
  lib/versioner: Expand tildes in version directory (fixes syncthing#9241) (syncthing#9327)
  lib/scanner: Prevent sync-conflict for receive-only local modifications  (syncthing#9323)
  gui, man, authors: Update docs, translations, and contributors
  Fix website security link in README.md (syncthing#9325)
  cmd/syncthing: Add CLI completion functionality (fixes syncthing#8616) (syncthing#9226)
  lib/api: Save session & CSRF tokens to database, add option to stay logged in (fixes syncthing#9151) (syncthing#9284)
  Update dependencies (syncthing#9321)
  gui: Always inform about loading data in Restore Versions modal (syncthing#9317)
  lib/build: Allow semver build in version regex (fixes syncthing#9267) (syncthing#9316)
  gui: Keep short deviceID length consistent + xrefs (fixes syncthing#9313) (syncthing#9314)
  build(deps): bump actions/download-artifact from 3 to 4 (syncthing#9294)
  build(deps): bump actions/upload-artifact from 3 to 4 (syncthing#9293)
  gui, man, authors: Update docs, translations, and contributors
  gui, lib/scanner: Improve scan progress indication (ref syncthing#8331) (syncthing#9308)
  lib/protocol: handle empty names in unixOwnershipEqual (fixes syncthing#9039) (syncthing#9306)
  gui, man, authors: Update docs, translations, and contributors
  etc/linux-desktop: use double dash for long options (syncthing#9301)
  lib/connections: Skip allocation in check for missing port (syncthing#9297)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants