fix: skip repo avatar upload when no file is selected (#11335)

Submitting the repo avatar form without selecting a file shows a raw Go error: `Avatar.Open: open : no such file or directory.`. The existing `nil` check does not prevent this from happening.

The user avatar handler already guards against this same problem with [`form.Avatar != nil && form.Avatar.Filename != ""`](https://codeberg.org/forgejo/forgejo/src/commit/e1cecbd276841a5762ad034a920be8c2732e295f/routers/web/user/setting/profile.go#L141), I've done the same for the repo avatar handler.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11335
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Bram Hagens <bram@bramh.me>
Co-committed-by: Bram Hagens <bram@bramh.me>

Author: bramhaag

routers/web/repo/setting/avatar.go

@@ -4,13 +4,8 @@
 package setting
 
 import (
-	"errors"
 	"fmt"
-	"io"
 
-	"forgejo.org/modules/log"
-	"forgejo.org/modules/setting"
-	"forgejo.org/modules/typesniffer"
 	"forgejo.org/modules/web"
 	"forgejo.org/services/context"
 	"forgejo.org/services/forms"
@@ -19,37 +14,14 @@ import (
 
 // UpdateAvatarSetting update repo's avatar
 func UpdateAvatarSetting(ctx *context.Context, form forms.AvatarForm) error {
-	ctxRepo := ctx.Repo.Repository
-
-	if form.Avatar == nil {
-		// No avatar is uploaded and we not removing it here.
-		// No random avatar generated here.
-		// Just exit, no action.
-		if ctxRepo.CustomAvatarRelativePath() == "" {
-			log.Trace("No avatar was uploaded for repo: %d. Default icon will appear instead.", ctxRepo.ID)
-		}
-		return nil
-	}
-
-	r, err := form.Avatar.Open()
-	if err != nil {
-		return fmt.Errorf("Avatar.Open: %w", err)
-	}
-	defer r.Close()
-
-	if form.Avatar.Size > setting.Avatar.MaxFileSize {
-		return errors.New(ctx.Locale.TrString("settings.uploaded_avatar_is_too_big", form.Avatar.Size/1024, setting.Avatar.MaxFileSize/1024))
-	}
-
-	data, err := io.ReadAll(r)
+	data, err := forms.ReadAvatar(form.Avatar, ctx.Locale)
 	if err != nil {
-		return fmt.Errorf("io.ReadAll: %w", err)
+		return err
 	}
-	st := typesniffer.DetectContentType(data, "")
-	if !st.IsImage() || st.IsSvgImage() {
-		return errors.New(ctx.Locale.TrString("settings.uploaded_avatar_not_a_image"))
+	if data == nil {
+		return nil
 	}
-	if err = repo_service.UploadAvatar(ctx, ctxRepo, data); err != nil {
+	if err := repo_service.UploadAvatar(ctx, ctx.Repo.Repository, data); err != nil {
 		return fmt.Errorf("UploadAvatar: %w", err)
 	}
 	return nil

routers/web/user/setting/profile.go

@@ -5,9 +5,7 @@
 package setting
 
 import (
-	"errors"
 	"fmt"
-	"io"
 	"math/big"
 	"net/http"
 	"os"
@@ -25,7 +23,6 @@ import (
 	"forgejo.org/modules/optional"
 	"forgejo.org/modules/setting"
 	"forgejo.org/modules/translation"
-	"forgejo.org/modules/typesniffer"
 	"forgejo.org/modules/util"
 	"forgejo.org/modules/web"
 	"forgejo.org/modules/web/middleware"
@@ -138,27 +135,12 @@ func UpdateAvatarSetting(ctx *context.Context, form *forms.AvatarForm, ctxUser *
 		ctxUser.AvatarEmail = form.Gravatar
 	}
 
-	if form.Avatar != nil && form.Avatar.Filename != "" {
-		fr, err := form.Avatar.Open()
-		if err != nil {
-			return fmt.Errorf("Avatar.Open: %w", err)
-		}
-		defer fr.Close()
-
-		if form.Avatar.Size > setting.Avatar.MaxFileSize {
-			return errors.New(ctx.Locale.TrString("settings.uploaded_avatar_is_too_big", form.Avatar.Size/1024, setting.Avatar.MaxFileSize/1024))
-		}
-
-		data, err := io.ReadAll(fr)
-		if err != nil {
-			return fmt.Errorf("io.ReadAll: %w", err)
-		}
-
-		st := typesniffer.DetectContentType(data, "")
-		if !st.IsImage() || st.IsSvgImage() {
-			return errors.New(ctx.Locale.TrString("settings.uploaded_avatar_not_a_image"))
-		}
-		if err = user_service.UploadAvatar(ctx, ctxUser, data); err != nil {
+	data, err := forms.ReadAvatar(form.Avatar, ctx.Locale)
+	if err != nil {
+		return err
+	}
+	if data != nil {
+		if err := user_service.UploadAvatar(ctx, ctxUser, data); err != nil {
 			return fmt.Errorf("UploadAvatar: %w", err)
 		}
 	} else if ctxUser.UseCustomAvatar && ctxUser.Avatar == "" {

services/forms/avatar.go

@@ -0,0 +1,45 @@
+// Copyright 2018 The Gitea Authors. All rights reserved.
+// Copyright 2026 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package forms
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"mime/multipart"
+
+	"forgejo.org/modules/setting"
+	"forgejo.org/modules/translation"
+	"forgejo.org/modules/typesniffer"
+)
+
+// ReadAvatar reads and validates an avatar from a multipart file header.
+func ReadAvatar(header *multipart.FileHeader, locale translation.Locale) ([]byte, error) {
+	if header == nil || header.Filename == "" {
+		return nil, nil
+	}
+
+	r, err := header.Open()
+	if err != nil {
+		return nil, fmt.Errorf("Avatar.Open: %w", err)
+	}
+	defer r.Close()
+
+	if header.Size > setting.Avatar.MaxFileSize {
+		return nil, errors.New(locale.TrString("settings.uploaded_avatar_is_too_big", header.Size/1024, setting.Avatar.MaxFileSize/1024))
+	}
+
+	data, err := io.ReadAll(r)
+	if err != nil {
+		return nil, fmt.Errorf("io.ReadAll: %w", err)
+	}
+
+	st := typesniffer.DetectContentType(data, "")
+	if !st.IsImage() || st.IsSvgImage() {
+		return nil, errors.New(locale.TrString("settings.uploaded_avatar_not_a_image"))
+	}
+
+	return data, nil
+}

tests/integration/repo_avatar_test.go

@@ -0,0 +1,94 @@
+// Copyright 2026 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package integration
+
+import (
+	"bytes"
+	"image/png"
+	"io"
+	"mime/multipart"
+	"net/http"
+	"testing"
+
+	repo_model "forgejo.org/models/repo"
+	"forgejo.org/models/unittest"
+	user_model "forgejo.org/models/user"
+	"forgejo.org/modules/avatar"
+	app_context "forgejo.org/services/context"
+	"forgejo.org/tests"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRepoAvatar(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+
+	session := loginUser(t, user2.Name)
+	avatarURL := "/" + repo.OwnerName + "/" + repo.Name + "/settings/avatar"
+
+	t.Run("valid avatar", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		img, err := avatar.RandomImage([]byte("seed"))
+		require.NoError(t, err)
+
+		imgData := &bytes.Buffer{}
+		require.NoError(t, png.Encode(imgData, img))
+
+		body := &bytes.Buffer{}
+		writer := multipart.NewWriter(body)
+		require.NoError(t, writer.WriteField("source", "local"))
+		part, err := writer.CreateFormFile("avatar", "avatar.png")
+		require.NoError(t, err)
+		_, err = io.Copy(part, imgData)
+		require.NoError(t, err)
+		require.NoError(t, writer.Close())
+
+		req := NewRequestWithBody(t, "POST", avatarURL, body)
+		req.Header.Add("Content-Type", writer.FormDataContentType())
+		session.MakeRequest(t, req, http.StatusSeeOther)
+
+		flashCookie := session.GetCookie(app_context.CookieNameFlash)
+		require.NotNil(t, flashCookie)
+		assert.Contains(t, flashCookie.Value, "success")
+
+		// Verify avatar was actually set
+		repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+		assert.NotEmpty(t, repo.CustomAvatarRelativePath())
+
+		// Verify avatar is accessible
+		req = NewRequest(t, "GET", "/repo-avatars/"+repo.Avatar)
+		_ = session.MakeRequest(t, req, http.StatusOK)
+
+		// Clean up
+		req = NewRequest(t, "POST", avatarURL+"/delete")
+		session.MakeRequest(t, req, http.StatusOK)
+	})
+
+	t.Run("no file selected", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		// Simulate what a browser sends when user did not select a file
+		body := &bytes.Buffer{}
+		writer := multipart.NewWriter(body)
+		require.NoError(t, writer.WriteField("source", "local"))
+		_, err := writer.CreateFormFile("avatar", "")
+		require.NoError(t, err)
+		require.NoError(t, writer.Close())
+
+		req := NewRequestWithBody(t, "POST", avatarURL, body)
+		req.Header.Add("Content-Type", writer.FormDataContentType())
+		session.MakeRequest(t, req, http.StatusSeeOther)
+
+		// Should silently skip
+		flashCookie := session.GetCookie(app_context.CookieNameFlash)
+		if flashCookie != nil {
+			assert.NotContains(t, flashCookie.Value, "error")
+		}
+	})
+}