From 946afb6c3350ec0c9cd4b73464692bb308d5d270 Mon Sep 17 00:00:00 2001
From: Nick Khyl <nickk@tailscale.com>
Date: Tue, 6 Aug 2024 12:31:21 -0500
Subject: [PATCH] libtailscale, android: translate NoSuchKeyException as
 syspolicy.ErrNoSuchKey

Currently, NoSuchKeyException gets translated by gomobile to a Go error with "no such key" as the text.
It is imperative for syspolicy.Handler implementations to return syspolicy.ErrNoSuchKey if a policy setting
is not configured, so this PR adds translation for errors that do not already wrap syspolicy.ErrNoSuchKey,
but have "no such key" as the text.

Updates tailscale/tailscale#12687

Signed-off-by: Nick Khyl <nickk@tailscale.com>
---
 .../java/com/tailscale/ipn/mdm/MDMSettings.kt |  6 +++--
 libtailscale/syspolicy_handler.go             | 26 +++++++++----------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/android/src/main/java/com/tailscale/ipn/mdm/MDMSettings.kt b/android/src/main/java/com/tailscale/ipn/mdm/MDMSettings.kt
index 6e6da12..546a0c2 100644
--- a/android/src/main/java/com/tailscale/ipn/mdm/MDMSettings.kt
+++ b/android/src/main/java/com/tailscale/ipn/mdm/MDMSettings.kt
@@ -12,8 +12,10 @@ import kotlin.reflect.jvm.jvmErasure
 
 object MDMSettings {
   // The String message used in this NoSuchKeyException must match the value of
-  // syspolicy.ErrNoSuchKey defined in Go, since the backend checks the value
-  // returned by the handler for equality using errors.Is().
+  // syspolicy.ErrNoSuchKey defined in Go. We compare against its exact text
+  // to determine whether the requested policy setting is not configured and
+  // an actual syspolicy.ErrNoSuchKey should be returned from syspolicyHandler
+  // to the backend.
   class NoSuchKeyException : Exception("no such key")
 
   val forceEnabled = BooleanMDMSetting("ForceEnabled", "Force Enabled Connection Toggle")
diff --git a/libtailscale/syspolicy_handler.go b/libtailscale/syspolicy_handler.go
index a311e1d..767f3b9 100644
--- a/libtailscale/syspolicy_handler.go
+++ b/libtailscale/syspolicy_handler.go
@@ -6,7 +6,6 @@ package libtailscale
 import (
 	"encoding/json"
 	"errors"
-	"log"
 
 	"tailscale.com/util/syspolicy"
 )
@@ -22,10 +21,7 @@ func (h syspolicyHandler) ReadString(key string) (string, error) {
 		return "", syspolicy.ErrNoSuchKey
 	}
 	retVal, err := h.a.appCtx.GetSyspolicyStringValue(key)
-	if err != nil && !errors.Is(err, syspolicy.ErrNoSuchKey) {
-		log.Printf("syspolicy: %s (str) will not be set: %v", key, err)
-	}
-	return retVal, err
+	return retVal, translateHandlerError(err)
 }
 
 func (h syspolicyHandler) ReadBoolean(key string) (bool, error) {
@@ -33,19 +29,15 @@ func (h syspolicyHandler) ReadBoolean(key string) (bool, error) {
 		return false, syspolicy.ErrNoSuchKey
 	}
 	retVal, err := h.a.appCtx.GetSyspolicyBooleanValue(key)
-	if err != nil && !errors.Is(err, syspolicy.ErrNoSuchKey) {
-		log.Printf("syspolicy: %s (bool) will not be set: %v", key, err)
-	}
-	return retVal, err
+	return retVal, translateHandlerError(err)
 }
 
 func (h syspolicyHandler) ReadUInt64(key string) (uint64, error) {
 	if key == "" {
 		return 0, syspolicy.ErrNoSuchKey
 	}
-	// TODO(angott): drop ReadUInt64 everywhere. We are not using it.
-	log.Fatalf("ReadUInt64 is not implemented on Android")
-	return 0, nil
+	// We don't have any UInt64 policy settings as of 2024-08-06.
+	return 0, errors.New("ReadUInt64 is not implemented on Android")
 }
 
 func (h syspolicyHandler) ReadStringArray(key string) ([]string, error) {
@@ -53,8 +45,7 @@ func (h syspolicyHandler) ReadStringArray(key string) ([]string, error) {
 		return nil, syspolicy.ErrNoSuchKey
 	}
 	retVal, err := h.a.appCtx.GetSyspolicyStringArrayJSONValue(key)
-	if err != nil && !errors.Is(err, syspolicy.ErrNoSuchKey) {
-		log.Printf("syspolicy: %s (strArr) will not be set: %v", key, err)
+	if err := translateHandlerError(err); err != nil {
 		return nil, err
 	}
 	if retVal == "" {
@@ -67,3 +58,10 @@ func (h syspolicyHandler) ReadStringArray(key string) ([]string, error) {
 	}
 	return arr, err
 }
+
+func translateHandlerError(err error) error {
+	if err != nil && !errors.Is(err, syspolicy.ErrNoSuchKey) && err.Error() == syspolicy.ErrNoSuchKey.Error() {
+		return syspolicy.ErrNoSuchKey
+	}
+	return err // may be nil or non-nil
+}