Fix share access accounting and validation

Author: okatu-loli

internal/db/share.go

@@ -24,20 +24,20 @@ func GetShareByCreatorAndShareID(creatorID uint, shareID string) (*model.Share,
 	return &share, nil
 }
 
-func ShareIDExists(shareID string) bool {
+func ShareIDExists(shareID string) (bool, error) {
 	var count int64
 	if err := db.Model(&model.Share{}).Where("share_id = ?", shareID).Count(&count).Error; err != nil {
-		return false
+		return false, err
 	}
-	return count > 0
+	return count > 0, nil
 }
 
-func ShareIDExistsExceptID(shareID string, id uint) bool {
+func ShareIDExistsExceptID(shareID string, id uint) (bool, error) {
 	var count int64
 	if err := db.Model(&model.Share{}).Where("share_id = ? AND id <> ?", shareID, id).Count(&count).Error; err != nil {
-		return false
+		return false, err
 	}
-	return count > 0
+	return count > 0, nil
 }
 
 func CreateShare(share *model.Share) error {

server/handles/share.go

@@ -2,7 +2,9 @@ package handles
 
 import (
 	"crypto/subtle"
+	"errors"
 	"fmt"
+	"net/http"
 	"net/url"
 	stdpath "path"
 	"regexp"
@@ -24,6 +26,11 @@ const shareAccessTokenLifetime = 24 * time.Hour
 
 var shareIDPattern = regexp.MustCompile(`^[A-Za-z0-9_-]{1,32}$`)
 
+var (
+	errShareIDInvalid = errors.New("share_id must be 1-32 characters of letters, numbers, underscore or hyphen")
+	errShareIDExists  = errors.New("share link already exists")
+)
+
 type CreateShareReq struct {
 	Path          string `json:"path" binding:"required"`
 	ShareID       string `json:"share_id"`
@@ -184,7 +191,11 @@ func normalizeOptionalShareName(name, fallback string) string {
 func generateShareID() (string, error) {
 	for range 10 {
 		shareID := random.String(8)
-		if !db.ShareIDExists(shareID) {
+		exists, err := db.ShareIDExists(shareID)
+		if err != nil {
+			return "", err
+		}
+		if !exists {
 			return shareID, nil
 		}
 	}
@@ -200,7 +211,7 @@ func validateCustomShareID(shareID string) error {
 		return nil
 	}
 	if !shareIDPattern.MatchString(shareID) {
-		return fmt.Errorf("share_id must be 1-32 characters of letters, numbers, underscore or hyphen")
+		return errShareIDInvalid
 	}
 	return nil
 }
@@ -217,13 +228,21 @@ func resolveRequestedShareID(rawShareID, fallback string, excludeID uint) (strin
 		return "", err
 	}
 	if excludeID == 0 {
-		if db.ShareIDExists(shareID) {
-			return "", fmt.Errorf("share link already exists")
+		exists, err := db.ShareIDExists(shareID)
+		if err != nil {
+			return "", fmt.Errorf("check share id availability: %w", err)
+		}
+		if exists {
+			return "", errShareIDExists
 		}
 		return shareID, nil
 	}
-	if db.ShareIDExistsExceptID(shareID, excludeID) {
-		return "", fmt.Errorf("share link already exists")
+	exists, err := db.ShareIDExistsExceptID(shareID, excludeID)
+	if err != nil {
+		return "", fmt.Errorf("check share id availability: %w", err)
+	}
+	if exists {
+		return "", errShareIDExists
 	}
 	return shareID, nil
 }
@@ -334,6 +353,10 @@ func ensureShareAccess(c *gin.Context, share *model.Share, token string) bool {
 	return true
 }
 
+func shouldTrackShareContentAccess(c *gin.Context) bool {
+	return c.Request.Method != http.MethodHead
+}
+
 func resolveShareTarget(share *model.Share, rawRelPath string) (string, string, error) {
 	cleanRelPath := utils.FixAndCleanPath(rawRelPath)
 	if !share.IsDir && cleanRelPath != "/" {
@@ -349,6 +372,14 @@ func resolveShareTarget(share *model.Share, rawRelPath string) (string, string,
 	return target, cleanRelPath, nil
 }
 
+func resolveShareWildcardTarget(share *model.Share, rawPath string) (string, string, error) {
+	path, err := url.PathUnescape(rawPath)
+	if err != nil {
+		return "", "", err
+	}
+	return resolveShareTarget(share, strings.TrimPrefix(path, "/"))
+}
+
 func buildPublicShareAssetURL(c *gin.Context, prefix, shareID, relPath, token string, preview bool) string {
 	base := common.GetApiUrl(c.Request) + prefix + shareID
 	cleanPath := utils.FixAndCleanPath(relPath)
@@ -423,7 +454,11 @@ func CreateShare(c *gin.Context) {
 	}
 	shareID, err := resolveRequestedShareID(req.ShareID, "", 0)
 	if err != nil {
-		common.ErrorResp(c, err, 400)
+		if errors.Is(err, errShareIDInvalid) || errors.Is(err, errShareIDExists) {
+			common.ErrorResp(c, err, 400)
+			return
+		}
+		common.ErrorResp(c, err, 500, true)
 		return
 	}
 	allowPreview := true
@@ -483,7 +518,11 @@ func UpdateShare(c *gin.Context) {
 
 	shareID, err := resolveRequestedShareID(req.NewShareID, share.ShareID, share.ID)
 	if err != nil {
-		common.ErrorResp(c, err, 400)
+		if errors.Is(err, errShareIDInvalid) || errors.Is(err, errShareIDExists) {
+			common.ErrorResp(c, err, 400)
+			return
+		}
+		common.ErrorResp(c, err, 500, true)
 		return
 	}
 

server/handles/share_public.go

@@ -2,7 +2,6 @@ package handles
 
 import (
 	stdpath "path"
-	"strings"
 	"time"
 
 	"github.com/alist-org/alist/v3/internal/db"
@@ -143,10 +142,6 @@ func ListPublicShare(c *gin.Context) {
 		}
 		content = append(content, toPublicShareObjResp(c, share, item, itemTargetPath, itemRelPath, token))
 	}
-	if err := recordShareAccess(share); err != nil {
-		common.ErrorResp(c, err, 500, true)
-		return
-	}
 	common.SuccessResp(c, PublicShareListResp{
 		Content:    content,
 		Total:      int64(total),
@@ -213,7 +208,7 @@ func ShareDown(c *gin.Context) {
 	if !ensureShareAccess(c, share, token) {
 		return
 	}
-	targetPath, _, err := resolveShareTarget(share, strings.TrimPrefix(c.Param("path"), "/"))
+	targetPath, _, err := resolveShareWildcardTarget(share, c.Param("path"))
 	if err != nil {
 		common.ErrorResp(c, err, 400)
 		return
@@ -227,10 +222,12 @@ func ShareDown(c *gin.Context) {
 		common.ErrorStrResp(c, "directory download is not supported", 400)
 		return
 	}
-	_ = db.TouchShareDownload(share.ShareID)
-	if err := recordShareAccess(share); err != nil {
-		common.ErrorResp(c, err, 500, true)
-		return
+	if shouldTrackShareContentAccess(c) {
+		_ = db.TouchShareDownload(share.ShareID)
+		if err := recordShareAccess(share); err != nil {
+			common.ErrorResp(c, err, 500, true)
+			return
+		}
 	}
 	c.Set("path", targetPath)
 	Down(c)
@@ -253,7 +250,7 @@ func ShareProxy(c *gin.Context) {
 	if !ensureShareAccess(c, share, token) {
 		return
 	}
-	targetPath, _, err := resolveShareTarget(share, strings.TrimPrefix(c.Param("path"), "/"))
+	targetPath, _, err := resolveShareWildcardTarget(share, c.Param("path"))
 	if err != nil {
 		common.ErrorResp(c, err, 400)
 		return
@@ -267,9 +264,11 @@ func ShareProxy(c *gin.Context) {
 		common.ErrorStrResp(c, "directory preview is not supported", 400)
 		return
 	}
-	if err := recordShareAccess(share); err != nil {
-		common.ErrorResp(c, err, 500, true)
-		return
+	if shouldTrackShareContentAccess(c) {
+		if err := recordShareAccess(share); err != nil {
+			common.ErrorResp(c, err, 500, true)
+			return
+		}
 	}
 	c.Set("path", targetPath)
 	Proxy(c)