ShortURL: Use new Error type (#50859)

pkg/api/short_url.go

@@ -1,10 +1,8 @@
 package api
 
 import (
-	"errors"
 	"fmt"
 	"net/http"
-	"path"
 	"strings"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
@@ -19,24 +17,12 @@ import (
 func (hs *HTTPServer) createShortURL(c *models.ReqContext) response.Response {
 	cmd := dtos.CreateShortURLCmd{}
 	if err := web.Bind(c.Req, &cmd); err != nil {
-		return response.Error(http.StatusBadRequest, "bad request data", err)
+		return response.Err(models.ErrShortURLBadRequest.Errorf("bad request data: %w", err))
 	}
 	hs.log.Debug("Received request to create short URL", "path", cmd.Path)
-
-	cmd.Path = strings.TrimSpace(cmd.Path)
-
-	if path.IsAbs(cmd.Path) {
-		hs.log.Error("Invalid short URL path", "path", cmd.Path)
-		return response.Error(400, "Path should be relative", nil)
-	}
-	if strings.Contains(cmd.Path, "../") {
-		hs.log.Error("Invalid short URL path", "path", cmd.Path)
-		return response.Error(400, "Invalid path", nil)
-	}
-
 	shortURL, err := hs.ShortURLService.CreateShortURL(c.Req.Context(), c.SignedInUser, cmd.Path)
 	if err != nil {
-		return response.Error(500, "Failed to create short URL", err)
+		return response.Err(err)
 	}
 
 	url := fmt.Sprintf("%s/goto/%s?orgId=%d", strings.TrimSuffix(setting.AppUrl, "/"), shortURL.Uid, c.OrgId)
@@ -59,7 +45,7 @@ func (hs *HTTPServer) redirectFromShortURL(c *models.ReqContext) {
 
 	shortURL, err := hs.ShortURLService.GetShortURLByUID(c.Req.Context(), c.SignedInUser, shortURLUID)
 	if err != nil {
-		if errors.Is(err, models.ErrShortURLNotFound) {
+		if models.ErrShortURLNotFound.Is(err) {
 			hs.log.Debug("Not redirecting short URL since not found")
 			return
 		}

pkg/models/shorturl.go

@@ -1,12 +1,17 @@
 package models
 
 import (
-	"errors"
 	"time"
+
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 var (
-	ErrShortURLNotFound = errors.New("short URL not found")
+	ErrShortURLBadRequest   = errutil.NewBase(errutil.StatusBadRequest, "shorturl.bad-request")
+	ErrShortURLNotFound     = errutil.NewBase(errutil.StatusNotFound, "shorturl.not-found")
+	ErrShortURLAbsolutePath = errutil.NewBase(errutil.StatusValidationFailed, "shorturl.absolute-path", errutil.WithPublicMessage("Path should be relative"))
+	ErrShortURLInvalidPath  = errutil.NewBase(errutil.StatusValidationFailed, "shorturl.invalid-path", errutil.WithPublicMessage("Invalid short URL path"))
+	ErrShortURLInternal     = errutil.NewBase(errutil.StatusInternal, "shorturl.internal")
 )
 
 type ShortUrl struct {

pkg/services/shorturls/short_url_service.go

@@ -2,6 +2,8 @@ package shorturls
 
 import (
 	"context"
+	"path"
+	"strings"
 	"time"
 
 	"github.com/grafana/grafana/pkg/models"
@@ -36,7 +38,7 @@ func (s ShortURLService) GetShortURLByUID(ctx context.Context, user *models.Sign
 			return err
 		}
 		if !exists {
-			return models.ErrShortURLNotFound
+			return models.ErrShortURLNotFound.Errorf("short URL not found")
 		}
 
 		return nil
@@ -60,12 +62,21 @@ func (s ShortURLService) UpdateLastSeenAt(ctx context.Context, shortURL *models.
 	})
 }
 
-func (s ShortURLService) CreateShortURL(ctx context.Context, user *models.SignedInUser, path string) (*models.ShortUrl, error) {
+func (s ShortURLService) CreateShortURL(ctx context.Context, user *models.SignedInUser, relPath string) (*models.ShortUrl, error) {
+	relPath = strings.TrimSpace(relPath)
+
+	if path.IsAbs(relPath) {
+		return nil, models.ErrShortURLAbsolutePath.Errorf("expected relative path: %s", relPath)
+	}
+	if strings.Contains(relPath, "../") {
+		return nil, models.ErrShortURLInvalidPath.Errorf("path cannot contain '../': %s", relPath)
+	}
+
 	now := time.Now().Unix()
 	shortURL := models.ShortUrl{
 		OrgId:     user.OrgId,
 		Uid:       util.GenerateShortUID(),
-		Path:      path,
+		Path:      relPath,
 		CreatedBy: user.UserId,
 		CreatedAt: now,
 	}
@@ -75,7 +86,7 @@ func (s ShortURLService) CreateShortURL(ctx context.Context, user *models.Signed
 		return err
 	})
 	if err != nil {
-		return nil, err
+		return nil, models.ErrShortURLInternal.Errorf("failed to insert shorturl: %w", err)
 	}
 
 	return &shortURL, nil

pkg/services/shorturls/short_url_service_test.go

@@ -5,9 +5,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/stretchr/testify/require"
 )
 
 func TestShortURLService(t *testing.T) {
@@ -79,7 +80,7 @@ func TestShortURLService(t *testing.T) {
 
 		shortURL, err := service.GetShortURLByUID(context.Background(), user, "testnotfounduid")
 		require.Error(t, err)
-		require.Equal(t, models.ErrShortURLNotFound, err)
+		require.True(t, models.ErrShortURLNotFound.Is(err))
 		require.Nil(t, shortURL)
 	})
 }

pkg/util/errutil/errors.go

@@ -80,6 +80,18 @@ func (b Base) Errorf(format string, args ...interface{}) Error {
 	}
 }
 
+// Is validates that an Error has the same reason and messageID as the
+// Base.
+func (b Base) Is(err error) bool {
+	gfErr := Error{}
+	ok := errors.As(err, &gfErr)
+	if !ok {
+		return false
+	}
+
+	return b.reason.Status() == gfErr.Reason.Status() && b.messageID == gfErr.MessageID
+}
+
 // Error is the error type for errors within Grafana, extending
 // the Go error type with Grafana specific metadata to reduce
 // boilerplate error handling for status codes and internationalization