Skip to content

Add fix for MMS attachments#839

Merged
AchoArnold merged 4 commits intomainfrom
feature/mms-attachment-refactor
Mar 23, 2026
Merged

Add fix for MMS attachments#839
AchoArnold merged 4 commits intomainfrom
feature/mms-attachment-refactor

Conversation

@AchoArnold
Copy link
Copy Markdown
Member

No description provided.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 23, 2026

Greptile Summary

This PR simplifies the MMS attachment model across the full stack — replacing the {content_type, url} struct with a plain []string of URLs. The server-side content-type inference (which was extension-based and fragile) is removed; instead, the Android client now reads the actual Content-Type header from the HTTP response when downloading each attachment, and uses that MIME type when composing the MMS PDU. This is a cleaner design that avoids silent type mismatches between what the API guessed and what the file actually is.

Key changes:

  • Android: Message.attachments is now List<String>; downloadAttachment returns Pair<File?, MediaType?> so the real content type flows into the PDU; extractFileName derives a proper filename from the URL path or MIME type.
  • API/Go: Message.Attachments changed to pq.StringArray (text[]); MessageAttachment struct and GetAttachmentContentType helper removed; attachment URL validation moved to a new multipleAttachmentURLRule declarative rule.
  • Web: Attachment submission simplified to pass URL strings directly; attachment display changed from inline image rendering to plain hyperlinks.

Two bugs need to be addressed before merging:

  • HttpSmsApiService.kt lines 199–219: The null guard for response.body was removed but the body is still dereferenced directly (body.contentLength(), body.byteStream(), response.body.contentType()). In OkHttp 4.x, Response.body is ResponseBody?; this is either a compile error or an NPE at runtime on null-body responses.
  • bulk_message_request.go line 53: strings.Split(input.AttachmentURLs, ",") when AttachmentURLs == "" returns [""] not []string{}, injecting a spurious empty-URL attachment into every CSV bulk message that has no attachments.

A softer concern: the Discord handler no longer forwards attachment_urls to the MessageSend request, silently dropping Discord-initiated MMS attachments.

Confidence Score: 3/5

  • Two concrete bugs — a likely compile error / NPE in the Kotlin download path and a data-corruption issue in CSV bulk messaging — need fixes before this is safe to merge.
  • The overall refactoring direction is sound and most of the stack changes are clean. However, the null-body removal in HttpSmsApiService.kt breaks the primary MMS send path (either fails to compile or throws NPE at runtime), and the strings.Split("", ",") bug in bulk_message_request.go corrupts CSV bulk messages that have no attachments. Both issues affect core user paths.
  • android/app/src/main/java/com/httpsms/HttpSmsApiService.kt (null body dereference) and api/pkg/requests/bulk_message_request.go (empty-string split produces spurious attachment).

Important Files Changed

Filename Overview
android/app/src/main/java/com/httpsms/HttpSmsApiService.kt Refactored downloadAttachment to return Pair<File?, MediaType?> — but the null guard for response.body was removed while the body is still accessed directly, causing a compile error or NPE on null bodies.
api/pkg/requests/bulk_message_request.go ToMessageSendParams uses strings.Split(input.AttachmentURLs, ",") unconditionally; when AttachmentURLs is empty this produces [""], injecting a spurious empty-URL attachment into every CSV bulk message that has no attachments.
android/app/src/main/java/com/httpsms/FirebaseMessagingService.kt Attachment list changed from List<Attachment> to List<String>, downloadedFiles now carries the MediaType alongside the file, and a new extractFileName helper derives a proper filename from the URL and MIME type. Logic looks correct aside from the null-body issue in HttpSmsApiService.
api/pkg/entities/message.go Replaced []MessageAttachment (JSON-serialised struct) with pq.StringArray / text[]. The DB schema change is straightforward; existing rows with the old JSON column will need a migration.
api/pkg/validators/validator.go Added multipleAttachmentURLRule custom rule that validates each URL in the []string slice for a valid http/https scheme and non-empty host. Logic is clean.
api/pkg/handlers/discord_handler.go Removes all MMS attachment support from the Discord integration without replacement; users sending attachments via Discord bot will have them silently dropped.

Sequence Diagram

sequenceDiagram
    participant API as httpSMS API
    participant FCM as FirebaseMessagingService
    participant HttpSvc as HttpSmsApiService
    participant Carrier as MMS Carrier

    API->>FCM: Push notification (Message with attachments: []string)
    FCM->>FCM: handleMmsMessage(message)
    loop For each attachment URL
        FCM->>HttpSvc: downloadAttachment(context, url, messageId, index)
        HttpSvc->>HttpSvc: GET url (OkHttp)
        HttpSvc-->>FCM: Pair<File, MediaType> (or Pair<null,null> on failure)
        FCM->>FCM: extractFileName(url, index, mimeType)
    end
    FCM->>FCM: Build PduBody (text part + media parts with real MediaType)
    FCM->>FCM: PduComposer.make() → pduBytes
    FCM->>Carrier: sendMultimediaMessage(pduUri)
    Carrier-->>FCM: Sent broadcast
    FCM->>API: POST /v1/messages/{id}/events (sent)
Loading

Comments Outside Diff (1)

  1. api/pkg/handlers/discord_handler.go, line 416-424 (link)

    P2 Discord integration silently drops MMS attachments

    The refactoring removes the attachment_urls option parsing from the Discord handler's createRequest method. Users who were sending MMS messages via the Discord bot by passing attachment_urls will now have their attachments silently ignored — the MessageSend request is built without any Attachments field at all.

    If Discord MMS support should be preserved, the new []string attachment format makes it straightforward to re-add:

    attachments := []string{}
    for _, u := range strings.Split(getOption("attachment_urls"), ",") {
        if clean := strings.TrimSpace(u); clean != "" {
            attachments = append(attachments, clean)
        }
    }
    return requests.MessageSend{
        From:        getOption("from"),
        To:          getOption("to"),
        Content:     getOption("message"),
        Attachments: attachments,
    }

    If this is intentional (Discord attachments are not supported in the new model), please document it.

Reviews (1): Last reviewed commit: "Add fix for MMS attachments" | Re-trigger Greptile

Comment thread android/app/src/main/java/com/httpsms/HttpSmsApiService.kt
Comment thread api/pkg/requests/bulk_message_request.go Outdated
Comment thread android/app/src/main/java/com/httpsms/HttpSmsApiService.kt Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates MMS attachment handling across the httpSMS stack by changing attachments from {content_type, url} objects to plain URL strings, adjusting validation/storage accordingly, and updating the UI/mobile rendering/sending behavior.

Changes:

  • Web: send and display attachments as URL strings (links) instead of typed attachment objects.
  • API: change request/service/event/entity attachment types to []string, add a custom validator rule for attachment URLs.
  • Android: update message model + MMS send flow to download attachments and use the server-provided content type.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
web/pages/threads/_id/index.vue Render attachments as clickable URL links in the thread view.
web/pages/messages/index.vue Send attachments to the API as a trimmed string[] instead of {content_type,url} objects; update page title casing.
web/models/message.ts Update Message.attachments type to string[] | null.
api/pkg/validators/validator.go Add multipleAttachmentURL validation rule for []string attachment URLs.
api/pkg/validators/message_handler_validator.go Apply attachment URL validation rules to send/bulk-send requests; remove prior per-attachment checks.
api/pkg/services/message_service.go Update MessageSendParams.Attachments type to []string.
api/pkg/requests/message_send_request.go Change attachments field to []string and sanitize/trim attachments.
api/pkg/requests/message_bulk_send_request.go Change bulk-send attachments field to []string and sanitize/trim attachments; mark encrypted as optional.
api/pkg/requests/bulk_message_request.go Sanitize comma-separated attachment URLs; pass attachments into MessageSendParams.
api/pkg/handlers/discord_handler.go Remove parsing/surfacing of Discord attachment_urls into the send request.
api/pkg/events/message_api_sent_event.go Change event payload attachments type to []string.
api/pkg/entities/message.go Change persisted Message.Attachments storage to pq.StringArray (text[]).
android/app/src/main/java/com/httpsms/Models.kt Update Message.attachments model to List<String>?.
android/app/src/main/java/com/httpsms/HttpSmsApiService.kt Return (File, MediaType) info when downloading attachments; enforce max size constant.
android/app/src/main/java/com/httpsms/FirebaseMessagingService.kt Build MMS parts using downloaded file + detected content type; improve filenames; adjust cleanup.
android/app/src/main/java/com/httpsms/Constants.kt Introduce MAX_MMS_ATTACHMENT_SIZE constant.

Comment thread api/pkg/requests/bulk_message_request.go
Comment thread api/pkg/validators/validator.go Outdated
Comment thread web/pages/threads/_id/index.vue
Comment thread android/app/src/main/java/com/httpsms/Constants.kt Outdated
Comment thread android/app/src/main/java/com/httpsms/HttpSmsApiService.kt
Comment thread android/app/src/main/java/com/httpsms/FirebaseMessagingService.kt Outdated
Comment thread api/pkg/handlers/discord_handler.go
Comment thread api/pkg/validators/message_handler_validator.go
Comment thread api/pkg/entities/message.go Outdated
@AchoArnold AchoArnold merged commit cbc9b55 into main Mar 23, 2026
5 of 7 checks passed
@AchoArnold AchoArnold deleted the feature/mms-attachment-refactor branch March 23, 2026 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants