Conversation
…basic file type check
|
Will run through E2E testing tomorrow to confirm everything is working as intended |
PR Summary
Overall, this PR focuses on enhancing the messaging capabilities of our service, enabling users to send multimedia messages, along with maintaining the optimum performance by controlling the size of attachments. |
Greptile SummaryThis PR adds MMS (Multimedia Messaging Service) support to the httpSMS application, enabling users to send messages with attachments like images and videos. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Web/Discord
participant API
participant Validator
participant Android
participant AttachmentServer
participant MMS
User->>Web/Discord: Send message with attachment URLs
Web/Discord->>API: POST /v1/messages/send
API->>Validator: Validate attachment URLs
Validator->>AttachmentServer: HEAD request (check size)
AttachmentServer-->>Validator: Content-Length header
Validator->>Validator: Cache result (24h TTL)
Validator-->>API: Validation result
API->>API: Store message with attachments
API->>Android: Push notification via FCM
Android->>AttachmentServer: Download attachments
AttachmentServer-->>Android: File data
Android->>Android: Build MMS PDU with attachments
Android->>Android: Write PDU to cache file
Android->>MMS: sendMultimediaMessage(pduUri)
MMS-->>Android: Broadcast (success/failure)
Android->>Android: Cleanup PDU file
Android->>API: Report status
Last reviewed commit: 47dde30 |
There was a problem hiding this comment.
Pull request overview
Adds end-to-end MMS support by introducing “attachments” across the API, web UI, bulk upload templates, and Android client message sending pipeline (including MMS PDU composition and dispatch).
Changes:
- Extend message send/bulk send APIs (and Discord slash command) to accept attachment URLs with content types, and persist them on
entities.Message. - Update bulk message CSV/XLSX templates + bulk-file parsing/validation to support
AttachmentURLs(optional). - Add web UI fields for composing MMS and displaying message attachments; add Android MMS composition/sending + attachment download and cache cleanup.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| web/static/templates/httpsms-bulk.xlsx | Updates Excel bulk-send template to include optional attachment URLs. |
| web/static/templates/httpsms-bulk.csv | Updates CSV bulk-send template to include optional send time + attachment URLs. |
| web/pages/threads/_id/index.vue | Renders per-message attachment gallery in thread view. |
| web/pages/messages/index.vue | Adds attachment URL input and includes attachments in send payload. |
| web/models/message.ts | Adds attachments to the web Message model. |
| web/components/MessageThread.vue | Minor UI/asset changes in thread list component (icon import). |
| api/pkg/validators/validator.go | Adds URL reachability/size validation with caching for attachments. |
| api/pkg/validators/message_handler_validator.go | Validates attachments on single + bulk send endpoints. |
| api/pkg/validators/bulk_message_handler_validator.go | Parses/validates attachment URLs from bulk CSV/XLSX uploads. |
| api/pkg/services/message_service.go | Propagates attachments through send flow and persistence. |
| api/pkg/services/discord_service.go | Adds optional attachment_urls option to Discord slash command. |
| api/pkg/requests/message_send_request.go | Adds attachments field to send request and params conversion. |
| api/pkg/requests/message_bulk_send_request.go | Adds attachments field to bulk-send request and params conversion. |
| api/pkg/requests/bulk_message_request.go | Adds AttachmentURLs CSV column and maps to attachments on send. |
| api/pkg/handlers/discord_handler.go | Parses attachment_urls and displays attachment URLs in Discord response embed. |
| api/pkg/events/message_api_sent_event.go | Includes attachments in the emitted “message.api.sent” event payload. |
| api/pkg/entities/message.go | Adds persisted Attachments JSON field + content-type helper. |
| api/pkg/di/container.go | Wires app cache into validators for attachment URL validation caching. |
| android/app/src/main/res/xml/file_paths.xml | Adds FileProvider cache path for MMS PDU/attachments. |
| android/app/src/main/java/com/httpsms/SmsManagerService.kt | Adds sendMultimediaMessage wrapper. |
| android/app/src/main/java/com/httpsms/SentReceiver.kt | Cleans up cached MMS PDU file on send completion. |
| android/app/src/main/java/com/httpsms/Models.kt | Adds attachment model + exposes message attachments in API model. |
| android/app/src/main/java/com/httpsms/HttpSmsApiService.kt | Downloads attachments with a size limit into cache for MMS composition. |
| android/app/src/main/java/com/httpsms/FirebaseMessagingService.kt | Detects MMS vs SMS; composes MMS PDU with downloaded media and dispatches via SmsManager. |
| android/app/src/main/AndroidManifest.xml | Registers FileProvider for sharing MMS PDU to platform MMS service. |
| android/app/build.gradle | Adds SMS/MMS library dependency used for PDU composition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if resp.StatusCode < 200 || resp.StatusCode >= 400 { | ||
| errMsg := fmt.Sprintf("url returned an error status code: %d", resp.StatusCode) | ||
| saveToCache(ctx, c, cacheKey, errMsg) | ||
| return fmt.Errorf(errMsg) | ||
| } | ||
|
|
||
| const maxSizeBytes = 1.5 * 1024 * 1024 | ||
|
|
||
| if resp.ContentLength > int64(maxSizeBytes) { | ||
| errMsg := fmt.Sprintf("file size (%.2f MB) exceeds the 1.5 MB carrier limit", float64(resp.ContentLength)/(1024*1024)) | ||
| saveToCache(ctx, c, cacheKey, errMsg) | ||
| return fmt.Errorf(errMsg) | ||
| } | ||
|
|
||
| saveToCache(ctx, c, cacheKey, "valid") |
There was a problem hiding this comment.
The size validation relies on resp.ContentLength from a HEAD response. For many servers this is -1/unknown (or HEAD is unsupported), which will incorrectly treat potentially huge attachments as valid. Consider treating unknown length as invalid or falling back to a bounded GET/Range request so the 1.5MB limit is actually enforced.
| import { | ||
| mdiPlus, | ||
| mdiDownload, | ||
| mdiCheckAll, | ||
| mdiCheck, | ||
| mdiAlert, | ||
| mdiAccount, | ||
| mdiPaperclip, | ||
| } from '@mdi/js' | ||
|
|
||
| @Component | ||
| export default class MessageThread extends Vue { | ||
| mdiPlus = mdiPlus | ||
| mdiDownload = mdiDownload | ||
| mdiAccount = mdiAccount | ||
| mdiAlert = mdiAlert | ||
| mdiCheck = mdiCheck | ||
| mdiCheckAll = mdiCheckAll | ||
| mdiPaperclip = mdiPaperclip |
There was a problem hiding this comment.
mdiPaperclip is imported and assigned on the component instance but never used in the template/script. If this isn’t needed, remove it to avoid unused-symbol warnings (or add the intended attachment indicator to the UI).
| val maxSizeBytes = 1.5 * 1024 * 1024 // most (modern?) carriers have a 2MB limit, so targetting 1.5MB should be safe | ||
| val contentLength = response.body?.contentLength() ?: -1L | ||
| if (contentLength > maxSizeBytes) { | ||
| Timber.e("Attachment is too large ($contentLength bytes).") | ||
| response.close() | ||
| return null | ||
| } |
There was a problem hiding this comment.
maxSizeBytes is a Double, but contentLength is a Long; if (contentLength > maxSizeBytes) won’t compile in Kotlin without an explicit conversion. Define maxSizeBytes as a Long (or convert contentLength to Double) so the comparison is type-correct.
| func validateAttachmentURL(ctx context.Context, c cache.Cache, attachmentURL string) error { | ||
| cacheKey := "mms-url-validation:" + attachmentURL | ||
|
|
||
| if cachedVal, err := c.Get(ctx, cacheKey); err == nil { | ||
| if cachedVal == "valid" { | ||
| return nil | ||
| } | ||
| return fmt.Errorf(cachedVal) | ||
| } | ||
|
|
||
| client := &http.Client{ | ||
| Timeout: 5 * time.Second, | ||
| } | ||
|
|
||
| req, err := http.NewRequest(http.MethodHead, attachmentURL, nil) | ||
| if err != nil { | ||
| errMsg := fmt.Sprintf("invalid url format") | ||
| saveToCache(ctx, c, cacheKey, errMsg) | ||
| return fmt.Errorf(errMsg) | ||
| } | ||
|
|
||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| errMsg := fmt.Sprintf("could not reach the url") | ||
| saveToCache(ctx, c, cacheKey, errMsg) | ||
| return fmt.Errorf(errMsg) |
There was a problem hiding this comment.
validateAttachmentURL makes a server-side HTTP request to a user-supplied URL. As-is this enables SSRF (including redirects/DNS rebinding) to internal/metadata endpoints. Consider blocking private/loopback/link-local ranges after DNS resolution and disabling/fencing redirects (or validating final destination) before performing any request.
| client := &http.Client{ | ||
| Timeout: 5 * time.Second, | ||
| } | ||
|
|
||
| req, err := http.NewRequest(http.MethodHead, attachmentURL, nil) | ||
| if err != nil { | ||
| errMsg := fmt.Sprintf("invalid url format") | ||
| saveToCache(ctx, c, cacheKey, errMsg) | ||
| return fmt.Errorf(errMsg) | ||
| } | ||
|
|
||
| resp, err := client.Do(req) | ||
| if err != nil { |
There was a problem hiding this comment.
validateAttachmentURL creates a new http.Client and does a blocking request per attachment; with up to 10 attachments this can add ~50s to a single API call (and much more in bulk upload). Consider reusing a shared client and performing the request with http.NewRequestWithContext(ctx, ...) so cancellations/timeouts propagate.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Implementation for: #262