drive: move normalizeShareName into pkg drive and make func public (#11638)

This change makes the normalizeShareName function public, so it can be
used for validation in control.

Updates tailscale/corp#16827

Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>

drive/remote.go

@@ -7,7 +7,9 @@
 
 import (
 	"bytes"
+	"errors"
 	"net/http"
+	"regexp"
 	"strings"
 )
 
@@ -15,6 +17,12 @@
 	// DisallowShareAs forcibly disables sharing as a specific user, only used
 	// for testing.
 	DisallowShareAs     = false
+	ErrDriveNotEnabled  = errors.New("Taildrive not enabled")
+	ErrInvalidShareName = errors.New("Share names may only contain the letters a-z, underscore _, parentheses (), or spaces")
+)
+
+var (
+	shareNameRegex = regexp.MustCompile(`^[a-z0-9_\(\) ]+$`)
 )
 
 // AllowShareAs reports whether sharing files as a specific user is allowed.
@@ -103,3 +111,20 @@ type FileSystemForRemote interface {
 	// Close() stops serving the WebDAV content
 	Close() error
 }
+
+// NormalizeShareName normalizes the given share name and returns an error if
+// it contains any disallowed characters.
+func NormalizeShareName(name string) (string, error) {
+	// Force all share names to lowercase to avoid potential incompatibilities
+	// with clients that don't support case-sensitive filenames.
+	name = strings.ToLower(name)
+
+	// Trim whitespace
+	name = strings.TrimSpace(name)
+
+	if !shareNameRegex.MatchString(name) {
+		return "", ErrInvalidShareName
+	}
+
+	return name, nil
+}

drive/remote_test.go

@@ -1,7 +1,7 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-package ipnlocal
+package drive
 
 import (
 	"fmt"
@@ -29,7 +29,7 @@ func TestNormalizeShareName(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(fmt.Sprintf("name %q", tt.name), func(t *testing.T) {
-			got, err := normalizeShareName(tt.name)
+			got, err := NormalizeShareName(tt.name)
 			if tt.err != nil && err != tt.err {
 				t.Errorf("wanted error %v, got %v", tt.err, err)
 			} else if got != tt.want {

ipn/ipnlocal/drive.go

@@ -4,10 +4,8 @@
 package ipnlocal
 
 import (
-	"errors"
 	"fmt"
 	"os"
-	"regexp"
 	"slices"
 	"strings"
 
@@ -24,12 +22,6 @@
 	DriveLocalPort = 8080
 )
 
-var (
-	shareNameRegex      = regexp.MustCompile(`^[a-z0-9_\(\) ]+$`)
-	ErrDriveNotEnabled  = errors.New("Taildrive not enabled")
-	ErrInvalidShareName = errors.New("Share names may only contain the letters a-z, underscore _, parentheses (), or spaces")
-)
-
 // DriveSharingEnabled reports whether sharing to remote nodes via Taildrive is
 // enabled. This is currently based on checking for the drive:share node
 // attribute.
@@ -62,7 +54,7 @@ func (b *LocalBackend) driveAccessEnabledLocked() bool {
 func (b *LocalBackend) DriveSetServerAddr(addr string) error {
 	fs, ok := b.sys.DriveForRemote.GetOK()
 	if !ok {
-		return ErrDriveNotEnabled
+		return drive.ErrDriveNotEnabled
 	}
 
 	fs.SetFileServerAddr(addr)
@@ -75,7 +67,7 @@ func (b *LocalBackend) DriveSetServerAddr(addr string) error {
 // limited to alphanumeric characters and the underscore _.
 func (b *LocalBackend) DriveSetShare(share *drive.Share) error {
 	var err error
-	share.Name, err = normalizeShareName(share.Name)
+	share.Name, err = drive.NormalizeShareName(share.Name)
 	if err != nil {
 		return err
 	}
@@ -91,29 +83,12 @@ func (b *LocalBackend) DriveSetShare(share *drive.Share) error {
 	return nil
 }
 
-// normalizeShareName normalizes the given share name and returns an error if
-// it contains any disallowed characters.
-func normalizeShareName(name string) (string, error) {
-	// Force all share names to lowercase to avoid potential incompatibilities
-	// with clients that don't support case-sensitive filenames.
-	name = strings.ToLower(name)
-
-	// Trim whitespace
-	name = strings.TrimSpace(name)
-
-	if !shareNameRegex.MatchString(name) {
-		return "", ErrInvalidShareName
-	}
-
-	return name, nil
-}
-
 func (b *LocalBackend) driveSetShareLocked(share *drive.Share) (views.SliceView[*drive.Share, drive.ShareView], error) {
 	existingShares := b.pm.prefs.DriveShares()
 
 	fs, ok := b.sys.DriveForRemote.GetOK()
 	if !ok {
-		return existingShares, ErrDriveNotEnabled
+		return existingShares, drive.ErrDriveNotEnabled
 	}
 
 	addedShare := false
@@ -151,7 +126,7 @@ func (b *LocalBackend) driveSetShareLocked(share *drive.Share) (views.SliceView[
 // - share already exists under new name
 func (b *LocalBackend) DriveRenameShare(oldName, newName string) error {
 	var err error
-	newName, err = normalizeShareName(newName)
+	newName, err = drive.NormalizeShareName(newName)
 	if err != nil {
 		return err
 	}
@@ -172,7 +147,7 @@ func (b *LocalBackend) driveRenameShareLocked(oldName, newName string) (views.Sl
 
 	fs, ok := b.sys.DriveForRemote.GetOK()
 	if !ok {
-		return existingShares, ErrDriveNotEnabled
+		return existingShares, drive.ErrDriveNotEnabled
 	}
 
 	found := false
@@ -212,7 +187,7 @@ func (b *LocalBackend) DriveRemoveShare(name string) error {
 	// Force all share names to lowercase to avoid potential incompatibilities
 	// with clients that don't support case-sensitive filenames.
 	var err error
-	name, err = normalizeShareName(name)
+	name, err = drive.NormalizeShareName(name)
 	if err != nil {
 		return err
 	}
@@ -233,7 +208,7 @@ func (b *LocalBackend) driveRemoveShareLocked(name string) (views.SliceView[*dri
 
 	fs, ok := b.sys.DriveForRemote.GetOK()
 	if !ok {
-		return existingShares, ErrDriveNotEnabled
+		return existingShares, drive.ErrDriveNotEnabled
 	}
 
 	found := false

ipn/ipnlocal/local_test.go

@@ -2404,13 +2404,13 @@ func TestDriveManageShares(t *testing.T) {
 		{
 			name:   "add_bad_name",
 			add:    &drive.Share{Name: "$"},
-			expect: ErrInvalidShareName,
+			expect: drive.ErrInvalidShareName,
 		},
 		{
 			name:     "add_disabled",
 			disabled: true,
 			add:      &drive.Share{Name: "a"},
-			expect:   ErrDriveNotEnabled,
+			expect:   drive.ErrDriveNotEnabled,
 		},
 		{
 			name: "remove",
@@ -2439,7 +2439,7 @@ func TestDriveManageShares(t *testing.T) {
 			name:     "remove_disabled",
 			disabled: true,
 			remove:   "b",
-			expect:   ErrDriveNotEnabled,
+			expect:   drive.ErrDriveNotEnabled,
 		},
 		{
 			name: "rename",
@@ -2474,13 +2474,13 @@ func TestDriveManageShares(t *testing.T) {
 		{
 			name:   "rename_bad_name",
 			rename: [2]string{"a", "$"},
-			expect: ErrInvalidShareName,
+			expect: drive.ErrInvalidShareName,
 		},
 		{
 			name:     "rename_disabled",
 			disabled: true,
 			rename:   [2]string{"a", "c"},
-			expect:   ErrDriveNotEnabled,
+			expect:   drive.ErrDriveNotEnabled,
 		},
 	}
 

ipn/localapi/localapi.go

@@ -2792,7 +2792,7 @@ func (h *Handler) serveShares(w http.ResponseWriter, r *http.Request) {
 		}
 		err = h.b.DriveSetShare(&share)
 		if err != nil {
-			if errors.Is(err, ipnlocal.ErrInvalidShareName) {
+			if errors.Is(err, drive.ErrInvalidShareName) {
 				http.Error(w, "invalid share name", http.StatusBadRequest)
 				return
 			}
@@ -2833,7 +2833,7 @@ func (h *Handler) serveShares(w http.ResponseWriter, r *http.Request) {
 				http.Error(w, "share name already used", http.StatusBadRequest)
 				return
 			}
-			if errors.Is(err, ipnlocal.ErrInvalidShareName) {
+			if errors.Is(err, drive.ErrInvalidShareName) {
 				http.Error(w, "invalid share name", http.StatusBadRequest)
 				return
 			}