stbridge/saf: Fix missing O_RDWR in Create() and enforce PFD modes

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 {

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{