Skip to content

Commit 9e7a61c

Browse files
authored
Fix report issue upload attachment size check (#8814)
* Fix report issue uploads when attachment size metadata is stale * code review updates
1 parent 168e1ae commit 9e7a61c

2 files changed

Lines changed: 31 additions & 20 deletions

File tree

lantern-core/utils/reportissue/attachments.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,11 @@ var attachmentTypesByExtension = map[string]string{
3636
}
3737

3838
type AttachmentMetadata struct {
39-
Name string `json:"name"`
40-
Path string `json:"path"`
41-
MimeType string `json:"mimeType"`
42-
SizeBytes int64 `json:"sizeBytes"`
39+
Name string `json:"name"`
40+
Path string `json:"path"`
41+
MimeType string `json:"mimeType"`
42+
// Client-provided size is a stale-prone hint; validate current file size instead.
43+
SizeBytes int64 `json:"sizeBytes"`
4344
}
4445

4546
// LoadAttachments decodes, validates, and reads screenshot attachments for issue reports.
@@ -118,9 +119,6 @@ func prepareAttachment(attachment AttachmentMetadata) (preparedAttachment, error
118119
if path == "" {
119120
return preparedAttachment{}, fmt.Errorf("attachment %q path is required", name)
120121
}
121-
if attachment.SizeBytes < 0 {
122-
return preparedAttachment{}, fmt.Errorf("attachment %q size must be non-negative", name)
123-
}
124122

125123
attachmentType := resolveDeclaredAttachmentType(attachment.MimeType, name)
126124
if !isAllowedAttachmentType(attachmentType) {
@@ -134,8 +132,8 @@ func prepareAttachment(attachment AttachmentMetadata) (preparedAttachment, error
134132
if info.IsDir() {
135133
return preparedAttachment{}, fmt.Errorf("attachment %q must be a file", name)
136134
}
137-
if info.Size() != attachment.SizeBytes {
138-
return preparedAttachment{}, fmt.Errorf("attachment %q changed on disk before upload", name)
135+
if info.Size() == 0 {
136+
return preparedAttachment{}, fmt.Errorf("attachment %q must not be empty", name)
139137
}
140138

141139
return preparedAttachment{

lantern-core/utils/reportissue/attachments_test.go

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -92,28 +92,41 @@ func TestBuildAttachmentReadsHEICData(t *testing.T) {
9292
}
9393
}
9494

95-
func TestBuildAttachmentRejectsChangedFiles(t *testing.T) {
95+
func TestBuildAttachmentAllowsCachedSizeMismatch(t *testing.T) {
9696
tmpDir := t.TempDir()
9797
path := filepath.Join(tmpDir, "vpn_error.png")
98-
if err := os.WriteFile(path, testPNGData, 0o644); err != nil {
98+
data := append([]byte{}, testPNGData...)
99+
data = append(data, 0x00, 0x01, 0x02)
100+
if err := os.WriteFile(path, data, 0o644); err != nil {
99101
t.Fatalf("write test attachment: %v", err)
100102
}
101103

102-
_, err := validateMetadata([]AttachmentMetadata{{
104+
raw, err := json.Marshal([]AttachmentMetadata{{
103105
Name: "vpn_error.png",
104106
Path: path,
105107
MimeType: "image/png",
106-
SizeBytes: 999,
108+
SizeBytes: -1,
107109
}})
108-
if err == nil {
109-
t.Fatalf("expected size mismatch to fail")
110+
if err != nil {
111+
t.Fatalf("marshal attachments: %v", err)
112+
}
113+
114+
attachments, err := LoadAttachments(string(raw))
115+
if err != nil {
116+
t.Fatalf("LoadAttachments returned error: %v", err)
117+
}
118+
if len(attachments) != 1 {
119+
t.Fatalf("expected 1 attachment, got %d", len(attachments))
120+
}
121+
if string(attachments[0].Data) != string(data) {
122+
t.Fatalf("attachment data mismatch: got %q want %q", string(attachments[0].Data), string(data))
110123
}
111124
}
112125

113-
func TestBuildAttachmentRejectsZeroSizeBypass(t *testing.T) {
126+
func TestBuildAttachmentRejectsEmptyFiles(t *testing.T) {
114127
tmpDir := t.TempDir()
115128
path := filepath.Join(tmpDir, "vpn_error.png")
116-
if err := os.WriteFile(path, testPNGData, 0o644); err != nil {
129+
if err := os.WriteFile(path, nil, 0o644); err != nil {
117130
t.Fatalf("write test attachment: %v", err)
118131
}
119132

@@ -123,8 +136,8 @@ func TestBuildAttachmentRejectsZeroSizeBypass(t *testing.T) {
123136
MimeType: "image/png",
124137
SizeBytes: 0,
125138
}})
126-
if err == nil {
127-
t.Fatalf("expected missing exact size to fail")
139+
if err == nil || !strings.Contains(err.Error(), "must not be empty") {
140+
t.Fatalf("expected empty attachment error, got %v", err)
128141
}
129142
}
130143

@@ -169,7 +182,7 @@ func TestLoadAttachmentsRejectsTotalSizeOverLimit(t *testing.T) {
169182
Name: "huge.png",
170183
Path: path,
171184
MimeType: "image/png",
172-
SizeBytes: maxAttachmentBytes + 1,
185+
SizeBytes: int64(len(testPNGData)),
173186
}})
174187
if err != nil {
175188
t.Fatalf("marshal attachments: %v", err)

0 commit comments

Comments
 (0)