Use a more specific error for http source rejections (#415)

* use a more specific error for http source rejections * version bump * update unit tests
cshum · Mar 13, 2024 · 7f386ea · 7f386ea
1 parent cf1590e
commit 7f386ea
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 15 deletions.
diff --git a/errors.go b/errors.go
@@ -4,11 +4,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/cshum/imagor/imagorpath"
 	"net/http"
 	"regexp"
 	"strconv"
 	"strings"
+
+	"github.com/cshum/imagor/imagorpath"
 )
 
 var (
@@ -18,6 +19,8 @@ var (
 	ErrInvalid = NewError("invalid", http.StatusBadRequest)
 	// ErrMethodNotAllowed method not allowed error
 	ErrMethodNotAllowed = NewError("method not allowed", http.StatusMethodNotAllowed)
+	// ErrSourceNotAllowed http source not allowed error
+	ErrSourceNotAllowed = NewError("http source not allowed", http.StatusForbidden)
 	// ErrSignatureMismatch URL signature mismatch error
 	ErrSignatureMismatch = NewError("url signature mismatch", http.StatusForbidden)
 	// ErrTimeout timeout error

diff --git a/imagor.go b/imagor.go
@@ -5,10 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/cshum/imagor/imagorpath"
-	"go.uber.org/zap"
-	"golang.org/x/sync/semaphore"
-	"golang.org/x/sync/singleflight"
 	"io"
 	"net/http"
 	"net/url"
@@ -18,10 +14,15 @@ import (
 	"strings"
 	"sync"
 	"time"
+
+	"github.com/cshum/imagor/imagorpath"
+	"go.uber.org/zap"
+	"golang.org/x/sync/semaphore"
+	"golang.org/x/sync/singleflight"
 )
 
 // Version imagor version
-const Version = "1.4.9"
+const Version = "1.4.10"
 
 // Loader image loader interface
 type Loader interface {

diff --git a/loader/httploader/httploader.go b/loader/httploader/httploader.go
@@ -158,7 +158,7 @@ func (h *HTTPLoader) Get(r *http.Request, image string) (*imagor.Blob, error) {
 	u.Fragment = ""
 
 	if !isURLAllowed(u, h.AllowedSources) {
-		return nil, imagor.ErrInvalid
+		return nil, imagor.ErrSourceNotAllowed
 	}
 	client := &http.Client{
 		Transport:     h.Transport,
@@ -256,7 +256,7 @@ func (h *HTTPLoader) checkRedirect(r *http.Request, via []*http.Request) error {
 		return errors.New("stopped after 10 redirects")
 	}
 	if !isURLAllowed(r.URL, h.AllowedSources) {
-		return imagor.ErrInvalid
+		return imagor.ErrSourceNotAllowed
 	}
 	return nil
 }

diff --git a/loader/httploader/httploader_test.go b/loader/httploader/httploader_test.go
@@ -108,17 +108,17 @@ func TestWithAllowedSources(t *testing.T) {
 		{
 			name:   "not allowed source",
 			target: "https://foo.boo/boooo",
-			err:    "imagor: 400 invalid",
+			err:    "imagor: 403 http source not allowed",
 		},
 		{
 			name:   "not allowed source",
 			target: "https://foo.barr/baz",
-			err:    "imagor: 400 invalid",
+			err:    "imagor: 403 http source not allowed",
 		},
 		{
 			name:   "not allowed source",
 			target: "https://boo.bar/baz",
-			err:    "imagor: 400 invalid",
+			err:    "imagor: 403 http source not allowed",
 		},
 		{
 			name:   "csv allowed source",
@@ -163,17 +163,17 @@ func TestWithAllowedSourceRegexp(t *testing.T) {
 		{
 			name:   "not allowed source",
 			target: "https://goo2.org/https://goo.org/image.png",
-			err:    "imagor: 400 invalid",
+			err:    "imagor: 403 http source not allowed",
 		},
 		{
 			name:   "not allowed source",
 			target: "https://foo.com/dogs/../cats/cat.jpg",
-			err:    "imagor: 400 invalid",
+			err:    "imagor: 403 http source not allowed",
 		},
 		{
 			name:   "not allowed source",
 			target: "https://foo.com/dogs/dog.jpg?size=small",
-			err:    "imagor: 400 invalid",
+			err:    "imagor: 403 http source not allowed",
 		},
 	})
 }
@@ -203,7 +203,7 @@ func TestWithAllowedSourcesRedirect(t *testing.T) {
 
 		b, err := blob.ReadAll()
 		assert.Empty(t, b)
-		assert.ErrorIs(t, err, imagor.ErrInvalid)
+		assert.ErrorIs(t, err, imagor.ErrSourceNotAllowed)
 	})
 
 	t.Run("Allowed redirect", func(t *testing.T) {