-
-
Notifications
You must be signed in to change notification settings - Fork 166
feat: add custom avatar upload functionality: #279 #321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,149 @@ | ||||||||||||||||||||||||||||||||||||||||
| package controllers | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||||||||
| "log" | ||||||||||||||||||||||||||||||||||||||||
| "mime/multipart" | ||||||||||||||||||||||||||||||||||||||||
| "net/http" | ||||||||||||||||||||||||||||||||||||||||
| "os" | ||||||||||||||||||||||||||||||||||||||||
| "path/filepath" | ||||||||||||||||||||||||||||||||||||||||
| "strings" | ||||||||||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| "arguehub/db" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| "github.com/gin-gonic/gin" | ||||||||||||||||||||||||||||||||||||||||
| "github.com/google/uuid" | ||||||||||||||||||||||||||||||||||||||||
| "go.mongodb.org/mongo-driver/bson" | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const ( | ||||||||||||||||||||||||||||||||||||||||
| maxAvatarSize = 5 << 20 // 5 MB | ||||||||||||||||||||||||||||||||||||||||
| avatarUploadDir = "./uploads/avatars" | ||||||||||||||||||||||||||||||||||||||||
| avatarURLPrefix = "/uploads/avatars" | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // allowedAvatarExtensions maps lowercase extensions to their expected MIME types. | ||||||||||||||||||||||||||||||||||||||||
| var allowedAvatarExtensions = map[string][]string{ | ||||||||||||||||||||||||||||||||||||||||
| ".jpg": {"image/jpeg"}, | ||||||||||||||||||||||||||||||||||||||||
| ".jpeg": {"image/jpeg"}, | ||||||||||||||||||||||||||||||||||||||||
| ".png": {"image/png"}, | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // UploadAvatar handles avatar file uploads for authenticated users. | ||||||||||||||||||||||||||||||||||||||||
| func UploadAvatar(c *gin.Context) { | ||||||||||||||||||||||||||||||||||||||||
| email := c.GetString("email") | ||||||||||||||||||||||||||||||||||||||||
| if email == "" { | ||||||||||||||||||||||||||||||||||||||||
| c.JSON(http.StatusUnauthorized, gin.H{"error": "Unauthorized"}) | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Limit request body size | ||||||||||||||||||||||||||||||||||||||||
| c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxAvatarSize+512) // small buffer for multipart overhead | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| file, header, err := c.Request.FormFile("avatar") | ||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||
| if err.Error() == "http: request body too large" { | ||||||||||||||||||||||||||||||||||||||||
| c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "File too large. Maximum size is 5MB."}) | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| c.JSON(http.StatusBadRequest, gin.H{"error": "No file provided or invalid upload. Use key 'avatar'."}) | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+46
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result:
Docs: see the Related note (Go 1.19 release notes): Sources: Citations:
🌐 Web query:
💡 Result:
1) Put a hard cap on upload size (recommended)Wrap the request body before calling func upload(w http.ResponseWriter, r *http.Request) {
const maxBody = 50 << 20 // 50 MiB (whole request, not just the file)
r.Body = http.MaxBytesReader(w, r.Body, maxBody)
if err := r.ParseMultipartForm(32 << 20); err != nil {
var mbe *http.MaxBytesError
if errors.As(err, &mbe) {
http.Error(w, "upload too large", http.StatusRequestEntityTooLarge) // 413
return
}
http.Error(w, "bad multipart form", http.StatusBadRequest)
return
}
f, fh, err := r.FormFile("file")
if err != nil {
http.Error(w, "missing/invalid file", http.StatusBadRequest)
return
}
defer f.Close()
// Optional per-file check (not a security boundary by itself):
if fh.Size > (40 << 20) {
http.Error(w, "file too large", http.StatusRequestEntityTooLarge)
return
}
// stream/save f...
}Why this works:
2) For truly large files: stream instead of
|
||||||||||||||||||||||||||||||||||||||||
| if err != nil { | |
| if err.Error() == "http: request body too large" { | |
| c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "File too large. Maximum size is 5MB."}) | |
| return | |
| } | |
| c.JSON(http.StatusBadRequest, gin.H{"error": "No file provided or invalid upload. Use key 'avatar'."}) | |
| return | |
| } | |
| if err != nil { | |
| var maxBytesErr *http.MaxBytesError | |
| if errors.As(err, &maxBytesErr) { | |
| c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "File too large. Maximum size is 5MB."}) | |
| return | |
| } | |
| c.JSON(http.StatusBadRequest, gin.H{"error": "No file provided or invalid upload. Use key 'avatar'."}) | |
| return | |
| } |
🤖 Prompt for AI Agents
In `@backend/controllers/upload_controller.go` around lines 46 - 53, The current
error check compares err.Error() to a string; instead use errors.As to detect
*http.MaxBytesError and handle it specially: in the upload handler where err is
returned (the block using c.JSON), attempt var maxErr *http.MaxBytesError; if
errors.As(err,&maxErr) respond with http.StatusRequestEntityTooLarge and the
same "File too large. Maximum size is 5MB." message, otherwise fall back to the
generic BadRequest response; ensure you import the errors package and reference
*http.MaxBytesError and the handler function that processes the upload (the
block currently checking err and calling c.JSON).
Copilot
AI
Feb 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avatarURL is built with a hardcoded http://localhost:1313 base, which will break in production and in any non-default dev setup. Consider building the URL from the incoming request (scheme/host, respecting reverse-proxy headers) or storing a relative path like /uploads/avatars/<file> and letting the frontend/baseURL resolve it.
| // Build the public URL | |
| avatarURL := fmt.Sprintf("http://localhost:1313%s/%s", avatarURLPrefix, uniqueName) | |
| // Build the public URL as a relative path; let the frontend/base URL resolve the host and scheme. | |
| avatarURL := fmt.Sprintf("%s/%s", avatarURLPrefix, uniqueName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded localhost:1313 URL will break in any non-local deployment.
The avatar URL stored in MongoDB will be unusable in staging/production. This should use the request's host/scheme or a configurable base URL from the application config.
Proposed fix
- avatarURL := fmt.Sprintf("http://localhost:1313%s/%s", avatarURLPrefix, uniqueName)
+ scheme := "http"
+ if c.Request.TLS != nil {
+ scheme = "https"
+ }
+ avatarURL := fmt.Sprintf("%s://%s%s/%s", scheme, c.Request.Host, avatarURLPrefix, uniqueName)Alternatively, inject a base URL from the application configuration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| avatarURL := fmt.Sprintf("http://localhost:1313%s/%s", avatarURLPrefix, uniqueName) | |
| scheme := "http" | |
| if c.Request.TLS != nil { | |
| scheme = "https" | |
| } | |
| avatarURL := fmt.Sprintf("%s://%s%s/%s", scheme, c.Request.Host, avatarURLPrefix, uniqueName) |
🤖 Prompt for AI Agents
In `@backend/controllers/upload_controller.go` at line 95, Replace the hardcoded
"http://localhost:1313" used to build avatarURL with a dynamic base URL: read
the scheme and host from the incoming *http.Request (request.URL.Scheme or infer
via r.TLS and r.Host) or, preferably, a configurable base URL from your app
config, and then join that base with avatarURLPrefix and uniqueName; update the
avatar URL construction (the avatarURL variable) to use this computed base URL
and include a sensible fallback if neither request-derived host nor config is
available.
Copilot
AI
Feb 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says “still return the URL so the frontend can retry” but the response on DB failure only returns an error field (no URL). Please either include the URL in the error response (if that’s the intended behavior) or update the comment and clean up the saved file so the system doesn’t get into a half-written state.
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Avatar saved but failed to update profile. Please try again."}) | |
| c.JSON(http.StatusInternalServerError, gin.H{ | |
| "error": "Avatar saved but failed to update profile. Please try again.", | |
| "avatar_url": avatarURL, | |
| }) |
Copilot
AI
Feb 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the DB update fails or no user is matched, the avatar file has already been written to disk and is never cleaned up, leading to orphaned files accumulating over time. Either remove the saved file on these failure paths (and/or delete the user’s previous custom avatar when replacing it) or restructure the flow to avoid committing the file until the DB update succeeds.
| // File was saved but DB failed — still return the URL so the frontend can retry | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Avatar saved but failed to update profile. Please try again."}) | |
| return | |
| } | |
| if result.MatchedCount == 0 { | |
| log.Printf("UploadAvatar: No user found with email: %s", email) | |
| // DB update failed; remove the saved file to avoid leaving an orphaned avatar on disk. | |
| if removeErr := os.Remove(savePath); removeErr != nil { | |
| log.Printf("UploadAvatar: Failed to remove orphaned avatar file %s: %v", savePath, removeErr) | |
| } | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Avatar saved but failed to update profile. Please try again."}) | |
| return | |
| } | |
| if result.MatchedCount == 0 { | |
| log.Printf("UploadAvatar: No user found with email: %s", email) | |
| // No user matched; remove the saved file to avoid leaving an orphaned avatar on disk. | |
| if removeErr := os.Remove(savePath); removeErr != nil { | |
| log.Printf("UploadAvatar: Failed to remove orphaned avatar file %s: %v", savePath, removeErr) | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User email logged in plaintext — PII concern.
Lines 116 and 121 log the user's email address. Consider logging a user ID or a hashed/truncated identifier instead to reduce PII exposure in logs.
🤖 Prompt for AI Agents
In `@backend/controllers/upload_controller.go` around lines 115 - 121, The logs in
UploadAvatar currently print the user's email (variables email) on both failure
and success paths (checked via result.MatchedCount and avatarURL); replace those
plaintext emails with a non-PII identifier such as the user's ID (if available
on the user object) or a hashed/truncated version of email (e.g., SHA256 or
first/last 4 chars) before logging so logs no longer contain full PII. Locate
the UploadAvatar handler and update the log.Printf calls to use the chosen safe
identifier (derived from email or using user.ID) while keeping the rest of the
messages unchanged.
Copilot
AI
Feb 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint returns avatar_url (snake_case), while the rest of the API uses avatarUrl (camelCase) for the same field (e.g., profile responses). For consistency and to reduce client-side special casing, return avatarUrl here as well (and align the frontend typings/usages accordingly).
| "message": "Avatar uploaded successfully", | |
| "avatar_url": avatarURL, | |
| "message": "Avatar uploaded successfully", | |
| "avatarUrl": avatarURL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No cleanup of old avatar files on re-upload — unbounded disk growth.
Each upload creates a new UUID-named file but never deletes the previous avatar. Over time this leaks disk space. Before saving the new file, query the user's current avatarUrl, extract the filename, and delete it from disk.
🤖 Prompt for AI Agents
In `@backend/controllers/upload_controller.go` around lines 101 - 127, In the
UploadAvatar handler, prevent orphaned avatar files by reading the user's
existing avatarUrl before saving the new file: query the users collection (e.g.,
with db.MongoDatabase.Collection("users").FindOne using the same email/dbCtx),
extract the existing filename from the returned avatarUrl, and delete that file
from disk (os.Remove) if it exists and is not the default placeholder; then
proceed to save the new UUID-named file and run the current UpdateOne call that
sets "avatarUrl" and "updatedAt". Ensure deletion errors are logged but do not
block the new upload, and avoid deleting if avatarUrl is empty or matches a
shared default.
Copilot
AI
Feb 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateMIMEType returns false on any read error. For small files, Read can return n > 0 with err == io.EOF, which would incorrectly reject otherwise valid images. Consider treating io.EOF as non-fatal when n > 0 (and import/use io).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seek error silently ignored; consider returning false on failure.
If the file doesn't implement Seek or if Seek fails, subsequent reads (e.g., c.SaveUploadedFile) will read from byte 512 onward, producing a corrupt saved file. At minimum, return false when the seek interface is unavailable or when seek errors.
Proposed fix
func validateMIMEType(file multipart.File, allowedMIMEs []string) bool {
buf := make([]byte, 512)
n, err := file.Read(buf)
if err != nil {
return false
}
- // Reset reader position for subsequent reads
- if seeker, ok := file.(interface{ Seek(int64, int) (int64, error) }); ok {
- seeker.Seek(0, 0)
+ seeker, ok := file.(interface{ Seek(int64, int) (int64, error) })
+ if !ok {
+ return false
+ }
+ if _, err := seeker.Seek(0, 0); err != nil {
+ return false
}
detectedType := http.DetectContentType(buf[:n])🤖 Prompt for AI Agents
In `@backend/controllers/upload_controller.go` around lines 130 - 148, The
validateMIMEType function currently ignores whether the uploaded file supports
seeking and ignores seek errors—change the logic in validateMIMEType to assert
the file to io.Seeker (or equivalent interface), and if the assertion fails or
seeker.Seek(0, io.SeekStart) returns an error, return false immediately; this
ensures readers like c.SaveUploadedFile will see the file from the start and
prevents saving corrupted uploads. Keep the rest of the MIME detection flow
(reading 512 bytes, DetectContentType, comparing against allowedMIMEs)
unchanged.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,7 +71,7 @@ import { | |
| ChartTooltip, | ||
| ChartTooltipContent, | ||
| } from "@/components/ui/chart"; | ||
| import { getProfile, updateProfile } from "@/services/profileService"; | ||
| import { getProfile, updateProfile, uploadAvatar } from "@/services/profileService"; | ||
| import { getAuthToken } from "@/utils/auth"; | ||
| import { DateRange } from "react-day-picker"; | ||
| import AvatarModal from "../components/AvatarModal"; | ||
|
|
@@ -177,6 +177,8 @@ const Profile: React.FC = () => { | |
| to: undefined, | ||
| }); | ||
| const inputRef = useRef<HTMLInputElement>(null); | ||
| const fileInputRef = useRef<HTMLInputElement>(null); | ||
| const [avatarUploading, setAvatarUploading] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| const fetchDashboard = async () => { | ||
|
|
@@ -315,6 +317,52 @@ const Profile: React.FC = () => { | |
| } | ||
| }; | ||
|
|
||
| const handleAvatarUpload = async (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const file = e.target.files?.[0]; | ||
| if (!file || !dashboard?.profile) return; | ||
|
|
||
| // Validate file type | ||
| const allowedTypes = ["image/jpeg", "image/png"]; | ||
| if (!allowedTypes.includes(file.type)) { | ||
| setErrorMessage("Invalid file type. Only JPG and PNG files are allowed."); | ||
| return; | ||
| } | ||
|
|
||
| // Validate file size (5MB) | ||
| const MAX_SIZE = 5 * 1024 * 1024; | ||
| if (file.size > MAX_SIZE) { | ||
| setErrorMessage("File too large. Maximum size is 5MB."); | ||
| return; | ||
| } | ||
|
|
||
| const token = getAuthToken(); | ||
| if (!token) { | ||
| setErrorMessage("Authentication token is missing."); | ||
| return; | ||
| } | ||
|
Comment on lines
+324
to
+342
|
||
|
|
||
| setAvatarUploading(true); | ||
| setErrorMessage(""); | ||
|
|
||
| try { | ||
| const result = await uploadAvatar(token, file); | ||
| // Update local state immediately | ||
| setDashboard({ | ||
| ...dashboard, | ||
| profile: { ...dashboard.profile, avatarUrl: result.avatar_url }, | ||
| }); | ||
| setSuccessMessage("Avatar uploaded successfully!"); | ||
| } catch (err) { | ||
| setErrorMessage( | ||
| err instanceof Error ? err.message : "Failed to upload avatar." | ||
| ); | ||
| } finally { | ||
| setAvatarUploading(false); | ||
| // Reset file input so re-selecting the same file triggers onChange | ||
| if (fileInputRef.current) fileInputRef.current.value = ""; | ||
| } | ||
| }; | ||
|
|
||
| if (loading) { | ||
| return ( | ||
| <div className="p-4 flex justify-center items-center min-h-[calc(100vh-4rem)]"> | ||
|
|
@@ -769,18 +817,40 @@ const Profile: React.FC = () => { | |
| )} | ||
| <div className="flex flex-col items-center mb-4"> | ||
| <div className="relative w-16 h-16 sm:w-20 sm:h-20 md:w-24 md:h-24 rounded-full overflow-hidden bg-muted flex-shrink-0 mb-2 border-2 border-primary shadow-md group"> | ||
| <img | ||
| src={profile.avatarUrl || defaultAvatar} | ||
| alt="Avatar" | ||
| className="object-cover w-full h-full" | ||
| {avatarUploading ? ( | ||
| <div className="flex items-center justify-center w-full h-full bg-muted"> | ||
| <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-primary" /> | ||
| </div> | ||
| ) : ( | ||
| <img | ||
| src={profile.avatarUrl || defaultAvatar} | ||
| alt="Avatar" | ||
| className="object-cover w-full h-full" | ||
| /> | ||
| )} | ||
| <div className="absolute inset-0 bg-black bg-opacity-50 flex items-center justify-center gap-2 opacity-0 group-hover:opacity-100 transition-opacity"> | ||
| <button | ||
| onClick={() => fileInputRef.current?.click()} | ||
| className="p-1 rounded-full hover:bg-white/20 transition-colors" | ||
| title="Upload Photo" | ||
| > | ||
| <ChevronRight className="w-5 h-5 text-white rotate-90" /> | ||
| </button> | ||
| <button | ||
| onClick={() => setIsAvatarModalOpen(true)} | ||
| className="p-1 rounded-full hover:bg-white/20 transition-colors" | ||
| title="Choose Avatar" | ||
| > | ||
| <ImageIcon className="w-5 h-5 text-white" /> | ||
| </button> | ||
| </div> | ||
| <input | ||
| ref={fileInputRef} | ||
| type="file" | ||
| accept="image/jpeg,image/png" | ||
| className="hidden" | ||
| onChange={handleAvatarUpload} | ||
| /> | ||
| <button | ||
| onClick={() => setIsAvatarModalOpen(true)} | ||
| className="absolute inset-0 bg-black bg-opacity-50 flex items-center justify-center opacity-0 group-hover:opacity-100 transition-opacity" | ||
| title="Edit Avatar" | ||
| > | ||
| <ImageIcon className="w-6 h-6 text-white" /> | ||
| </button> | ||
| </div> | ||
| <AvatarModal | ||
| isOpen={isAvatarModalOpen} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The request-size error check relies on comparing
err.Error()to a specific string, which is brittle (the error can be wrapped or have a different message). Prefer detecting*http.MaxBytesErrorviaerrors.As(or similar) so oversized uploads reliably return 413.