Skip to content

Commit 556234b

Browse files
AchoArnoldCopilot
andcommitted
fix: address second round of PR review feedback
- Make content field optional when attachments are present (MMS-only messages) - SanitizeFilename: strip all non-alphanumeric chars, replace spaces with dashes - Add 3MB total attachment size limit in validation - Update tests for new SanitizeFilename behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a35f8a3 commit 556234b

3 files changed

Lines changed: 31 additions & 12 deletions

File tree

api/pkg/repositories/attachment_repository.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,16 @@ func ContentTypeFromExtension(ext string) string {
8181
// Returns "attachment-{index}" if the sanitized name is empty.
8282
func SanitizeFilename(name string, index int) string {
8383
name = strings.TrimSuffix(name, filepath.Ext(name))
84-
name = strings.ReplaceAll(name, "/", "")
85-
name = strings.ReplaceAll(name, "\\", "")
86-
name = strings.ReplaceAll(name, "..", "")
87-
name = strings.TrimSpace(name)
84+
85+
var builder strings.Builder
86+
for _, r := range strings.ToLower(name) {
87+
if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') {
88+
builder.WriteRune(r)
89+
} else if r == ' ' {
90+
builder.WriteRune('-')
91+
}
92+
}
93+
name = strings.Trim(builder.String(), "-")
8894

8995
if name == "" {
9096
return fmt.Sprintf("attachment-%d", index)

api/pkg/repositories/attachment_repository_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,14 @@ func TestSanitizeFilename(t *testing.T) {
4343
{"photo.jpg", 0, "photo"},
4444
{"../../etc/passwd", 0, "etcpasswd"},
4545
{"hello/world\\test", 0, "helloworldtest"},
46-
{"normal_file", 0, "normal_file"},
46+
{"normal_file", 0, "normalfile"},
4747
{"", 0, "attachment-0"},
4848
{" ", 0, "attachment-0"},
4949
{"...", 1, "attachment-1"},
50+
{"My Photo", 0, "my-photo"},
51+
{"file name with spaces.png", 0, "file-name-with-spaces"},
52+
{"UPPER_CASE", 0, "uppercase"},
53+
{"special!@#chars", 0, "specialchars"},
5054
}
5155
for _, tt := range tests {
5256
t.Run(tt.name, func(t *testing.T) {

api/pkg/validators/message_handler_validator.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ func NewMessageHandlerValidator(
4848
}
4949

5050
const (
51-
maxAttachmentCount = 10
52-
maxAttachmentSize = (3 * 1024 * 1024) / 2 // 1.5 MB
51+
maxAttachmentCount = 10
52+
maxAttachmentSize = (3 * 1024 * 1024) / 2 // 1.5 MB per attachment
53+
maxTotalAttachmentSize = 3 * 1024 * 1024 // 3 MB total
5354
)
5455

5556
// ValidateMessageReceive validates the requests.MessageReceive request
@@ -64,11 +65,12 @@ func (validator MessageHandlerValidator) ValidateMessageReceive(_ context.Contex
6465
"from": []string{
6566
"required",
6667
},
67-
"content": []string{
68-
"required",
69-
"min:1",
70-
"max:2048",
71-
},
68+
"content": func() []string {
69+
if len(request.Attachments) > 0 {
70+
return []string{"max:2048"}
71+
}
72+
return []string{"required", "min:1", "max:2048"}
73+
}(),
7274
"sim": []string{
7375
"required",
7476
"in:" + strings.Join([]string{
@@ -102,6 +104,7 @@ func (validator MessageHandlerValidator) validateAttachments(attachments []reque
102104
return errors
103105
}
104106

107+
totalSize := 0
105108
for i, attachment := range attachments {
106109
if !allowedTypes[attachment.ContentType] {
107110
errors.Add("attachments", fmt.Sprintf("attachment [%d] has unsupported content type [%s]", i, attachment.ContentType))
@@ -117,6 +120,12 @@ func (validator MessageHandlerValidator) validateAttachments(attachments []reque
117120
if len(decoded) > maxAttachmentSize {
118121
errors.Add("attachments", fmt.Sprintf("attachment [%d] size [%d] exceeds maximum of [%d] bytes", i, len(decoded), maxAttachmentSize))
119122
}
123+
124+
totalSize += len(decoded)
125+
}
126+
127+
if totalSize > maxTotalAttachmentSize {
128+
errors.Add("attachments", fmt.Sprintf("total attachment size [%d] exceeds maximum of [%d] bytes", totalSize, maxTotalAttachmentSize))
120129
}
121130

122131
return errors

0 commit comments

Comments
 (0)