diff --git a/internal/netutil.go b/internal/netutil.go deleted file mode 100644 index 677216d..0000000 --- a/internal/netutil.go +++ /dev/null @@ -1,57 +0,0 @@ -package netutil - -import ( - "fmt" - "net/url" - "os" - "path" - "path/filepath" - - "github.com/publiccodeyml/libpubliccode/v5/internal/httpclient" -) - -// downloadFile download the file in the path. -func downloadFile(client *httpclient.Client, filepath string, url *url.URL, headers map[string]string) error { - // Create the file. - out, err := os.Create(filepath) - if err != nil { - return fmt.Errorf("creating file: %w", err) - } - - defer func() { - _ = out.Close() - }() - - // Get the data from the url. - body, err := client.Get(url.String(), headers) - if err != nil { - return fmt.Errorf("downloading %s: %w", url, err) - } - - // Write the body to file. - if _, err = out.Write(body); err != nil { - return fmt.Errorf("writing file: %w", err) - } - - return nil -} - -// DownloadTmpFile downloads the passed URL to a temporary directory. -// Caller is responsible for removing the temporary directory. -func DownloadTmpFile(client *httpclient.Client, url *url.URL, headers map[string]string) (string, error) { - // Create a temp dir - tmpdir, err := os.MkdirTemp("", "publiccode.yml-parser-go") - if err != nil { - return "", fmt.Errorf("creating temp dir: %w", err) - } - - // Download the file in the temp dir. - tmpFile := filepath.Join(tmpdir, path.Base(url.Path)) - - err = downloadFile(client, tmpFile, url, headers) - if err != nil { - return "", err - } - - return tmpFile, nil -} diff --git a/internal/netutil_test.go b/internal/netutil_test.go deleted file mode 100644 index 9492855..0000000 --- a/internal/netutil_test.go +++ /dev/null @@ -1,84 +0,0 @@ -package netutil - -import ( - "net/http" - "net/http/httptest" - "net/url" - "os" - "testing" - - "github.com/publiccodeyml/libpubliccode/v5/internal/httpclient" -) - -func newTestClient(httpClient *http.Client) *httpclient.Client { - return httpclient.New(httpClient) -} - -func TestDownloadTmpFileSuccess(t *testing.T) { - const content = "hello from server" - - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(content)) - })) - defer srv.Close() - - client := newTestClient(srv.Client()) - - u, _ := url.Parse(srv.URL + "/testfile.txt") - path, err := DownloadTmpFile(client, u, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - defer func() { - dir := "" - if path != "" { - dir = path[:len(path)-len("/testfile.txt")] - _ = os.Remove(path) - _ = os.Remove(dir) - } - }() - - data, err := os.ReadFile(path) - if err != nil { - t.Fatalf("can't read downloaded file: %v", err) - } - if string(data) != content { - t.Errorf("unexpected content: %q", string(data)) - } -} - -func TestDownloadTmpFileHTTPError(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Error(w, "server error", http.StatusInternalServerError) - })) - defer srv.Close() - - client := newTestClient(srv.Client()) - - u, _ := url.Parse(srv.URL + "/testfile.txt") - _, err := DownloadTmpFile(client, u, nil) - if err == nil { - t.Error("expected error for a 5xx response") - } -} - -func TestDownloadTmpFileConnectionRefused(t *testing.T) { - client := newTestClient(&http.Client{}) - - u, _ := url.Parse("http://localhost:1/testfile.txt") - _, err := DownloadTmpFile(client, u, nil) - if err == nil { - t.Error("expected error for connection refused") - } -} - -func TestDownloadFileCreateFails(t *testing.T) { - client := newTestClient(&http.Client{}) - u, _ := url.Parse("http://localhost:1/testfile.txt") - - // Try to create a file in a path that doesn't exist. - err := downloadFile(client, "/nonexistent/directory/testfile.txt", u, nil) - if err == nil { - t.Error("expected error for nonexistent directory") - } -} diff --git a/v0_test.go b/v0_test.go index 076c05e..783543a 100644 --- a/v0_test.go +++ b/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,7 +790,7 @@ 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: @@ -798,7 +798,7 @@ func TestRelativePathsOrURLs(t *testing.T) { // // (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}, //}}, } diff --git a/validations.go b/validations.go index 7b085f9..fd2b3d0 100644 --- a/validations.go +++ b/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 @@ func (p *Parser) validLogo(u url.URL, network bool) (bool, error) { 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) } } diff --git a/validations_test.go b/validations_test.go index 8d62477..5361dd6 100644 --- a/validations_test.go +++ b/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: ""}, + {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) + } + } +}