Fix regression in AbsoluteProxifyURL()

Regression introduced in commit 66b8483 PR #2499
miniflux · Mar 19, 2024 · 4cd6039 · 4cd6039
1 parent fa9697b
commit 4cd6039
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 19 deletions.
diff --git a/internal/googlereader/handler.go b/internal/googlereader/handler.go
@@ -766,7 +766,7 @@ func subscribe(newFeed Stream, category Stream, title string, store *storage.Sto
 	}
 
 	created, localizedError := mff.CreateFeed(store, userID, &feedRequest)
-	if err != nil {
+	if localizedError != nil {
 		return nil, localizedError.Error()
 	}
 

diff --git a/internal/proxy/media_proxy_test.go b/internal/proxy/media_proxy_test.go
@@ -157,6 +157,31 @@ func TestProxyFilterWithHttpsAlways(t *testing.T) {
 	}
 }
 
+func TestAbsoluteProxyFilterWithHttpsAlways(t *testing.T) {
+	os.Clearenv()
+	os.Setenv("PROXY_OPTION", "all")
+	os.Setenv("PROXY_MEDIA_TYPES", "image")
+	os.Setenv("PROXY_PRIVATE_KEY", "test")
+
+	var err error
+	parser := config.NewParser()
+	config.Opts, err = parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	r := mux.NewRouter()
+	r.HandleFunc("/proxy/{encodedDigest}/{encodedURL}", func(w http.ResponseWriter, r *http.Request) {}).Name("proxy")
+
+	input := `<p><img src="https://website/folder/image.png" alt="Test"/></p>`
+	output := AbsoluteProxyRewriter(r, "localhost", input)
+	expected := `<p><img src="http://localhost/proxy/LdPNR1GBDigeeNp2ArUQRyZsVqT_PWLfHGjYFrrWWIY=/aHR0cHM6Ly93ZWJzaXRlL2ZvbGRlci9pbWFnZS5wbmc=" alt="Test"/></p>`
+
+	if expected != output {
+		t.Errorf(`Not expected output: got "%s" instead of "%s"`, output, expected)
+	}
+}
+
 func TestProxyFilterWithHttpsAlwaysAndCustomProxyServer(t *testing.T) {
 	os.Clearenv()
 	os.Setenv("PROXY_OPTION", "all")
@@ -182,6 +207,56 @@ func TestProxyFilterWithHttpsAlwaysAndCustomProxyServer(t *testing.T) {
 	}
 }
 
+func TestProxyFilterWithHttpsAlwaysAndIncorrectCustomProxyServer(t *testing.T) {
+	os.Clearenv()
+	os.Setenv("PROXY_OPTION", "all")
+	os.Setenv("PROXY_MEDIA_TYPES", "image")
+	os.Setenv("PROXY_URL", "http://:8080example.com")
+
+	var err error
+	parser := config.NewParser()
+	config.Opts, err = parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	r := mux.NewRouter()
+	r.HandleFunc("/proxy/{encodedDigest}/{encodedURL}", func(w http.ResponseWriter, r *http.Request) {}).Name("proxy")
+
+	input := `<p><img src="https://website/folder/image.png" alt="Test"/></p>`
+	output := ProxyRewriter(r, input)
+	expected := `<p><img src="https://website/folder/image.png" alt="Test"/></p>`
+
+	if expected != output {
+		t.Errorf(`Not expected output: got "%s" instead of "%s"`, output, expected)
+	}
+}
+
+func TestAbsoluteProxyFilterWithHttpsAlwaysAndCustomProxyServer(t *testing.T) {
+	os.Clearenv()
+	os.Setenv("PROXY_OPTION", "all")
+	os.Setenv("PROXY_MEDIA_TYPES", "image")
+	os.Setenv("PROXY_URL", "https://proxy-example/proxy")
+
+	var err error
+	parser := config.NewParser()
+	config.Opts, err = parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	r := mux.NewRouter()
+	r.HandleFunc("/proxy/{encodedDigest}/{encodedURL}", func(w http.ResponseWriter, r *http.Request) {}).Name("proxy")
+
+	input := `<p><img src="https://website/folder/image.png" alt="Test"/></p>`
+	output := ProxyRewriter(r, input)
+	expected := `<p><img src="https://proxy-example/proxy/aHR0cHM6Ly93ZWJzaXRlL2ZvbGRlci9pbWFnZS5wbmc=" alt="Test"/></p>`
+
+	if expected != output {
+		t.Errorf(`Not expected output: got "%s" instead of "%s"`, output, expected)
+	}
+}
+
 func TestProxyFilterWithHttpInvalid(t *testing.T) {
 	os.Clearenv()
 	os.Setenv("PROXY_OPTION", "invalid")

diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go
@@ -7,6 +7,7 @@ import (
 	"crypto/hmac"
 	"crypto/sha256"
 	"encoding/base64"
+	"log/slog"
 	"net/url"
 	"path"
 
@@ -18,35 +19,50 @@ import (
 )
 
 // ProxifyURL generates a relative URL for a proxified resource.
-func ProxifyURL(router *mux.Router, link string) string {
-	if link == "" {
+func ProxifyURL(router *mux.Router, originalResourceURL string) string {
+	if originalResourceURL == "" {
 		return ""
 	}
 
-	if proxyImageUrl := config.Opts.ProxyUrl(); proxyImageUrl != "" {
-		proxyUrl, err := url.Parse(proxyImageUrl)
-		if err != nil {
-			return ""
-		}
-		proxyUrl.Path = path.Join(proxyUrl.Path, base64.URLEncoding.EncodeToString([]byte(link)))
-		return proxyUrl.String()
+	if customProxyURL := config.Opts.ProxyUrl(); customProxyURL != "" {
+		return ProxifyURLWithCustomProxy(originalResourceURL, customProxyURL)
 	}
 
 	mac := hmac.New(sha256.New, config.Opts.ProxyPrivateKey())
-	mac.Write([]byte(link))
+	mac.Write([]byte(originalResourceURL))
 	digest := mac.Sum(nil)
-	return route.Path(router, "proxy", "encodedDigest", base64.URLEncoding.EncodeToString(digest), "encodedURL", base64.URLEncoding.EncodeToString([]byte(link)))
+	return route.Path(router, "proxy", "encodedDigest", base64.URLEncoding.EncodeToString(digest), "encodedURL", base64.URLEncoding.EncodeToString([]byte(originalResourceURL)))
 }
 
 // AbsoluteProxifyURL generates an absolute URL for a proxified resource.
-func AbsoluteProxifyURL(router *mux.Router, host, link string) string {
-	proxifiedUrl := ProxifyURL(router, link)
-
-	if config.Opts.ProxyUrl() == "" {
-		return proxifiedUrl
+func AbsoluteProxifyURL(router *mux.Router, host, originalResourceURL string) string {
+	if customProxyURL := config.Opts.ProxyUrl(); customProxyURL != "" {
+		return ProxifyURLWithCustomProxy(originalResourceURL, customProxyURL)
 	}
+
+	proxifiedUrl := ProxifyURL(router, originalResourceURL)
+	scheme := "http"
 	if config.Opts.HTTPS {
-		return "https://" + host + proxifiedUrl
+		scheme = "https"
 	}
-	return "http://" + host + proxifiedUrl
+
+	return scheme + "://" + host + proxifiedUrl
+}
+
+func ProxifyURLWithCustomProxy(originalResourceURL, customProxyURL string) string {
+	if customProxyURL == "" {
+		return originalResourceURL
+	}
+
+	proxyUrl, err := url.Parse(customProxyURL)
+	if err != nil {
+		slog.Error("Incorrect custom media proxy URL",
+			slog.String("custom_proxy_url", customProxyURL),
+			slog.Any("error", err),
+		)
+		return originalResourceURL
+	}
+
+	proxyUrl.Path = path.Join(proxyUrl.Path, base64.URLEncoding.EncodeToString([]byte(originalResourceURL)))
+	return proxyUrl.String()
 }