Skip to content

Commit 30a6077

Browse files
committed
Fix Unicode character support and form validation feedback
- Support international characters (accents, apostrophes) in names - Update regex to use Unicode letter/number patterns (\p{L}, \p{N}) - Add explicit dangerous character blacklist for security - Add JavaScript escaping helper for Alpine.js x-data attributes - Return HTML error messages instead of JSON for better UX - Add WebSocket error messaging for real-time validation feedback - Create reusable ErrorDisplay component for consistent error UI - Add GetClient method to Hub for targeted error messaging - Comprehensive test coverage for French, German, Spanish, and other international names
1 parent f7d37be commit 30a6077

10 files changed

Lines changed: 153 additions & 33 deletions

File tree

internal/handlers/room.go

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ func (h *RoomHandlers) CreateRoom(re *core.RequestEvent) error {
4040
// Validate and sanitize room name
4141
sanitizedName, err := security.ValidateRoomName(name)
4242
if err != nil {
43-
return re.JSON(http.StatusBadRequest, map[string]string{
44-
"error": err.Error(),
45-
})
43+
// Return HTML error for htmx or regular form submission
44+
component := templates.ErrorDisplay(err.Error())
45+
re.Response.WriteHeader(http.StatusBadRequest)
46+
return templates.Render(re.Response, re.Request, component)
4647
}
4748
name = sanitizedName
4849

@@ -59,26 +60,26 @@ func (h *RoomHandlers) CreateRoom(re *core.RequestEvent) error {
5960
customValues = h.voteValidator.GetFibonacciValues()
6061
case "custom":
6162
if customValuesRaw == "" {
62-
return re.JSON(http.StatusBadRequest, map[string]string{
63-
"error": "Custom values are required when using custom pointing method",
64-
})
63+
component := templates.ErrorDisplay("Custom values are required when using custom pointing method")
64+
re.Response.WriteHeader(http.StatusBadRequest)
65+
return templates.Render(re.Response, re.Request, component)
6566
}
6667

6768
parsedValues, err := h.voteValidator.ParseCustomValues(customValuesRaw)
6869
if err != nil {
69-
return re.JSON(http.StatusBadRequest, map[string]string{
70-
"error": fmt.Sprintf("Invalid custom values: %s", err.Error()),
71-
})
70+
component := templates.ErrorDisplay(fmt.Sprintf("Invalid custom values: %s", err.Error()))
71+
re.Response.WriteHeader(http.StatusBadRequest)
72+
return templates.Render(re.Response, re.Request, component)
7273
}
7374
customValues = parsedValues
7475
}
7576

7677
// Create room in database
7778
roomRecord, err := h.roomManager.CreateRoom(name, pointingMethod, customValues)
7879
if err != nil {
79-
return re.JSON(http.StatusInternalServerError, map[string]string{
80-
"error": "Failed to create room",
81-
})
80+
component := templates.ErrorDisplay("Failed to create room. Please try again.")
81+
re.Response.WriteHeader(http.StatusInternalServerError)
82+
return templates.Render(re.Response, re.Request, component)
8283
}
8384

8485
// Redirect to room
@@ -259,26 +260,26 @@ func (h *RoomHandlers) JoinRoom(re *core.RequestEvent) error {
259260

260261
// Validate room ID
261262
if err := security.ValidateUUID(roomID); err != nil {
262-
return re.JSON(http.StatusBadRequest, map[string]string{
263-
"error": "Invalid room ID",
264-
})
263+
component := templates.ErrorDisplay("Invalid room ID")
264+
re.Response.WriteHeader(http.StatusBadRequest)
265+
return templates.Render(re.Response, re.Request, component)
265266
}
266267

267268
// Validate and sanitize participant name
268269
sanitizedName, err := security.ValidateParticipantName(name)
269270
if err != nil {
270-
return re.JSON(http.StatusBadRequest, map[string]string{
271-
"error": err.Error(),
272-
})
271+
component := templates.ErrorDisplay(err.Error())
272+
re.Response.WriteHeader(http.StatusBadRequest)
273+
return templates.Render(re.Response, re.Request, component)
273274
}
274275
name = sanitizedName
275276

276277
// Verify room exists
277278
_, err = h.roomManager.GetRoom(roomID)
278279
if err != nil {
279-
return re.JSON(http.StatusNotFound, map[string]string{
280-
"error": "Room not found",
281-
})
280+
component := templates.ErrorDisplay("Room not found")
281+
re.Response.WriteHeader(http.StatusNotFound)
282+
return templates.Render(re.Response, re.Request, component)
282283
}
283284

284285
// Determine role
@@ -293,9 +294,9 @@ func (h *RoomHandlers) JoinRoom(re *core.RequestEvent) error {
293294
// Create participant in database
294295
participantRecord, err := h.roomManager.AddParticipant(roomID, name, participantRole, sessionCookie)
295296
if err != nil {
296-
return re.JSON(http.StatusInternalServerError, map[string]string{
297-
"error": "Failed to join room",
298-
})
297+
component := templates.ErrorDisplay("Failed to join room. Please try again.")
298+
re.Response.WriteHeader(http.StatusInternalServerError)
299+
return templates.Render(re.Response, re.Request, component)
299300
}
300301

301302
// Set cookie

internal/handlers/ws.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,35 +626,54 @@ func (h *WSHandler) getRoomState(roomID string) (models.RoomState, error) {
626626
}
627627

628628
func (h *WSHandler) handleUpdateName(roomID string, msg *models.WSMessage, participantID string) {
629+
// Helper to send error to the client
630+
sendError := func(message string) {
631+
client := h.hub.GetClient(roomID, participantID)
632+
if client != nil {
633+
h.hub.SendToClient(client, &models.WSMessage{
634+
Type: models.MsgTypeError,
635+
Payload: map[string]any{
636+
"message": message,
637+
"action": "update_name",
638+
},
639+
})
640+
}
641+
}
642+
629643
if participantID == "" {
630644
log.Printf("Update name rejected: no participant ID")
645+
sendError("Could not identify participant")
631646
return
632647
}
633648

634649
// Extract new name from payload
635650
payload, ok := msg.Payload.(map[string]any)
636651
if !ok {
637652
log.Printf("Invalid update name payload format")
653+
sendError("Invalid request format")
638654
return
639655
}
640656

641657
newName, ok := payload["name"].(string)
642658
if !ok {
643659
log.Printf("Invalid name value type")
660+
sendError("Invalid name format")
644661
return
645662
}
646663

647664
// Validate and sanitize name
648665
sanitizedName, err := security.ValidateParticipantName(newName)
649666
if err != nil {
650667
log.Printf("Invalid participant name: %v", err)
668+
sendError(err.Error())
651669
return
652670
}
653671
newName = sanitizedName
654672

655673
// Update participant name in database
656674
if err := h.roomManager.UpdateParticipantName(participantID, newName); err != nil {
657675
log.Printf("Failed to update participant name: %v", err)
676+
sendError("Failed to update name. Please try again.")
658677
return
659678
}
660679

@@ -671,36 +690,55 @@ func (h *WSHandler) handleUpdateName(roomID string, msg *models.WSMessage, parti
671690
}
672691

673692
func (h *WSHandler) handleUpdateRoomName(roomID string, msg *models.WSMessage, participantID string) {
693+
// Helper to send error to the client
694+
sendError := func(message string) {
695+
client := h.hub.GetClient(roomID, participantID)
696+
if client != nil {
697+
h.hub.SendToClient(client, &models.WSMessage{
698+
Type: models.MsgTypeError,
699+
Payload: map[string]any{
700+
"message": message,
701+
"action": "update_room_name",
702+
},
703+
})
704+
}
705+
}
706+
674707
// Verify participant is the room creator
675708
if !h.roomManager.IsRoomCreator(roomID, participantID) {
676709
log.Printf("Update room name rejected: participant %s is not room creator", participantID)
710+
sendError("Only the room creator can change the room name")
677711
return
678712
}
679713

680714
// Extract new name from payload
681715
payload, ok := msg.Payload.(map[string]any)
682716
if !ok {
683717
log.Printf("Invalid update room name payload format")
718+
sendError("Invalid request format")
684719
return
685720
}
686721

687722
newName, ok := payload["name"].(string)
688723
if !ok {
689724
log.Printf("Invalid room name value type")
725+
sendError("Invalid room name format")
690726
return
691727
}
692728

693729
// Validate and sanitize room name
694730
sanitizedName, err := security.ValidateRoomName(newName)
695731
if err != nil {
696732
log.Printf("Invalid room name: %v", err)
733+
sendError(err.Error())
697734
return
698735
}
699736
newName = sanitizedName
700737

701738
// Update room name in database
702739
if err := h.roomManager.UpdateRoomName(roomID, newName); err != nil {
703740
log.Printf("Failed to update room name: %v", err)
741+
sendError("Failed to update room name. Please try again.")
704742
return
705743
}
706744

internal/models/message.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,5 @@ const (
3131
MsgTypeNameUpdated = "name_updated"
3232
MsgTypeRoomNameUpdated = "room_name_updated"
3333
MsgTypeConfigUpdated = "config_updated"
34+
MsgTypeError = "error" // Error message to client
3435
)

internal/security/validators.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,13 @@ var (
2020
pocketbaseIDRegex = regexp.MustCompile(`^[a-zA-Z0-9]{15}$`)
2121
// UUID validation regex (for potential future use)
2222
uuidRegex = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)
23-
// Name validation regex (alphanumeric, spaces, common punctuation)
24-
nameRegex = regexp.MustCompile(`^[a-zA-Z0-9\s\-_.]+$`)
23+
// Name validation regex - Unicode letters, digits, spaces, apostrophes, hyphens, underscores, dots
24+
// \p{L} matches any Unicode letter (includes accented characters)
25+
// \p{N} matches any Unicode number
26+
// ' allows apostrophes (for French and English possessives)
27+
nameRegex = regexp.MustCompile(`^[\p{L}\p{N}\s'\-_.]+$`)
28+
// Dangerous characters that could be used for injection attacks
29+
dangerousCharsRegex = regexp.MustCompile(`[<>{}[\]\\;|&$()` + "`" + `]`)
2530
)
2631

2732
// ValidateUUID validates that a string is a valid PocketBase ID or UUID format
@@ -69,9 +74,14 @@ func ValidateName(name string, maxLen int) (string, error) {
6974
return "", fmt.Errorf("name too long (max %d characters)", maxLen)
7075
}
7176

72-
// Check for invalid characters
77+
// Check for invalid characters (must match allowed character set)
7378
if !nameRegex.MatchString(name) {
74-
return "", fmt.Errorf("name contains invalid characters (allowed: letters, numbers, spaces, hyphens, underscores, dots)")
79+
return "", fmt.Errorf("name contains invalid characters (allowed: letters, numbers, spaces, apostrophes, hyphens, underscores, dots)")
80+
}
81+
82+
// Check for dangerous characters that could be used for injection
83+
if dangerousCharsRegex.MatchString(name) {
84+
return "", fmt.Errorf("name contains potentially dangerous characters")
7585
}
7686

7787
// Check for control characters (belt-and-suspenders with regex)

internal/services/hub.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,20 @@ func (h *Hub) GetRoomCount() int {
250250
func (h *Hub) SetMessageHandler(handler MessageHandler) {
251251
h.messageHandler = handler
252252
}
253+
254+
// GetClient finds a client in a room by participant ID
255+
func (h *Hub) GetClient(roomID string, participantID string) *Client {
256+
value, ok := h.rooms.Load(roomID)
257+
if !ok {
258+
return nil
259+
}
260+
261+
clients := value.(map[*Client]bool)
262+
for client := range clients {
263+
if client.participantID == participantID {
264+
return client
265+
}
266+
}
267+
268+
return nil
269+
}

tests/unit/security/validators_test.go

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,28 @@ func TestValidateRoomName(t *testing.T) {
6262
{"valid with trailing space", "Sprint Planning ", "Sprint Planning", false},
6363
{"minimum length", "S", "S", false},
6464
{"maximum length", strings.Repeat("a", 100), strings.Repeat("a", 100), false},
65+
// French names with accents and apostrophes
66+
{"french with apostrophe", "L'équipe Sprint", "L'équipe Sprint", false},
67+
{"french with multiple accents", "Réunion d'été", "Réunion d'été", false},
68+
{"english possessive", "Bob's Team", "Bob's Team", false},
69+
{"simple accent", "Café Planning", "Café Planning", false},
70+
{"multiple apostrophes", "L'équipe d'Alice", "L'équipe d'Alice", false},
71+
{"german umlauts", "Müller's Planung", "Müller's Planung", false},
72+
{"spanish accents", "Reunión España", "Reunión España", false},
6573

6674
// Invalid cases
6775
{"empty", "", "", true},
6876
{"whitespace only", " ", "", true},
6977
{"too long", strings.Repeat("a", 101), "", true},
7078
{"xss attempt", "<script>alert('xss')</script>", "", true},
7179
{"sql injection", "'; DROP TABLE rooms--", "", true},
72-
{"special chars", "Sprint @ Planning", "", true},
80+
{"special chars @", "Sprint @ Planning", "", true},
7381
{"control characters", "Sprint\nPlanning", "", true},
7482
{"unicode emoji", "Sprint 🚀", "", true},
83+
{"brackets", "Room[1]", "", true},
84+
{"pipe", "Room|Test", "", true},
85+
{"ampersand", "Room & Planning", "", true},
86+
{"dollar sign", "Room$123", "", true},
7587
}
7688

7789
for _, tt := range tests {
@@ -103,6 +115,15 @@ func TestValidateParticipantName(t *testing.T) {
103115
{"minimum length", "A", "A", false},
104116
{"maximum length", strings.Repeat("a", 50), strings.Repeat("a", 50), false},
105117
{"trim whitespace", " Alice ", "Alice", false},
118+
// French and international names
119+
{"french name with accent", "François", "François", false},
120+
{"french name with apostrophe", "D'Artagnan", "D'Artagnan", false},
121+
{"german name", "Müller", "Müller", false},
122+
{"spanish name", "José García", "José García", false},
123+
{"portuguese name", "João", "João", false},
124+
{"scandinavian name", "Søren", "Søren", false},
125+
{"polish name", "Łukasz", "Łukasz", false},
126+
{"multiple accents", "Stéphane Bücher", "Stéphane Bücher", false},
106127

107128
// Invalid cases
108129
{"empty", "", "", true},
@@ -112,8 +133,11 @@ func TestValidateParticipantName(t *testing.T) {
112133
{"img onerror", "<img src=x onerror=alert('xss')>", "", true},
113134
{"event handler", "<div onload=alert('xss')>Alice</div>", "", true},
114135
{"sql injection", "'; DROP TABLE--", "", true},
115-
{"special chars", "Alice@Bob", "", true},
136+
{"special chars @", "Alice@Bob", "", true},
116137
{"control chars", "Alice\x00Bob", "", true},
138+
{"brackets", "Alice[0]", "", true},
139+
{"pipe", "Alice|Bob", "", true},
140+
{"ampersand", "Alice&Bob", "", true},
117141
}
118142

119143
for _, tt := range tests {

web/templates/error_display.templ

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package templates
2+
3+
// ErrorDisplay renders an error message alert
4+
templ ErrorDisplay(message string) {
5+
<div id="error-display" class="mb-6 p-4 bg-red-50 border border-red-200 rounded-xl">
6+
<div class="flex items-start gap-3">
7+
<span class="text-red-500 text-xl">⚠️</span>
8+
<div class="flex-1">
9+
<p class="text-sm font-medium text-red-900">{ message }</p>
10+
</div>
11+
</div>
12+
</div>
13+
}

web/templates/home.templ

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,10 @@ templ roomCreationForm(voteTemplates []services.TemplateInfo, errorParam string)
103103
</div>
104104
</div>
105105
}
106+
<!-- Error Container -->
107+
<div id="form-errors"></div>
106108
<!-- Form Fields -->
107-
<form method="POST" action="/room" x-data="roomForm()" class="space-y-6">
109+
<form method="POST" action="/room" x-data="roomForm()" hx-post="/room" hx-target="#form-errors" hx-swap="innerHTML" class="space-y-6">
108110
<!-- Room Name Input -->
109111
<div>
110112
<label for="name" class="block text-sm font-semibold text-slate-700 mb-2">

web/templates/join_modal.templ

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ templ JoinModal(roomID string) {
55
<div class="elevated-card bg-gradient-to-br from-white to-slate-50 rounded-2xl shadow-2xl max-w-md w-full mx-4 p-8" x-show="show">
66
<h2 class="text-2xl font-bold text-slate-800 mb-2">Join Room</h2>
77
<p class="text-sm text-slate-600 mb-6">Enter your details to participate</p>
8-
<form hx-post={"/room/" + roomID + "/join"} hx-swap="outerHTML" class="space-y-6">
8+
<!-- Error Container -->
9+
<div id="join-form-errors"></div>
10+
<form hx-post={"/room/" + roomID + "/join"} hx-target="#join-form-errors" hx-swap="innerHTML" class="space-y-6">
911
<div>
1012
<label for="name" class="block text-sm font-semibold text-slate-700 mb-2">
1113
Your Name

web/templates/room.templ

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,21 @@ package templates
22

33
import (
44
"fmt"
5+
"strings"
56
"github.com/damione1/planning-poker/internal/models"
67
)
78

9+
// escapeJS escapes a string for safe use in JavaScript string literals
10+
func escapeJS(s string) string {
11+
s = strings.ReplaceAll(s, "\\", "\\\\") // Escape backslashes first
12+
s = strings.ReplaceAll(s, "'", "\\'") // Escape single quotes
13+
s = strings.ReplaceAll(s, "\"", "\\\"") // Escape double quotes
14+
s = strings.ReplaceAll(s, "\n", "\\n") // Escape newlines
15+
s = strings.ReplaceAll(s, "\r", "\\r") // Escape carriage returns
16+
s = strings.ReplaceAll(s, "\t", "\\t") // Escape tabs
17+
return s
18+
}
19+
820
templ Room(room *models.Room, participant *models.Participant, isCreator bool) {
921
@Base(room.Name) {
1022
<div class="bg-white rounded-lg shadow-lg p-6 min-h-[600px]" x-data="roomStateManager()" hx-ext="ws" ws-connect={"/ws/" + room.ID}>
@@ -116,7 +128,7 @@ templ Room(room *models.Room, participant *models.Participant, isCreator bool) {
116128
<div class="flex items-center gap-3">
117129
if participant != nil {
118130
<div
119-
x-data={ fmt.Sprintf("{ editing: false, newName: '', currentName: '%s', participantId: '%s' }", participant.Name, participant.ID) }
131+
x-data={ fmt.Sprintf("{ editing: false, newName: '', currentName: '%s', participantId: '%s' }", escapeJS(participant.Name), participant.ID) }
120132
@name-updated.window="if ($event.detail.participantId === participantId) { currentName = $event.detail.name }"
121133
class="flex items-center gap-3 px-4 py-2 bg-gradient-to-br from-primary-50 to-success-50 border border-primary-200 rounded-xl shadow-sm"
122134
>

0 commit comments

Comments
 (0)