fix: validate remote logos with a single fetch

v0_test.go

@@ -230,7 +230,7 @@ func TestInvalidTestcasesV0(t *testing.T) {
 		},
 		// Local publiccode.yml and URL in logo: should look for the logo remotely
 		"logo_missing_url.yml": ValidationResults{
-			ValidationError{"", "logo", "HTTP GET failed for https://google.com/no_such_file.png: not found", 18, 1},
+			ValidationError{"", "logo", "downloading https://google.com/no_such_file.png: not found", 18, 1},
 		},
 
 		// monochromeLogo
@@ -790,15 +790,15 @@ func TestRelativePathsOrURLs(t *testing.T) {
 		// Remote publiccode.yml and URL in logo:
 		// should look for the logo remotely
 		{"https://raw.githubusercontent.com/italia/publiccode-parser-go/2a1f25a36e0f2ab2d71efb67db8b66ff00573fbe/testdata/v0/invalid/logo_missing_url.yml", ValidationResults{
-			ValidationError{"", "logo", "HTTP GET failed for https://google.com/no_such_file.png: not found", 18, 1},
+			ValidationError{"", "logo", "downloading https://google.com/no_such_file.png: not found", 18, 1},
 		}},
 
 		// Local publiccode.yml and URL in logo:
 		// should look for the logo remotely
 		//
 		// (already tested in TestInvalidTestcasesV0)
 		// "testdata/v0/invalid/logo_missing_url.yml", ValidationResults{
-		//	ValidationError{"logo", "HTTP GET failed for https://google.com/no_such_file.png: not found", 18, 1},
+		//	ValidationError{"logo", "downloading https://google.com/no_such_file.png: not found", 18, 1},
 		//}},
 	}
 

validations.go

@@ -1,12 +1,14 @@
 package publiccode
 
 import (
+	"bytes"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"image"
 	"image/png"
+	"io"
 	"math/rand"
 	"net/url"
 	"os"
@@ -179,51 +181,40 @@
 		return false, fmt.Errorf("invalid file extension for: %s", netutil.DisplayURL(&u)) //nolint:err113,lll // dynamic message with path context
 	}
 
-	if exists, err := p.fileExists(u, network); !exists {
-		return false, err
-	}
+	var content io.Reader
 
-	var localPath string
-	// Remote. Create a temp dir, download and check the file. Remove the temp dir.
-	if u.Scheme != "file" {
-		var err error
+	if u.Scheme == "file" {
+		if exists, err := p.fileExists(u, network); !exists {
+			return false, err
+		}
 
-		if !network {
+		if ext != ".png" {
 			return true, nil
 		}
 
-		localPath, err = netutil.DownloadTmpFile(p.httpclient, &u, getHeaderFromDomain(p.domain, u.String()))
+		f, err := os.Open(u.Path)
 		if err != nil {
-			return false, fmt.Errorf("downloading %s: %w", u.String(), err)
+			return false, fmt.Errorf("opening %s: %w", u.Path, err)
 		}
 
-		defer func() {
-			if localPath == "" {
-				return
-			}
-
-			if err := os.Remove(localPath); err != nil { //nolint:gosec // G703: path from validated download
-				fmt.Fprintf(os.Stderr, "failed to remove %s: %v\n", localPath, err)
-			}
+		defer f.Close()
 
-			dir := path.Dir(localPath)
-			if err = os.Remove(dir); err != nil { //nolint:gosec // G703: path from validated download
-				fmt.Fprintf(os.Stderr, "failed to remove %s: %v\n", dir, err)
-			}
-		}()
+		content = f
 	} else {
-		localPath = u.Path
-	}
+		if !network {
+			return true, nil
+		}
 
-	if ext == ".png" {
-		f, err := os.Open(localPath) //nolint:gosec // G703: path from validated download
+		body, err := p.httpclient.Get(u.String(), getHeaderFromDomain(p.domain, u.String()))
 		if err != nil {
-			return false, fmt.Errorf("opening %s: %w", localPath, err)
+			return false, fmt.Errorf("downloading %s: %w", u.String(), err)
 		}
 
-		defer f.Close()
+		content = bytes.NewReader(body)
+	}
 
-		if _, _, err := image.DecodeConfig(f); err != nil {
+	if ext == ".png" {
+		if _, _, err := image.DecodeConfig(content); err != nil {
 			return false, fmt.Errorf("%w", err)
 		}
 	}

validations_test.go

@@ -1,9 +1,14 @@
 package publiccode
 
 import (
+	"bytes"
+	"image"
+	"image/png"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -197,7 +202,6 @@ func TestValidLogoRemoteDownloadFailure(t *testing.T) {
 	}
 }
 
-
 func TestIsReachableSuccess(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
@@ -211,3 +215,95 @@ func TestIsReachableSuccess(t *testing.T) {
 		t.Errorf("expected reachable for 200 response, got err: %v", err)
 	}
 }
+
+func TestValidLogoRemoteSingleFetch(t *testing.T) {
+	var pngBody bytes.Buffer
+	if err := png.Encode(&pngBody, image.NewRGBA(image.Rect(0, 0, 1, 1))); err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tc := range []struct {
+		name      string
+		body      string
+		status    int
+		wantError string
+	}{
+		{name: "logo.svg", body: "<svg/>"},
+		{name: "logo.svgz", body: "compressed SVG"},
+		{name: "logo.jpg", body: "JPEG"},
+		{name: "logo.jpeg", body: "JPEG"},
+		{name: "logo.webp", body: "WebP"},
+		{name: "logo.avif", body: "AVIF"},
+		{name: "logo.png", body: pngBody.String()},
+		{name: "invalid.png", body: "invalid PNG", wantError: "image: unknown format"},
+		{name: "missing.svg", status: http.StatusInternalServerError, wantError: "unexpected status 500"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			tmp := t.TempDir()
+			t.Setenv("TMPDIR", tmp)
+			requests := 0
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				requests++
+				if r.Header.Get("Authorization") != "Basic dXNlcjpwYXNz" {
+					t.Error("missing configured authorization")
+				}
+				if requests > 1 {
+					w.WriteHeader(http.StatusInternalServerError)
+					return
+				}
+				if tc.status != 0 {
+					w.WriteHeader(tc.status)
+				}
+				_, _ = w.Write([]byte(tc.body))
+			}))
+			defer srv.Close()
+
+			p, err := NewParser(ParserConfig{AllowNetworkToPrivateHosts: true})
+			if err != nil {
+				t.Fatal(err)
+			}
+			u, _ := url.Parse(srv.URL + "/" + tc.name)
+			p.domain = Domain{UseTokenFor: []string{u.Host}, BasicAuth: []string{"user:pass"}}
+			ok, err := p.validLogo(*u, true)
+			if tc.wantError == "" {
+				if !ok || err != nil {
+					t.Fatalf("expected valid logo, got %v, %v", ok, err)
+				}
+			} else if ok || err == nil || !strings.Contains(err.Error(), tc.wantError) {
+				t.Fatalf("expected %q, got %v, %v", tc.wantError, ok, err)
+			}
+			if tc.status != 0 && strings.Count(err.Error(), "downloading ") != 1 {
+				t.Errorf("expected one downloading prefix: %v", err)
+			}
+			if requests != 1 {
+				t.Errorf("expected one request, got %d", requests)
+			}
+			entries, err := os.ReadDir(tmp)
+			if err != nil || len(entries) != 0 {
+				t.Errorf("temporary directory is not empty: %v, %v", entries, err)
+			}
+		})
+	}
+}
+
+func TestValidLogoLocalPNG(t *testing.T) {
+	p, err := NewParser(ParserConfig{DisableNetwork: true})
+	if err != nil {
+		t.Fatal(err)
+	}
+	filename := filepath.Join(t.TempDir(), "logo.png")
+	var body bytes.Buffer
+	if err := png.Encode(&body, image.NewRGBA(image.Rect(0, 0, 1, 1))); err != nil {
+		t.Fatal(err)
+	}
+	for _, content := range [][]byte{body.Bytes(), []byte("invalid PNG")} {
+		if err := os.WriteFile(filename, content, 0o600); err != nil {
+			t.Fatal(err)
+		}
+		ok, err := p.validLogo(url.URL{Scheme: "file", Path: filename}, false)
+		wantValid := bytes.Equal(content, body.Bytes())
+		if ok != wantValid || (err == nil) != wantValid {
+			t.Errorf("expected valid=%v, got %v, %v", wantValid, ok, err)
+		}
+	}
+}