From 5db9ed3d8d35f251b3638765fbac1cfb9f7a11ea Mon Sep 17 00:00:00 2001 From: Andrew Gunnerson Date: Tue, 28 Jul 2026 21:08:27 -0400 Subject: [PATCH] stbridge/saf: Fix missing O_RDWR in Create() and enforce PFD modes safFilesystem.Create() was not passing in O_RDWR, which caused it to either fail (Android 17+ with FileSystemProvider) or return a read-only safFile instance (all other scenarios). This didn't actually break anything since Create() is only used in the happy optimization path when renaming files with file versioning enabled. If it fails, Syncthing falls back to copy-and-delete. This commit also updates safNode.OpenFile() to enforce Android 17's ParcelFileDescriptor mode string restrictions so that the behavior is consistent across all Android versions. Fixes: #214 Signed-off-by: Andrew Gunnerson --- stbridge/saf.go | 65 ++++++++++++---- stbridge/saf_test.go | 178 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 221 insertions(+), 22 deletions(-) diff --git a/stbridge/saf.go b/stbridge/saf.go index e694071..b7bbc76 100644 --- a/stbridge/saf.go +++ b/stbridge/saf.go @@ -810,6 +810,26 @@ func (sn *safNode) MkdirAll(opts *safOpts, name string) (*safNode, bool, error) return current, currentIsNew, nil } +// android-10.0.0_r1 is the first Android version that guaranteed that O_TRUNC +// is only passed when 't' is present for "w" vs "wt" [1]. However, "rw" has +// never truncated since the very beginning [2]. +// +// android-17.0.0_r1 is the first Android version that started enforcing that +// the ParcelFileDescriptor modes cannot be anything besides the six below [3]. +// +// [1] https://android.googlesource.com/platform/frameworks/base/+/63280e06fc64672ab36d14f852b13df2274cc328%5E!/ +// [2] https://android.googlesource.com/platform/frameworks/base/+/9066cfe9886ac131c34d59ed0e2d287b0e3c0087%5E!/ +// [3] https://android.googlesource.com/platform/frameworks/base/+/5b5468ef7ae91858b1fe485ade787e4c16058264%5E!/ +const ( + pfdMask = os.O_RDONLY | os.O_WRONLY | os.O_RDWR | os.O_TRUNC | os.O_APPEND + pfdR = os.O_RDONLY + pfdW = os.O_WRONLY + pfdWt = os.O_WRONLY | os.O_TRUNC + pfdWa = os.O_WRONLY | os.O_APPEND + pfdRw = os.O_RDWR + pfdRwt = os.O_RDWR | os.O_TRUNC +) + // Open or create the specified file. Only the [os.O_RDONLY], [os.O_WRONLY], // [os.O_RDWR], [os.O_CREATE], [os.O_TRUNC], and [os.O_EXCL] flags are // supported. @@ -841,23 +861,36 @@ func (sn *safNode) OpenFile(opts *safOpts, name string, flags int) (*safFile, er return nil, err } - safMode := "rw" - if flags&os.O_RDWR == 0 && isAlwaysSeekable(child.uri) { - if flags&os.O_WRONLY != 0 { - safMode = "w" - } else { + pfdFlags := flags & pfdMask + var safMode string + + if isAlwaysSeekable(child.uri) { + switch pfdFlags { + case pfdR: safMode = "r" + case pfdW: + safMode = "w" + case pfdWt: + safMode = "wt" + case pfdWa: + safMode = "wa" + case pfdRw: + safMode = "rw" + case pfdRwt: + safMode = "rwt" + default: + return nil, fmt.Errorf("invalid open flags for guaranteed seekable file: %q: %#x", child.uri, flags) + } + } else { + switch pfdFlags { + case pfdR, pfdW, pfdRw: + safMode = "rw" + case pfdWt, pfdRwt: + safMode = "rwt" + // "wa" is not representable. + default: + return nil, fmt.Errorf("invalid open flags for not guaranteed seekable file: %q: %#x", child.uri, flags) } - } - - // android-10.0.0_r1 is the first Android version that guaranteed that - // O_TRUNC is only passed when 't' is present for "w" vs "wt" [1]. However, - // "rw" has never truncated since the very beginning [2]. - // - // [1] https://android.googlesource.com/platform/frameworks/base/+/63280e06fc64672ab36d14f852b13df2274cc328%5E!/ - // [2] https://android.googlesource.com/platform/frameworks/base/+/9066cfe9886ac131c34d59ed0e2d287b0e3c0087%5E!/ - if flags&os.O_TRUNC != 0 { - safMode += "t" } fd, err := opts.client.OpenDocument(child.uri, safMode) @@ -1576,7 +1609,7 @@ func (sfs *safFilesystem) Chtimes(name string, atime time.Time, mtime time.Time) } func (sfs *safFilesystem) Create(name string) (fs.File, error) { - return sfs.OpenFile(name, os.O_CREATE|os.O_TRUNC, 0) + return sfs.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0) } func (sfs *safFilesystem) CreateSymlink(target, name string) error { diff --git a/stbridge/saf_test.go b/stbridge/saf_test.go index 5518323..2572aca 100644 --- a/stbridge/saf_test.go +++ b/stbridge/saf_test.go @@ -694,12 +694,6 @@ func TestOpenFileExists(t *testing.T) { } client, opts := newTestClient() - client.createDocument = func(parentDocumentUri, mimeType, name string) (string, error) { - if mimeType != safMimeTypeDir { - t.Errorf("invalid MIME type: %q", mimeType) - } - return name, nil - } client.openDocument = func(documentUri, mode string) (int, error) { if mode != "rw" { t.Errorf("invalid mode: %q", mode) @@ -722,6 +716,178 @@ func TestOpenFileExists(t *testing.T) { } } +func TestOpenFileModeString(t *testing.T) { + nodeInternal := &safNode{ + uri: "content://com.android.externalstorage.documents/tree/primary%3apath", + infoExpiry: safExpired, + childrenExpiry: safExpired, + watchManager: &safWatchManager{}, + } + nodeExternal := &safNode{ + uri: "content://com.chiller3.rsaf/tree/remote%3apath", + infoExpiry: safExpired, + childrenExpiry: safExpired, + watchManager: &safWatchManager{}, + } + + actualMode := "" + + client, opts := newTestClient() + client.openDocument = func(documentUri, mode string) (int, error) { + if !slices.Contains([]string{"r", "w", "wt", "wa", "rw", "rwt"}, mode) { + t.Errorf("invalid pfd mode: %q", mode) + } + + actualMode = mode + return syscall.Dup(syscall.Stdin) + } + + for _, i := range []struct { + internal bool + flags int + mode string + readable bool + writable bool + invalid bool + }{ + { + internal: true, + flags: os.O_RDONLY, + mode: "r", + readable: true, + writable: false, + }, + { + internal: false, + flags: os.O_RDONLY, + mode: "rw", + readable: true, + writable: false, + }, + { + internal: true, + flags: os.O_WRONLY, + mode: "w", + readable: false, + writable: true, + }, + { + internal: false, + flags: os.O_WRONLY, + mode: "rw", + readable: false, + writable: true, + }, + { + internal: true, + flags: os.O_WRONLY | os.O_TRUNC, + mode: "wt", + readable: false, + writable: true, + }, + { + internal: false, + flags: os.O_WRONLY | os.O_TRUNC, + mode: "rwt", + readable: false, + writable: true, + }, + { + internal: true, + flags: os.O_WRONLY | os.O_APPEND, + mode: "wa", + readable: false, + writable: true, + }, + { + internal: false, + flags: os.O_WRONLY | os.O_APPEND, + invalid: true, + }, + { + internal: true, + flags: os.O_RDWR, + mode: "rw", + readable: true, + writable: true, + }, + { + internal: false, + flags: os.O_RDWR, + mode: "rw", + readable: true, + writable: true, + }, + { + internal: true, + flags: os.O_RDWR | os.O_TRUNC, + mode: "rwt", + readable: true, + writable: true, + }, + { + internal: false, + flags: os.O_RDWR | os.O_TRUNC, + mode: "rwt", + readable: true, + writable: true, + }, + { + internal: true, + flags: os.O_TRUNC, + invalid: true, + }, + { + internal: false, + flags: os.O_TRUNC, + invalid: true, + }, + { + internal: true, + flags: os.O_APPEND, + invalid: true, + }, + { + internal: false, + flags: os.O_APPEND, + invalid: true, + }, + } { + actualMode = "" + var node *safNode + + if i.internal { + node = nodeInternal + } else { + node = nodeExternal + } + + file, err := node.OpenFile(opts, ".", i.flags) + if err != nil { + if i.invalid { + continue + } else { + t.Fatalf("should have succeeded: %+v: %v", i, err) + } + } + file.Close() + + if i.invalid { + t.Fatalf("should have failed: %+v", i) + } + + if actualMode != i.mode { + t.Errorf("invalid pfd mode: %q != %q", actualMode, i.mode) + } + if file.canRead != i.readable { + t.Errorf("invalid readable state: %v != %v", file.canRead, i.readable) + } + if file.canWrite != i.writable { + t.Errorf("invalid writable state: %v != %v", file.canWrite, i.writable) + } + } +} + func TestRemove(t *testing.T) { watchManager := &safWatchManager{} node := &safNode{