-
Notifications
You must be signed in to change notification settings - Fork 41
Description
Problem Statement
The POST /api/contacts endpoint currently accepts any valid JSON payload without proper validation, leading to:
- Data quality issues - contacts with empty or malformed emails stored in database
- Inconsistent state - records without any identifying information (no name, no email)
- Security concerns - potential injection attacks or buffer overflow with unbounded field lengths
- Poor API experience - clients don't receive clear feedback on invalid input
This creates technical debt and makes the API unreliable for production use.
Current Behavior
The endpoint accepts requests like these without error:
// Empty email - should fail
{
"email": "",
"first_name": ""
}
// Invalid email format - should fail
{
"email": "not-an-email",
"first_name": "John"
}
// No identifying information - should fail
{
"phone": "123"
}
// Unbounded field length - should fail
{
"email": "a@b.c" + ("x" * 1000000),
"first_name": "John"
}Currently returns 201 Created for all of these cases.
Proposed Solution
Add server-side validation to the contact creation handler that enforces:
Required Fields
- Email: Must be provided and match valid email format (RFC 5322 compliant)
- Name:
first_nameandlast_namemust be non-empty
Optional Field Validation (when provided)
- Email length: Maximum 255 characters
- Phone number: Valid international format if provided (E.164 or local format)
- Name fields: Maximum reasonable length (e.g., 100 characters each)
Error Response Format
Return 400 Bad Request with structured error details:
{
"error": "validation failed",
"details": [
"email is required and must be valid",
"first_name isrequired",
"last_name is required"
]
}Proposed Implementation
Location
internal/contact/handler.go:19 - There's already a TODO comment marking this spot
Approach
// internal/contact/validator.go (new file)
type ContactValidationError struct {
Field string
Message string
}
func ValidateCreateContactRequest(req *CreateContactRequest) []ContactValidationError {
var errors []ContactValidationError
// Email validation
if req.Email == "" {
errors = append(errors, ContactValidationError{
Field: "email",
Message: "email is required",
})
} else if !isValidEmail(req.Email) {
errors = append(errors, ContactValidationError{
Field: "email",
Message: "email must be valid format",
})
} else if len(req.Email) > 255 {
errors = append(errors, ContactValidationError{
Field: "email",
Message: "email must not exceed 255 characters",
})
}
// Name validation
if strings.TrimSpace(req.FirstName) == "" && strings.TrimSpace(req.LastName) == "" {
errors = append(errors, ContactValidationError{
Field: "name",
Message: "at least one of first_name or last_name is required",
})
}
// Phone validation (optional)
if req.Phone != "" && !isValidPhone(req.Phone) {
errors = append(errors, ContactValidationError{
Field: "phone",
Message: "phone must be valid format",
})
}
return errors
}Integration in Handler
// internal/contact/handler.go
func (h *Handler) CreateContact(w http.ResponseWriter, r *http.Request) {
var req CreateContactRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
respondError(w, http.StatusBadRequest, "invalid JSON")
return
}
// Add validation here (line 19)
if errs := ValidateCreateContactRequest(&req); len(errs) > 0 {
respondValidationError(w, errs)
return
}
// Continue with business logic...
}Implementation Checklist
- Create
internal/contact/validator.gowith validation functions - Implement email format validation (regex or stdlib)
- Implement phone number validation (consider library like
github.com/nyaruka/phonenumbers) - Add field length checks
- Update handler to call validation before processing
- Add unit tests for validation logic
- Add integration tests for endpoint behavior
- Update API documentation with validation rules
Testing Strategy
Unit Tests (validator_test.go)
func TestValidateCreateContactRequest(t *testing.T) {
tests := []struct {
name string
req CreateContactRequest
wantErr bool
errMsgs []string
}{
{
name: "valid contact",
req: CreateContactRequest{
Email: "test@example.com",
FirstName: "John",
},
wantErr: false,
},
{
name: "empty email",
req: CreateContactRequest{
FirstName: "John",
},
wantErr: true,
errMsgs: []string{"email is required"},
},
// ... more test cases
}
}Integration Tests
- Test endpoint returns 400 for invalid input
- Test endpoint returns 201 for valid input
- Test error message format
Questions / Discussion Points
-
Email validation strictness: Should we use simple regex or full RFC 5322 compliance?
- Simple regex is faster but may reject some valid emails
- RFC 5322 is more accurate but complex
-
Phone validation: Should we support international formats only, or allow flexible local formats?
- Suggestion: Accept common formats, don't be too strict initially
-
Field length limits: Are 255 chars for email and 100 for names reasonable?
- Based on common database varchar limits
-
Validation library: Should we use a validation library (e.g.,
go-playground/validator) or custom code?- Custom gives us control but more maintenance
- Library is battle-tested but adds dependency
Benefits
- ✅ Better data quality - ensures database contains valid, usable contacts
- ✅ Clear API contract - clients know exactly what's expected
- ✅ Easier debugging - validation errors caught early with clear messages
- ✅ Security - prevents potential exploits via malformed input
- ✅ Good first issue - isolated scope, touches core concepts (validation, HTTP errors)
Additional Context
Good First Issue: This is an excellent starting point for new contributors because:
- Clear, well-defined scope
- Touches important concepts (validation, error handling, testing)
- Doesn't require deep knowledge of business logic
- Has visible impact on API quality
- Low risk of breaking existing functionality
Files to familiarize yourself with:
internal/contact/handler.go- Where validation will be addedinternal/contact/types.go- Request/response structuresinternal/contact/handler_test.go- Existing test patterns