windows: make SID.String() conform to the String interface

This function shouldn't return an error. Like other String() functions everywhere in Golang, this should instead return empty or a token value during an error, so that it can be passed to %v and similar. Also, allow for SID strings of maximum size. Change-Id: Ib6d8407f8ad0bdabcb22c31b8f387594f2ea7672 Reviewed-on: https://go-review.googlesource.com/c/sys/+/196799 Run-TryBot: Jason A. Donenfeld <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Alex Brainman <[email protected]>
golang · Sep 22, 2019 · 0a153f0 · 0a153f0
1 parent 2dccfee
commit 0a153f0
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 13 deletions.
diff --git a/windows/security_windows.go b/windows/security_windows.go
@@ -227,16 +227,15 @@ func LookupSID(system, account string) (sid *SID, domain string, accType uint32,
 	}
 }
 
-// String converts SID to a string format
-// suitable for display, storage, or transmission.
-func (sid *SID) String() (string, error) {
+// String converts SID to a string format suitable for display, storage, or transmission.
+func (sid *SID) String() string {
 	var s *uint16
 	e := ConvertSidToStringSid(sid, &s)
 	if e != nil {
-		return "", e
+		return ""
 	}
 	defer LocalFree((Handle)(unsafe.Pointer(s)))
-	return UTF16ToString((*[256]uint16)(unsafe.Pointer(s))[:]), nil
+	return UTF16ToString((*[(1 << 30) - 1]uint16)(unsafe.Pointer(s))[:])
 }
 
 // Len returns the length, in bytes, of a valid security identifier SID.

diff --git a/windows/syscall_windows_test.go b/windows/syscall_windows_test.go
@@ -99,12 +99,8 @@ func TestCreateWellKnownSid(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Unable to create well known sid for administrators: %v", err)
 	}
-	sidStr, err := sid.String()
-	if err != nil {
-		t.Fatalf("Unable to convert sid into string: %v", err)
-	}
-	if sidStr != "S-1-5-32-544" {
-		t.Fatalf("Expecting administrators to be S-1-5-32-544, but found %s instead", sidStr)
+	if got, want := sid.String(), "S-1-5-32-544"; got != want {
+		t.Fatalf("Builtin Administrators SID = %s, want %s", got, want)
 	}
 }
 
@@ -279,8 +275,7 @@ func TestSddlConversion(t *testing.T) {
 		t.Fatal("Invalid security descriptor owner")
 	}
 	if !sdOwner.IsWellKnown(windows.WinBuiltinAdministratorsSid) {
-		got, _ := sdOwner.String()
-		t.Fatalf("Owner = %q; want S-1-5-32-544", got)
+		t.Fatalf("Owner = %q; want S-1-5-32-544", sdOwner)
 	}
 }