Przeglądaj źródła

refactor(validator): replace IsValidURL with urllib.IsAbsoluteURL

Frédéric Guillot 2 tygodni temu
rodzic
commit
4cd9dd6af7

+ 2 - 1
internal/googlereader/handler.go

@@ -24,6 +24,7 @@ import (
 	mff "miniflux.app/v2/internal/reader/handler"
 	mfs "miniflux.app/v2/internal/reader/subscription"
 	"miniflux.app/v2/internal/storage"
+	"miniflux.app/v2/internal/urllib"
 	"miniflux.app/v2/internal/validator"
 
 	"github.com/gorilla/mux"
@@ -405,7 +406,7 @@ func (h *handler) quickAddHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	feedURL := r.Form.Get(paramQuickAdd)
-	if !validator.IsValidURL(feedURL) {
+	if !urllib.IsAbsoluteURL(feedURL) {
 		json.BadRequest(w, r, fmt.Errorf("googlereader: invalid URL: %s", feedURL))
 		return
 	}

+ 3 - 2
internal/ui/form/subscription.go

@@ -8,6 +8,7 @@ import (
 	"strconv"
 
 	"miniflux.app/v2/internal/locale"
+	"miniflux.app/v2/internal/urllib"
 	"miniflux.app/v2/internal/validator"
 )
 
@@ -40,7 +41,7 @@ func (s *SubscriptionForm) Validate() *locale.LocalizedError {
 		return locale.NewLocalizedError("error.feed_mandatory_fields")
 	}
 
-	if !validator.IsValidURL(s.URL) {
+	if !urllib.IsAbsoluteURL(s.URL) {
 		return locale.NewLocalizedError("error.invalid_feed_url")
 	}
 
@@ -56,7 +57,7 @@ func (s *SubscriptionForm) Validate() *locale.LocalizedError {
 		return locale.NewLocalizedError("error.feed_invalid_urlrewrite_rule")
 	}
 
-	if s.ProxyURL != "" && !validator.IsValidURL(s.ProxyURL) {
+	if s.ProxyURL != "" && !urllib.IsAbsoluteURL(s.ProxyURL) {
 		return locale.NewLocalizedError("error.invalid_feed_proxy_url")
 	}
 

+ 3 - 3
internal/urllib/url.go

@@ -34,10 +34,10 @@ func hasHTTPPrefix(inputURL string) bool {
 	return strings.HasPrefix(inputURL, "https://") || strings.HasPrefix(inputURL, "http://")
 }
 
-// IsAbsoluteURL reports whether the link is absolute.
+// IsAbsoluteURL reports whether the link is absolute and starts with an HTTP or HTTPS scheme.
 func IsAbsoluteURL(inputURL string) bool {
-	if hasHTTPPrefix(inputURL) {
-		return true
+	if !hasHTTPPrefix(inputURL) {
+		return false
 	}
 	parsedURL, err := url.Parse(inputURL)
 	if err != nil {

+ 10 - 4
internal/urllib/url_test.go

@@ -45,10 +45,16 @@ func TestIsRelativePath(t *testing.T) {
 
 func TestIsAbsoluteURL(t *testing.T) {
 	scenarios := map[string]bool{
-		"https://example.org/file.pdf": true,
-		"magnet:?xt.1=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C&xt.2=urn:sha1:TXGCZQTH26NL6OUQAJJPFALHG2LTGBC7": true,
-		"invalid url":    false,
-		"/relative/path": false,
+		"https://example.org/file.pdf":                   true,
+		"https://example.org/file.pdf?download=1#page=2": true,
+		"mailto:user@example.org":                        false,
+		"data:text/plain,hello":                          false,
+		"magnet:?xt.1=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C&xt.2=urn:sha1:TXGCZQTH26NL6OUQAJJPFALHG2LTGBC7": false,
+		"invalid url":                false,
+		"/relative/path":             false,
+		"//example.org/path":         false,
+		" https://example.org/path":  false,
+		"\thttps://example.org/path": false,
 	}
 
 	for input, expected := range scenarios {

+ 6 - 5
internal/validator/feed.go

@@ -7,6 +7,7 @@ import (
 	"miniflux.app/v2/internal/locale"
 	"miniflux.app/v2/internal/model"
 	"miniflux.app/v2/internal/storage"
+	"miniflux.app/v2/internal/urllib"
 )
 
 // ValidateFeedCreation validates feed creation.
@@ -15,7 +16,7 @@ func ValidateFeedCreation(store *storage.Storage, userID int64, request *model.F
 		return locale.NewLocalizedError("error.feed_mandatory_fields")
 	}
 
-	if !IsValidURL(request.FeedURL) {
+	if !urllib.IsAbsoluteURL(request.FeedURL) {
 		return locale.NewLocalizedError("error.invalid_feed_url")
 	}
 
@@ -35,7 +36,7 @@ func ValidateFeedCreation(store *storage.Storage, userID int64, request *model.F
 		return locale.NewLocalizedError("error.feed_invalid_keeplist_rule")
 	}
 
-	if request.ProxyURL != "" && !IsValidURL(request.ProxyURL) {
+	if request.ProxyURL != "" && !urllib.IsAbsoluteURL(request.ProxyURL) {
 		return locale.NewLocalizedError("error.invalid_feed_proxy_url")
 	}
 
@@ -49,7 +50,7 @@ func ValidateFeedModification(store *storage.Storage, userID, feedID int64, requ
 			return locale.NewLocalizedError("error.feed_url_not_empty")
 		}
 
-		if !IsValidURL(*request.FeedURL) {
+		if !urllib.IsAbsoluteURL(*request.FeedURL) {
 			return locale.NewLocalizedError("error.invalid_feed_url")
 		}
 
@@ -63,7 +64,7 @@ func ValidateFeedModification(store *storage.Storage, userID, feedID int64, requ
 			return locale.NewLocalizedError("error.site_url_not_empty")
 		}
 
-		if !IsValidURL(*request.SiteURL) {
+		if !urllib.IsAbsoluteURL(*request.SiteURL) {
 			return locale.NewLocalizedError("error.invalid_site_url")
 		}
 	}
@@ -97,7 +98,7 @@ func ValidateFeedModification(store *storage.Storage, userID, feedID int64, requ
 			return locale.NewLocalizedError("error.proxy_url_not_empty")
 		}
 
-		if !IsValidURL(*request.ProxyURL) {
+		if !urllib.IsAbsoluteURL(*request.ProxyURL) {
 			return locale.NewLocalizedError("error.invalid_feed_proxy_url")
 		}
 	}

+ 3 - 2
internal/validator/subscription.go

@@ -6,15 +6,16 @@ package validator // import "miniflux.app/v2/internal/validator"
 import (
 	"miniflux.app/v2/internal/locale"
 	"miniflux.app/v2/internal/model"
+	"miniflux.app/v2/internal/urllib"
 )
 
 // ValidateSubscriptionDiscovery validates subscription discovery requests.
 func ValidateSubscriptionDiscovery(request *model.SubscriptionDiscoveryRequest) *locale.LocalizedError {
-	if !IsValidURL(request.URL) {
+	if !urllib.IsAbsoluteURL(request.URL) {
 		return locale.NewLocalizedError("error.invalid_site_url")
 	}
 
-	if request.ProxyURL != "" && !IsValidURL(request.ProxyURL) {
+	if request.ProxyURL != "" && !urllib.IsAbsoluteURL(request.ProxyURL) {
 		return locale.NewLocalizedError("error.invalid_proxy_url")
 	}
 

+ 0 - 7
internal/validator/validator.go

@@ -5,7 +5,6 @@ package validator // import "miniflux.app/v2/internal/validator"
 
 import (
 	"errors"
-	"net/url"
 	"regexp"
 	"strings"
 )
@@ -41,12 +40,6 @@ func IsValidRegex(expr string) bool {
 	return err == nil
 }
 
-// IsValidURL verifies if the provided value is a valid absolute URL.
-func IsValidURL(absoluteURL string) bool {
-	_, err := url.ParseRequestURI(absoluteURL)
-	return err == nil
-}
-
 // IsValidDomain verifies a single domain name against length and character constraints.
 func IsValidDomain(domain string) bool {
 	domain = strings.ToLower(domain)

+ 1 - 18
internal/validator/validator_test.go

@@ -3,24 +3,7 @@
 
 package validator // import "miniflux.app/v2/internal/validator"
 
-import (
-	"testing"
-)
-
-func TestIsValidURL(t *testing.T) {
-	scenarios := map[string]bool{
-		"https://www.example.org": true,
-		"http://www.example.org/": true,
-		"www.example.org":         false,
-	}
-
-	for link, expected := range scenarios {
-		result := IsValidURL(link)
-		if result != expected {
-			t.Errorf(`Unexpected result, got %v instead of %v`, result, expected)
-		}
-	}
-}
+import "testing"
 
 func TestValidateRange(t *testing.T) {
 	if err := ValidateRange(-1, 0); err == nil {