Skip to content

[mms] add extended support#379

Draft
capcom6 wants to merge 2 commits into
masterfrom
mms/extended-support
Draft

[mms] add extended support#379
capcom6 wants to merge 2 commits into
masterfrom
mms/extended-support

Conversation

@capcom6

@capcom6 capcom6 commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Added full MMS support for sending and receiving messages with attachments.
    • Added inbox attachment viewing and on-demand download for received MMS content.
    • Inbox results can now include attachment details when requested.
  • Bug Fixes

    • Improved handling of multipart SMS/MMS delivery and message state updates.
    • Tightened permission checks and fallback behavior for reliable MMS downloading.
  • Documentation

    • Expanded setup and API docs for MMS usage, permissions, webhooks, and attachment handling.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e09b1ac3-a732-4984-9e49-013053f4cff5

📥 Commits

Reviewing files that changed from the base of the PR and between 07f1b2d and 883ee5d.

📒 Files selected for processing (44)
  • README.md
  • app/src/main/AndroidManifest.xml
  • app/src/main/assets/api/swagger.json
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/res/values-zh/strings.xml
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/res/xml/root_preferences.xml
  • docs/MMS.md
✅ Files skipped from review due to trivial changes (4)
  • app/src/main/res/xml/root_preferences.xml
  • README.md
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • docs/MMS.md
🚧 Files skipped from review as they are similar to previous changes (36)
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/res/values-zh/strings.xml
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt

Walkthrough

This PR implements end-to-end MMS support: a vendored AOSP PDU composition library, MMS domain/API models, attachment storage, outbound send orchestration via SmsManager with sent-state broadcast handling, inbound MMS retrieval parsing with permission gating, new local server endpoints for attachment access, Android manifest/FileProvider wiring, and documentation.

Changes

MMS send, receive, and API exposure

Layer / File(s) Summary
AOSP PDU encoding library
app/src/main/java/.../mms/pdu/aosp/*
Vendored classes (PduHeaders, PduComposer, PduPart, PduBody, EncodedStringValue, GenericPdu, MultimediaMessagePdu, SendReq, ContentType, CharacterSets, exceptions) implement WSP/MMS header encoding and PDU byte composition.
MMS domain models and message contracts
data/entities/Message.kt, domain/MessageContent.kt, modules/gateway/GatewayApi.kt, modules/gateway/GatewayService.kt, modules/localserver/domain/messages/*, assets/api/swagger.json
Adds MessageType.Mms, MessageContent.Mms, gateway API MMS parsing, local server MmsMessage/PostMessageRequest validation, and OpenAPI schemas plus the attachment download endpoint contract.
Attachment storage and Android platform wiring
modules/mms/MmsAttachmentStorage.kt, modules/mms/Module.kt, App.kt, AndroidManifest.xml, res/xml/file_paths.xml
Persists inbound attachment bytes/metadata under app-private storage; registers Koin DI module; adds FileProvider, ACCESS_NETWORK_STATE permission, and file path config.
Outbound MMS send orchestration
modules/mms/MmsSender.kt, modules/messages/MessagesService.kt, modules/messages/MessagesRepository.kt, receivers/EventsReceiver.kt
Builds and sends MMS PDUs via SmsManager, manages PDU file lifecycle, handles ACTION_MMS_SENT broadcasts asynchronously, and updates processing state.
Inbound MMS retrieve, parse, and persistence
modules/receiver/parsers/MMSRetrieveParser.kt, MMSParser.kt, MmsContentObserver.kt, MmsReceiver.kt, ReceiverService.kt, MessagesReceiver.kt, IncomingMessagesService.kt
Parses M-Retrieve.conf PDUs, gates MMS observation on READ_SMS permission, tracks processing high-water marks, and decodes/stores attachments on incoming message save.
Local server MMS endpoints and route wiring
modules/localserver/routes/InboxRoutes.kt, MessagesRoutes.kt, WebService.kt
Adds includeAttachments query support, attachment metadata in inbox listing, an authenticated per-part attachment download endpoint, and MMS send/content mapping.
Documentation, localization, and config
docs/MMS.md, README.md, res/values-zh/strings.xml, res/xml/root_preferences.xml
Documents MMS setup, API usage, webhooks, limits, and troubleshooting; adds Chinese strings for default-SMS-app role.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MessagesService
  participant MmsSender
  participant PduComposer
  participant SmsManager
  participant EventsReceiver

  MessagesService->>MmsSender: send(messageId, phoneNumbers, mms, subscriptionId)
  MmsSender->>PduComposer: make()
  MmsSender->>SmsManager: sendMultimediaMessage(uri, PendingIntent)
  SmsManager-->>EventsReceiver: ACTION_MMS_SENT
  EventsReceiver->>MessagesService: processStateIntent(intent, resultCode)
  MessagesService->>MmsSender: cleanup(context, messageId)
Loading
sequenceDiagram
  participant MmsContentObserver
  participant ReceiverService
  participant MMSRetrieveParser
  participant IncomingMessagesService
  participant MmsAttachmentStorage

  MmsContentObserver->>ReceiverService: processMmsDownloaded(mmsId)
  ReceiverService->>MMSRetrieveParser: parse(pduBytes)
  MMSRetrieveParser-->>ReceiverService: MRetrieveConf(parts)
  ReceiverService->>IncomingMessagesService: save(InboxMessage.MMS)
  loop each attachment part
    IncomingMessagesService->>MmsAttachmentStorage: store(messageId, partId, bytes)
  end
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is related to the MMS-focused changes, but it is too vague to describe the main update clearly. Rename it to state the primary change, e.g. "Add end-to-end MMS support" or similar.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mms/extended-support

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt (1)

77-101: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Duplicate textFilter declaration and double registration will cause compilation error and duplicate message processing.

textFilter is declared twice (lines 77 and 85), which won't compile. Additionally, if this were to compile, the same INSTANCE would be registered twice for SMS_RECEIVED_ACTION—once on appContext (line 80) and once on context (line 88)—causing each SMS to be processed twice.

This appears to be incomplete refactoring or merge conflict residue.

🐛 Proposed fix: Remove the duplicate registration block
         fun register(context: Context) {
             val appContext = context.applicationContext
             unregister(appContext)

             val textFilter = IntentFilter().apply {
                 addAction(Intents.SMS_RECEIVED_ACTION)
             }
             appContext.registerReceiver(
                 INSTANCE,
                 textFilter
             )

-            val textFilter = IntentFilter().apply {
-                addAction(Intents.SMS_RECEIVED_ACTION)
-            }
-            context.registerReceiver(
-                INSTANCE,
-                textFilter
-            )
-
             val dataFilter = IntentFilter().apply {
                 addAction(Intents.DATA_SMS_RECEIVED_ACTION)
                 addDataScheme("sms")
                 addDataAuthority("*", "53739")
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt`
around lines 77 - 101, Remove the duplicate registration block for
SMS_RECEIVED_ACTION to fix the compilation error and prevent duplicate message
processing. You have two identical textFilter declarations and registrations of
the same INSTANCE receiver—one registering on appContext and another on context.
Delete the second duplicate textFilter IntentFilter block (the one that
registers on context) and keep only the first registration on appContext. This
will leave you with one textFilter for SMS_RECEIVED_ACTION on appContext and the
separate dataFilter block for DATA_SMS_RECEIVED_ACTION, eliminating both the
variable naming conflict and the duplicate receiver registration.
app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt (1)

130-146: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Permanent processing failure blocks subsequent messages.

If processMmsDownloaded(mmsId) throws an exception, the high-water mark is not advanced, causing the same failing MMS to be retried on every processNewMessages() call. A single corrupt or unparseable MMS would block processing of all later messages indefinitely.

Consider updating the mark after a configurable number of retries, or always advance the mark while logging the failure to prevent one bad message from blocking the pipeline.

Proposed fix: always advance mark, rely on logging for failed items
                 try {
                     processMmsDownloaded(mmsId)
-                    storage.mmsLastProcessedID = mmsId
                 } catch (e: Exception) {
                     logsService.insert(
                         LogEntry.Priority.ERROR,
                         MODULE_NAME,
                         "Failed processing downloaded MMS (id=$mmsId)",
-                        mapOf("mmsId" to mmsId)
+                        mapOf("mmsId" to mmsId, "error" to (e.message ?: e.toString()))
                     )
                 }
+                // Always advance mark to prevent one corrupt MMS from blocking the pipeline
+                storage.mmsLastProcessedID = mmsId
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt`
around lines 130 - 146, The issue is that when processMmsDownloaded(mmsId)
throws an exception in the while loop of the cursor processing block, the
high-water mark storage.mmsLastProcessedID is not advanced because it only gets
set inside the try block. This causes the same failing MMS to be retried
indefinitely, blocking all subsequent messages. Fix this by moving the
storage.mmsLastProcessedID = mmsId assignment outside and after the try-catch
block so that it always executes for every MMS, regardless of whether
processMmsDownloaded succeeds or fails. This allows the pipeline to advance past
problem messages while still logging the failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 56-62: The attachments array in the OpenAPI schema lacks a
maxItems constraint, which allows unrestricted MMS payload sizes and compromises
resource control validation. Add a maxItems property to the attachments array
definition in the swagger.json file to enforce a reasonable upper bound on the
number of attachments that can be included in a single MMS message, preventing
oversized payloads at the request validation level.
- Around line 66-90: The smsgateway.MmsAttachment schema currently describes
data and url as mutually exclusive, but the schema definition does not enforce
this constraint—both properties are optional with no validation ensuring exactly
one is provided. Add OpenAPI schema constraint logic (such as oneOf composition)
to the MmsAttachment object that enforces exactly one of the data or url fields
must be present, not both and not neither. This will align the schema validation
with the documented contract and eliminate runtime ambiguity.

In `@app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt`:
- Around line 257-267: The MMS content mapping now includes raw attachment
payload fields (data, url) that are sensitive. The existing error/failure
handling path logs the complete message object, which will leak this sensitive
attachment content and inflate log storage. Locate the error handling path that
logs the full message object and replace it with sanitized logging that excludes
the raw attachment payload fields (data and url) from the
MessageContent.Mms.Attachment objects, or log only safe metadata instead of the
complete message.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 145-165: The attachment response handler loads the entire file
into memory using file.readBytes() before sending it to the client, which can
cause OOM errors for large MMS attachments or concurrent requests. Replace the
call.respondBytes(file.readBytes(), contentType) invocation with
call.respondFile(file, contentType) to stream the file directly to the client
instead of buffering it entirely in memory. This will efficiently handle files
of any size while maintaining the same Content-Disposition header and
content-type settings.

In `@app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt`:
- Around line 330-371: The code resolves the fromMsisdn variable based on the
SIM slot and subscription details (to address carrier-specific MMS delivery
requirements), but then passes null to the mmsSender.send() method instead of
using the resolved value. The contradictory comment on lines 368-370 conflicts
with the earlier rationale for resolving fromMsisdn. Fix this inconsistency by
passing the resolved fromMsisdn variable to the mmsSender.send() call instead of
null, and update or remove the contradictory comment so the implementation
aligns with the stated strategy of providing explicit MSISDN when available to
avoid carrier-specific delivery failures.

In `@app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt`:
- Around line 63-67: In the remove function in MmsAttachmentStorage.kt, the
delete() method calls on both the individual files (in the forEach block) and
the directory itself are ignoring their Boolean return values, which means
failed deletions go unreported. Capture and check the return values of these
delete() operations and surface any failures through appropriate logging or
exception handling to prevent silent data retention on disk.

In `@app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt`:
- Around line 163-169: The code in MmsSender.kt reads the full response body
into memory with it.body?.bytes() without size validation, risking
OutOfMemoryError on large responses. Before calling bytes() on the response
body, check the Content-Length header from the response and validate that the
attachment size is within an acceptable limit (e.g., a reasonable max attachment
size constant). If the size exceeds the limit, throw a RuntimeException with a
descriptive message instead of attempting to allocate the full response into
memory.
- Around line 160-169: The code at the point where httpClient.newCall() is
executed fetches caller-supplied URLs without any validation, creating an SSRF
vulnerability. Add URL validation logic immediately after confirming the URL is
non-null and before making the HTTP request in the withContext block. Validate
that the URL does not point to private IP addresses (localhost, 127.0.0.1,
10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), link-local addresses
(169.254.0.0/16), or other restricted schemes. If the URL fails validation,
throw an exception with a clear error message instead of proceeding with the
fetch. This prevents malicious senders from using the attachment mechanism to
target private network endpoints and exfiltrate data via MMS.

In
`@app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java`:
- Around line 188-190: The isSupportedAudioType method rejects application/ogg
because the isAudioType check requires the content type to start with audio/,
even though AUDIO_OGG is listed in the supported types. Remove the isAudioType
check from the isSupportedAudioType method since isSupportedType already
validates against the complete list of supported audio types including
AUDIO_OGG, so the additional isAudioType check is unnecessary and overly
restrictive.

In
`@app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java`:
- Around line 217-218: The split() method in the EncodedStringValue class loses
charset fidelity by using platform-default encoding instead of respecting the
mCharacterSet field. In the location where a new EncodedStringValue is
constructed with temp[i].getBytes(), the getBytes() call needs to explicitly
specify the mCharacterSet parameter instead of relying on the platform default
encoding. This ensures that non-default encoded values maintain their original
charset when being split and re-encoded.

In
`@app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java`:
- Around line 796-813: The recipient validation logic incorrectly treats missing
fields as successful. The methods appendHeader at lines 796, 801, and 806 return
PDU_COMPOSE_FIELD_NOT_SET when a field is absent, which is different from
PDU_COMPOSE_CONTENT_ERROR. The current conditions only check for inequality to
PDU_COMPOSE_CONTENT_ERROR, allowing missing fields to be treated as successfully
added. Change the conditional checks for TO, CC, and BCC from checking "not
equal to PDU_COMPOSE_CONTENT_ERROR" to explicitly checking for a success
condition (likely equality to a success constant or checking that the return
value is not one of the error/missing constants). This ensures that recipient is
only set to true when a recipient field is actually present and successfully
added, allowing the validation at line 811 to correctly reject PDUs with no
recipients.

In `@app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java`:
- Around line 235-237: The constants
RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_LIMITATIONS_NOT_MET and
RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED both have
the hex value 0xE6, making them impossible to distinguish at runtime. Fix this
by assigning a unique hex value to
RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED. Based on
the sequence pattern where
RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_FORWARDING_DENIED uses 0xE8,
change RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED to
use 0xE7 to maintain proper hex value ordering and ensure each constant has a
distinct wire representation.

In `@app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java`:
- Around line 410-413: The code at lines 410-413 retrieves a `P_CONTENT_ID`
header from `mPartHeader` and immediately constructs a String from the result
without null-checking `contentId`. If the header is not present, `get()` returns
null, causing a NullPointerException when passed to the String constructor. Add
a null guard check after retrieving `contentId` from the map (after line 411) to
verify it is not null before calling `new String(contentId)`. If `contentId` is
null, provide an appropriate fallback value or return value instead of crashing.

In
`@app/src/main/java/me/capcom/smsgateway/modules/ping/services/PingForegroundService.kt`:
- Around line 59-64: The null check for intervalSeconds at line 59 exits the
thread without stopping the foreground service, leaving it running with no work
to do. Add a stopSelf() call before the return@thread statement when
intervalSeconds is null, mirroring the behavior of the interval <= 0 check in
the subsequent lines. This ensures the foreground service is properly cleaned up
when no valid interval is configured.

In
`@app/src/main/java/me/capcom/smsgateway/modules/receiver/SmsContentObserver.kt`:
- Around line 45-48: The constant MODULE_NAME is referenced in the
logsService.insert calls throughout the SmsContentObserver class but is not
defined in the companion object (only TAG is defined). Replace all occurrences
of MODULE_NAME with TAG in the logsService.insert method calls across the class.
This includes the log statements at lines 45-48 (anchor), 104-108, 120-124,
149-154, 177-182, and 194-196 (siblings). Each of these sites should have
MODULE_NAME replaced with TAG to match the defined constant in the companion
object and resolve the compilation error.

In `@docs/MMS.md`:
- Around line 288-315: The documentation describes a separate detail endpoint
`GET /inbox/{messageId}` that is not actually implemented in the code. Remove
this non-existent endpoint from the documentation and update the section to
reflect the two actual endpoints: the list endpoint that returns full message
details including attachment metadata, and the attachment bytes endpoint.
Alternatively, clarify that full message details (including the attachments
array shown in the example JSON) are returned as part of the list response from
`GET /inbox?type=MMS_DOWNLOADED`, eliminating the need for a separate detail
endpoint.

---

Outside diff comments:
In `@app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt`:
- Around line 77-101: Remove the duplicate registration block for
SMS_RECEIVED_ACTION to fix the compilation error and prevent duplicate message
processing. You have two identical textFilter declarations and registrations of
the same INSTANCE receiver—one registering on appContext and another on context.
Delete the second duplicate textFilter IntentFilter block (the one that
registers on context) and keep only the first registration on appContext. This
will leave you with one textFilter for SMS_RECEIVED_ACTION on appContext and the
separate dataFilter block for DATA_SMS_RECEIVED_ACTION, eliminating both the
variable naming conflict and the duplicate receiver registration.

In
`@app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt`:
- Around line 130-146: The issue is that when processMmsDownloaded(mmsId) throws
an exception in the while loop of the cursor processing block, the high-water
mark storage.mmsLastProcessedID is not advanced because it only gets set inside
the try block. This causes the same failing MMS to be retried indefinitely,
blocking all subsequent messages. Fix this by moving the
storage.mmsLastProcessedID = mmsId assignment outside and after the try-catch
block so that it always executes for every MMS, regardless of whether
processMmsDownloaded succeeds or fails. This allows the pipeline to advance past
problem messages while still logging the failures.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ccbb3ea3-ae9f-4980-903d-cbc7584aea66

📥 Commits

Reviewing files that changed from the base of the PR and between 458768f and 2951f90.

📒 Files selected for processing (48)
  • .idea/gradle.xml
  • .idea/misc.xml
  • README.md
  • app/src/main/AndroidManifest.xml
  • app/src/main/assets/api/swagger.json
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/ping/services/PingForegroundService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/SmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/StateStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/res/values-zh/strings.xml
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/res/xml/root_preferences.xml
  • docs/MMS.md

Comment thread app/src/main/assets/api/swagger.json
Comment on lines +66 to +90
"smsgateway.MmsAttachment": {
"properties": {
"contentType": {
"description": "MIME type. Common values: image/jpeg, image/png, image/gif, video/mp4, audio/amr, audio/mpeg, text/plain.",
"type": "string"
},
"name": {
"description": "Optional filename for the part (used as Content-Location).",
"nullable": true,
"type": "string"
},
"data": {
"description": "Base64-encoded bytes. Mutually exclusive with `url`.",
"format": "byte",
"nullable": true,
"type": "string"
},
"url": {
"description": "HTTP(S) URL the device will fetch immediately before sending. Mutually exclusive with `data`.",
"nullable": true,
"type": "string"
}
},
"required": ["contentType"],
"type": "object"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enforce data XOR url for smsgateway.MmsAttachment.

The schema text says “exactly one,” but current validation allows both or neither. That creates client/server contract drift and ambiguous runtime behavior.

Suggested fix
"smsgateway.MmsAttachment": {
  "properties": {
    ...
  },
+ "oneOf": [
+   { "required": ["data"] },
+   { "required": ["url"] }
+ ],
+ "not": {
+   "required": ["data", "url"]
+ },
  "required": ["contentType"],
  "type": "object"
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"smsgateway.MmsAttachment": {
"properties": {
"contentType": {
"description": "MIME type. Common values: image/jpeg, image/png, image/gif, video/mp4, audio/amr, audio/mpeg, text/plain.",
"type": "string"
},
"name": {
"description": "Optional filename for the part (used as Content-Location).",
"nullable": true,
"type": "string"
},
"data": {
"description": "Base64-encoded bytes. Mutually exclusive with `url`.",
"format": "byte",
"nullable": true,
"type": "string"
},
"url": {
"description": "HTTP(S) URL the device will fetch immediately before sending. Mutually exclusive with `data`.",
"nullable": true,
"type": "string"
}
},
"required": ["contentType"],
"type": "object"
"smsgateway.MmsAttachment": {
"properties": {
"contentType": {
"description": "MIME type. Common values: image/jpeg, image/png, image/gif, video/mp4, audio/amr, audio/mpeg, text/plain.",
"type": "string"
},
"name": {
"description": "Optional filename for the part (used as Content-Location).",
"nullable": true,
"type": "string"
},
"data": {
"description": "Base64-encoded bytes. Mutually exclusive with `url`.",
"format": "byte",
"nullable": true,
"type": "string"
},
"url": {
"description": "HTTP(S) URL the device will fetch immediately before sending. Mutually exclusive with `data`.",
"nullable": true,
"type": "string"
}
},
"oneOf": [
{ "required": ["data"] },
{ "required": ["url"] }
],
"not": {
"required": ["data", "url"]
},
"required": ["contentType"],
"type": "object"
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/assets/api/swagger.json` around lines 66 - 90, The
smsgateway.MmsAttachment schema currently describes data and url as mutually
exclusive, but the schema definition does not enforce this constraint—both
properties are optional with no validation ensuring exactly one is provided. Add
OpenAPI schema constraint logic (such as oneOf composition) to the MmsAttachment
object that enforces exactly one of the data or url fields must be present, not
both and not neither. This will align the schema validation with the documented
contract and eliminate runtime ambiguity.

Comment thread app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt Outdated
Comment on lines +235 to +237
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_LIMITATIONS_NOT_MET = 0xE6;
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED = 0xE6;
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_FORWARDING_DENIED = 0xE8;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Two distinct response-status constants collapse to the same wire value.

These two named statuses currently share 0xE6, so they cannot be represented distinctly.

Proposed fix
 public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_LIMITATIONS_NOT_MET  = 0xE6;
-public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED = 0xE6;
+public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED = 0xE7;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_LIMITATIONS_NOT_MET = 0xE6;
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED = 0xE6;
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_FORWARDING_DENIED = 0xE8;
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_LIMITATIONS_NOT_MET = 0xE6;
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED = 0xE7;
public static final int RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_FORWARDING_DENIED = 0xE8;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java`
around lines 235 - 237, The constants
RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_LIMITATIONS_NOT_MET and
RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED both have
the hex value 0xE6, making them impossible to distinguish at runtime. Fix this
by assigning a unique hex value to
RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED. Based on
the sequence pattern where
RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_FORWARDING_DENIED uses 0xE8,
change RESPONSE_STATUS_ERROR_PERMANENT_REPLY_CHARGING_REQUEST_NOT_ACCEPTED to
use 0xE7 to maintain proper hex value ordering and ensure each constant has a
distinct wire representation.

Comment on lines +410 to +413
if (null == location) {
byte[] contentId = (byte[]) mPartHeader.get(P_CONTENT_ID);
return "cid:" + new String(contentId);
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard null contentId before building the "cid:" fallback.

At Line 410–413, new String(contentId) can throw when no P_CONTENT_ID header is set, causing a hard crash instead of a safe fallback.

Suggested patch
         if (null == location) {
             byte[] contentId = (byte[]) mPartHeader.get(P_CONTENT_ID);
-            return "cid:" + new String(contentId);
+            if (contentId == null || contentId.length == 0) {
+                return null;
+            }
+            return "cid:" + new String(contentId);
         } else {
             return new String(location);
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (null == location) {
byte[] contentId = (byte[]) mPartHeader.get(P_CONTENT_ID);
return "cid:" + new String(contentId);
} else {
if (null == location) {
byte[] contentId = (byte[]) mPartHeader.get(P_CONTENT_ID);
if (contentId == null || contentId.length == 0) {
return null;
}
return "cid:" + new String(contentId);
} else {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java`
around lines 410 - 413, The code at lines 410-413 retrieves a `P_CONTENT_ID`
header from `mPartHeader` and immediately constructs a String from the result
without null-checking `contentId`. If the header is not present, `get()` returns
null, causing a NullPointerException when passed to the String constructor. Add
a null guard check after retrieving `contentId` from the map (after line 411) to
verify it is not null before calling `new String(contentId)`. If `contentId` is
null, provide an appropriate fallback value or return value instead of crashing.

Comment thread docs/MMS.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt (1)

163-166: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a collision-resistant PDU filename.

The lossy sanitizer can map distinct message IDs to the same PDU path, so concurrent or retried MMS sends can overwrite/cleanup each other’s files. Use a hash of the original ID instead of replacement.

Suggested fix
+import java.security.MessageDigest
+
@@
     private fun writePduFile(messageId: String, bytes: ByteArray): File {
         val dir = File(context.filesDir, "mms-out").apply { mkdirs() }
-        val safeId = messageId.replace(Regex("[^A-Za-z0-9_.-]"), "_")
-        val file = File(dir, "$safeId.pdu")
+        val file = File(dir, "${pduFileName(messageId)}.pdu")
         file.writeBytes(bytes)
         return file
     }
@@
         /** Clean up any stale PDU files for the given message. */
         fun cleanup(context: Context, messageId: String) {
-            val safeId = messageId.replace(Regex("[^A-Za-z0-9_.-]"), "_")
-            File(File(context.filesDir, "mms-out"), "$safeId.pdu").delete()
+            File(File(context.filesDir, "mms-out"), "${pduFileName(messageId)}.pdu").delete()
         }
+
+        private fun pduFileName(messageId: String): String =
+            MessageDigest.getInstance("SHA-256")
+                .digest(messageId.toByteArray(Charsets.UTF_8))
+                .joinToString("") { "%02x".format(it) }
     }

Also applies to: 203-205

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt` around lines
163 - 166, The writePduFile method uses a lossy character replacement sanitizer
to generate the safeId variable, which can cause different message IDs to map to
the same filename, leading to file collisions and overwrites during concurrent
or retried sends. Replace the lossy sanitizer approach with a
collision-resistant hash function (such as MD5 or SHA-256) applied to the
original messageId to generate a unique and safe filename identifier. This same
fix must also be applied at the second location mentioned (around lines 203-205)
where similar PDU filename generation occurs.
app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt (1)

39-65: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t advance the MMS high-water mark before durable processing.

Setting the mark to the current max before the posted catch-up skips MMS rows that arrived before permission/startup, and the unconditional advance after exceptions makes transient read/save failures unretryable. Only advance after successful processing, or persist a separate failed-ID quarantine for corrupt rows.

Suggested fix
-        // Initialize high-water mark to current max ID if not set
-        if (storage.mmsLastProcessedID == 0L) {
-            storage.mmsLastProcessedID = queryMaxMmsId()
-        }
-
         val thread = HandlerThread("MmsContentObserver").apply { start() }
@@
                 try {
                     processMmsDownloaded(mmsId)
                     storage.mmsLastProcessedID = mmsId
                 } catch (e: Exception) {
                     logsService.insert(
@@
                         mapOf("mmsId" to mmsId, "error" to (e.message ?: e.toString())),
                     )
                 }
-                // Always advance mark to prevent one corrupt MMS from blocking the pipeline
-                storage.mmsLastProcessedID = mmsId
             }
         }

Also applies to: 133-145

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt`
around lines 39 - 65, The high-water mark storage.mmsLastProcessedID is being
set to the current max MMS ID before the catch-up processing starts, which
causes rows that arrived before startup or permission grant to be skipped
forever. Remove the initialization block that sets storage.mmsLastProcessedID to
queryMaxMmsId() at the beginning of the observer setup. Instead, only advance
the high-water mark after successfully processing messages, ensuring that
transient failures during processNewMessages() remain retryable and no messages
are lost. Apply the same fix at the other affected location where the high-water
mark is managed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt`:
- Around line 231-233: The logging fields in the catch block of processMessage()
are accessing message.content multiple times (for getting the class name and
casting to check attachments), which will throw an exception again if the
content is invalid and caused the original error. Wrap the message.content
access in a runCatching block before constructing the log context, store the
result safely, and use that cached result when building the logging map entries
for messageType and attachmentsCount to prevent the catch block from being
aborted by a second exception.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 159-162: The Content-Disposition header in the InboxRoutes.kt file
is directly interpolating the sender-controlled attachment.displayName without
escaping, which allows header injection or malformed headers through quote and
CRLF characters. Before using attachment.displayName in the header value string
(in the call.response.header call), properly escape or encode it to prevent
injection attacks. Apply appropriate escaping such as removing or encoding
problematic characters like quotes and newlines, or use URL encoding for the
filename value.
- Around line 155-163: The current implementation uses call.respondFile(file)
which infers the content type from the filename, causing the stored
attachment.contentType to be lost. Replace the call.respondFile(file) invocation
with LocalFileContent, passing both the file and the attachment.contentType to
preserve the actual MIME type information that was stored when the attachment
was uploaded. The parseContentType() helper function (defined at lines 230-233)
may be useful for properly formatting the content type before passing it to
LocalFileContent.

In `@app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt`:
- Around line 149-154: The attachmentBytes function in MmsSender.kt has no size
validation for decoded Base64 attachment data, which allows arbitrarily large
attachments to be decoded and allocated in memory, risking Out-of-Memory errors.
Add a maximum size limit constant for MMS attachments and validate the decoded
byte array size immediately after the Base64.decode call in the attachmentBytes
function. If the decoded data exceeds this limit, throw an
IllegalArgumentException with a clear error message before the large allocation
causes issues. This bounds checking should occur within the try block to ensure
proper error handling.

---

Outside diff comments:
In `@app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt`:
- Around line 163-166: The writePduFile method uses a lossy character
replacement sanitizer to generate the safeId variable, which can cause different
message IDs to map to the same filename, leading to file collisions and
overwrites during concurrent or retried sends. Replace the lossy sanitizer
approach with a collision-resistant hash function (such as MD5 or SHA-256)
applied to the original messageId to generate a unique and safe filename
identifier. This same fix must also be applied at the second location mentioned
(around lines 203-205) where similar PDU filename generation occurs.

In
`@app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt`:
- Around line 39-65: The high-water mark storage.mmsLastProcessedID is being set
to the current max MMS ID before the catch-up processing starts, which causes
rows that arrived before startup or permission grant to be skipped forever.
Remove the initialization block that sets storage.mmsLastProcessedID to
queryMaxMmsId() at the beginning of the observer setup. Instead, only advance
the high-water mark after successfully processing messages, ensuring that
transient failures during processNewMessages() remain retryable and no messages
are lost. Apply the same fix at the other affected location where the high-water
mark is managed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3c80b2d7-971f-47e6-9ab9-e71e17f5d428

📥 Commits

Reviewing files that changed from the base of the PR and between 2951f90 and 146de57.

📒 Files selected for processing (12)
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • docs/MMS.md
💤 Files with no reviewable changes (2)
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
✅ Files skipped from review due to trivial changes (1)
  • docs/MMS.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt

Comment thread app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt Outdated
Comment thread app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt Outdated
Comment thread app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt (1)

141-141: ⚖️ Poor tradeoff

Consider lazy-loading or batch-loading attachments to avoid N+1 I/O.

Each call to toDomain() triggers listAttachmentRefs(), resulting in a separate attachmentStorage.list() per message. With the 500-message limit, this could noticeably degrade response times.

Options:

  1. Lazy approach: Add an optional includeAttachments query parameter (default false) so clients opt-in only when needed.
  2. Batch approach: Collect all message IDs first, then load attachments in a single call or grouped batches.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`
at line 141, The current implementation in the InboxRoutes.kt file has an N+1
I/O problem where each message's toDomain() call triggers a separate
listAttachmentRefs() lookup via attachmentStorage.list(). Implement the
lazy-loading approach by adding an optional includeAttachments query parameter
(default false) to the endpoint, then modify the toDomain() call to
conditionally include attachments only when the parameter is true. This allows
clients to opt-in to attachment loading only when needed, avoiding unnecessary
I/O on the 500-message result set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Around line 154-164: Add a file existence check after retrieving the
attachment file to handle the case where attachment metadata exists but the
actual file is missing from disk. After the line val file = attachment.file,
check if the file exists using appropriate file system methods. If the file does
not exist, return call.respond(HttpStatusCode.NotFound) before reaching the
LocalFileContent call to prevent a 500 error and instead properly return a 404
response.

---

Nitpick comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt`:
- Line 141: The current implementation in the InboxRoutes.kt file has an N+1 I/O
problem where each message's toDomain() call triggers a separate
listAttachmentRefs() lookup via attachmentStorage.list(). Implement the
lazy-loading approach by adding an optional includeAttachments query parameter
(default false) to the endpoint, then modify the toDomain() call to
conditionally include attachments only when the parameter is true. This allows
clients to opt-in to attachment loading only when needed, avoiding unnecessary
I/O on the 500-message result set.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7dce6a8a-d8f0-4e84-9273-33331a3b49f2

📥 Commits

Reviewing files that changed from the base of the PR and between 146de57 and 7d4ab8e.

📒 Files selected for processing (3)
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt

@capcom6 capcom6 force-pushed the mms/extended-support branch from 7d4ab8e to 42cba0d Compare June 16, 2026 02:39

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java`:
- Around line 283-297: The appendTextString method accesses text[0] without
first checking if the text array is empty or null, which can cause an
ArrayIndexOutOfBoundsException. Add a null and empty length check before the
conditional that accesses text[0], and handle empty arrays appropriately (either
by returning early or raising a compose error to match the expected behavior).
This same validation issue also exists in the part content-type encoding at
lines 911-921, so ensure the same defensive checks are applied wherever text
arrays are accessed without prior validation.
- Around line 851-855: The code at line 851 calls
mPduHeader.getTextString(PduHeaders.CONTENT_TYPE) which can return null, and
passing null directly to the String constructor will throw a
NullPointerException before your null check on contentTypeIdentifier is reached.
Add a null check immediately after calling
getTextString(PduHeaders.CONTENT_TYPE) and before constructing the String
object; if the returned value is null, return PDU_COMPOSE_CONTENT_ERROR
immediately since content type is mandatory. This ensures the null case is
handled gracefully before attempting String construction.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: aedf6e87-0974-4d63-966e-a03ed25292af

📥 Commits

Reviewing files that changed from the base of the PR and between 7d4ab8e and 42cba0d.

📒 Files selected for processing (45)
  • README.md
  • app/src/main/AndroidManifest.xml
  • app/src/main/assets/api/swagger.json
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/SmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/StateStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/res/values-zh/strings.xml
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/res/xml/root_preferences.xml
  • docs/MMS.md
✅ Files skipped from review due to trivial changes (3)
  • app/src/main/res/xml/root_preferences.xml
  • README.md
  • docs/MMS.md
🚧 Files skipped from review as they are similar to previous changes (35)
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/res/values-zh/strings.xml
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/StateStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/SmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt (3)

222-235: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Redact full MMS payloads from logs.

message can now be an InboxMessage.MMS containing base64 attachment data, so these log entries can persist media contents and message bodies. Log metadata only.

🛡️ Proposed fix
         logsService.insert(
             LogEntry.Priority.DEBUG,
             MODULE_NAME,
             "ReceiverService::process - message received",
-            mapOf("message" to message)
+            mapOf(
+                "messageType" to message.javaClass.simpleName,
+                "date" to message.date,
+                "subscriptionId" to message.subscriptionId,
+            )
         )
@@
             logsService.insert(
                 LogEntry.Priority.DEBUG,
                 MODULE_NAME,
                 "ReceiverService::process - duplicate message, skipping",
-                mapOf("message" to message)
+                mapOf(
+                    "messageType" to message.javaClass.simpleName,
+                    "date" to message.date,
+                    "subscriptionId" to message.subscriptionId,
+                )
             )
             return
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt`
around lines 222 - 235, The logsService.insert calls in ReceiverService::process
are logging the full message object which can contain base64 attachment data and
message bodies from MMS messages, exposing sensitive media content in logs.
Replace the mapOf("message" to message) parameter in both logsService.insert
calls with metadata-only information that identifies the message (such as its ID
or type) without including the actual message content or attachment data. This
ensures sensitive message bodies and media are not persisted in debug logs.

160-173: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Persist MMS attachments once, outside the webhook-only branch.

Attachment storage backs the inbox/download API, but process() only stores InboxMessage.MMS attachments when triggerWebhooks is true. The downloaded path compensates by storing at Line 161, causing duplicate writes for downloaded MMS while process(..., false) never creates downloadable files. Move MMS attachment persistence into one helper called after dedup for every MMS, then keep the webhook branch only for emission.

♻️ Suggested refactor
-            try {
-                attachmentStorage.store(messageId, part.partId, name, part.contentType, part.data)
-            } catch (e: Exception) {
-                logsService.insert(
-                    LogEntry.Priority.WARN,
-                    MODULE_NAME,
-                    "Failed to persist MMS attachment",
-                    mapOf(
-                        "messageId" to messageId,
-                        "partId" to part.partId,
-                        "error" to (e.message ?: e.toString()),
-                    )
-                )
-            }
             attachments += InboxMessage.MMS.Attachment(
                 partId = part.partId,
                 contentType = part.contentType,
@@
 
         val incoming = incomingMessagesService.save(message)
+
+        if (message is InboxMessage.MMS) {
+            persistMmsAttachments(message)
+        }
 
         if (triggerWebhooks) {
             val (type, payload) = when (message) {
@@
 
                 is InboxMessage.MMS -> {
-                    message.attachments.forEach { att ->
-                        val decoded = att.data?.let { runCatching {
-                            android.util.Base64.decode(it, android.util.Base64.DEFAULT)
-                        }.getOrNull() }
-                        if (decoded != null) {
-                            try {
-                                attachmentStorage.store(
-                                    message.messageId,
-                                    att.partId,
-                                    att.name,
-                                    att.contentType,
-                                    decoded,
-                                )
-                            } catch (e: Exception) {
-                                logsService.insert(
-                                    LogEntry.Priority.WARN,
-                                    MODULE_NAME,
-                                    "Failed to persist MMS attachment",
-                                    mapOf(
-                                        "messageId" to message.messageId,
-                                        "partId" to att.partId,
-                                        "error" to (e.message ?: e.toString()),
-                                    )
-                                )
-                            }
-                        }
-                    }
-
                     WebHookEvent.MmsDownloaded to MmsDownloadedPayload(
                         messageId = message.messageId,

Add the helper outside process:

private fun persistMmsAttachments(message: InboxMessage.MMS) {
    message.attachments.forEach { att ->
        val decoded = att.data?.let {
            runCatching { Base64.decode(it, Base64.DEFAULT) }.getOrNull()
        } ?: return@forEach

        try {
            attachmentStorage.store(
                message.messageId,
                att.partId,
                att.name,
                att.contentType,
                decoded,
            )
        } catch (e: Exception) {
            logsService.insert(
                LogEntry.Priority.WARN,
                MODULE_NAME,
                "Failed to persist MMS attachment",
                mapOf(
                    "messageId" to message.messageId,
                    "partId" to att.partId,
                    "error" to (e.message ?: e.toString()),
                )
            )
        }
    }
}

Also applies to: 248-309

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt`
around lines 160 - 173, The MMS attachment persistence logic is currently
duplicated and only executed within the webhook-triggered branch, causing
downloaded attachments to be stored twice while non-webhook calls never create
downloadable files. Extract the attachment storage and error handling logic
(currently in the try-catch block with attachmentStorage.store and
logsService.insert) into a new private helper method called
persistMmsAttachments that accepts an InboxMessage.MMS parameter and iterates
over its attachments. Call this helper method once after message deduplication
for every MMS message, regardless of the triggerWebhooks flag, and remove the
attachment persistence code from inside the webhook-only conditional branch so
the webhook branch focuses only on emission.

113-125: ⚠️ Potential issue | 🔴 Critical

Constrain PDU path before any read or delete operation.

EXTRA_PDU_PATH is extracted from an exported broadcast intent without validation. The code reads the file at line 125 without any canonical path check, allowing an attacker to read arbitrary files. The delete guard at line 206-209 does canonicalize, but it permits deletion of any file under files/mms-in/—the same tree where persisted MMS attachments are stored. A spoofed callback can therefore delete legitimate attachment files.

Implement a dedicated temporary PDU directory separate from attachment storage and validate the resolved path before any file operation (exists(), readBytes(), or delete()).

🛡️ Suggested direction
-        val file = File(path)
+        val file = resolvePduFile(context, path) ?: return
         if (!file.exists()) {
             logsService.insert(
                 LogEntry.Priority.WARN,
@@
     private fun deletePduFile(context: Context, path: String?) {
         if (path.isNullOrBlank()) return
         runCatching {
-            val file = File(path).canonicalFile
-            val root = File(context.filesDir, "mms-in").canonicalFile
-            if (file.path.startsWith(root.path + File.separator)) {
+            val file = resolvePduFile(context, path) ?: return@runCatching
+            if (file.isFile) {
                 file.delete()
             } else {
                 logsService.insert(
@@
         }
     }
+
+    private fun resolvePduFile(context: Context, path: String): File? {
+        // Use a temp PDU directory that is distinct from MmsAttachmentStorage's
+        // persisted attachment root, and update the producer to write there too.
+        val root = File(context.filesDir, "mms-pdu").canonicalFile
+        val file = File(path).canonicalFile
+        if (file.parentFile?.canonicalFile != root) {
+            logsService.insert(
+                LogEntry.Priority.WARN,
+                MODULE_NAME,
+                "Refused MMS PDU path outside temp PDU directory",
+                mapOf("path" to path)
+            )
+            return null
+        }
+        return file
+    }

Update the producer to write PDU files to the new temp-only directory:

-                putExtra(EventsReceiver.EXTRA_PDU_PATH, file.absolutePath)
+                putExtra(EventsReceiver.EXTRA_PDU_PATH, File(context.filesDir, "mms-pdu").apply { mkdirs() }.resolve(file.name).absolutePath)

Also applies to: 203–209

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt`
around lines 113 - 125, The PDU path extracted from the EXTRA_PDU_PATH broadcast
intent is not validated before file operations, allowing arbitrary file reads
and deletions. Create a dedicated temporary PDU directory separate from the
attachment storage directory used for MMS files. Before any file operation
including exists(), readBytes() (at line 125), and delete() (at lines 206-209),
validate that the resolved canonical path remains within this temporary PDU
directory. This prevents both unauthorized reads of arbitrary files and prevents
deletion of legitimate attachment files stored in the mms-in directory. Update
the producer to write PDU files to the new temp-only directory, and add
canonical path validation checks at both the read operation and the delete
operation locations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt`:
- Around line 222-235: The logsService.insert calls in ReceiverService::process
are logging the full message object which can contain base64 attachment data and
message bodies from MMS messages, exposing sensitive media content in logs.
Replace the mapOf("message" to message) parameter in both logsService.insert
calls with metadata-only information that identifies the message (such as its ID
or type) without including the actual message content or attachment data. This
ensures sensitive message bodies and media are not persisted in debug logs.
- Around line 160-173: The MMS attachment persistence logic is currently
duplicated and only executed within the webhook-triggered branch, causing
downloaded attachments to be stored twice while non-webhook calls never create
downloadable files. Extract the attachment storage and error handling logic
(currently in the try-catch block with attachmentStorage.store and
logsService.insert) into a new private helper method called
persistMmsAttachments that accepts an InboxMessage.MMS parameter and iterates
over its attachments. Call this helper method once after message deduplication
for every MMS message, regardless of the triggerWebhooks flag, and remove the
attachment persistence code from inside the webhook-only conditional branch so
the webhook branch focuses only on emission.
- Around line 113-125: The PDU path extracted from the EXTRA_PDU_PATH broadcast
intent is not validated before file operations, allowing arbitrary file reads
and deletions. Create a dedicated temporary PDU directory separate from the
attachment storage directory used for MMS files. Before any file operation
including exists(), readBytes() (at line 125), and delete() (at lines 206-209),
validate that the resolved canonical path remains within this temporary PDU
directory. This prevents both unauthorized reads of arbitrary files and prevents
deletion of legitimate attachment files stored in the mms-in directory. Update
the producer to write PDU files to the new temp-only directory, and add
canonical path validation checks at both the read operation and the delete
operation locations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f4873861-0201-4bcb-89a7-237b5a533126

📥 Commits

Reviewing files that changed from the base of the PR and between 42cba0d and 56f4685.

📒 Files selected for processing (1)
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt (1)

65-70: ⚠️ Potential issue | 🟠 Major

Harden the status-broadcast trust boundary before accepting ACTION_MMS_SENT.

This dynamic receiver accepts custom status actions without sender validation. While explicit intents targeting EventsReceiver::class.java provide some protection, the hardcoded action strings are discoverable in the APK, and message IDs are enumerable. A malicious app could send spoofed broadcasts targeting the app's receiver and tamper with message state via processStateIntent, which updates state and triggers cleanup based only on EXTRA_MESSAGE_ID and resultCode with no verification of intent authenticity.

Consider securing this with: (1) non-exported dynamic registration flags where supported, and (2) a per-message nonce/token validated in processStateIntent before state mutation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt` around
lines 65 - 70, The dynamic receiver registration in EventsReceiver accepts
broadcast actions without sender validation, creating a security risk where
malicious apps can spoof broadcasts. Add the RECEIVER_NOT_EXPORTED flag to the
context.registerReceiver call (for Android 12+) to restrict which apps can send
intents to this receiver. Additionally, modify the processStateIntent method to
validate intent authenticity by implementing a per-message verification
mechanism (such as a nonce or token) that is checked before any state mutation
occurs based on EXTRA_MESSAGE_ID and resultCode, ensuring only legitimate status
broadcasts from the system are processed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt`:
- Around line 36-38: The persistMmsAttachments method is being called before the
message is inserted into the repository, causing orphaned attachment files if
the database insert fails. Move the persistMmsAttachments call to execute after
the repository.insert operation completes successfully. This ensures attachments
are only persisted when the corresponding inbox message row is guaranteed to
exist in the database. Apply this ordering fix to all MMS attachment persistence
operations in the file, including the location mentioned at lines 49-63.
- Around line 68-73: The Base64 decoding operation in the
IncomingMessagesService uses runCatching with getOrNull() which silently
converts decode failures to null with no logging, making attachment decode
errors invisible for debugging. Modify the decoded variable assignment to
capture and log decode failures by adding a failure handler to the runCatching
block that logs the exception when Base64.decode fails, ensuring attachment
decode errors are tracked and visible in the application logs.

---

Outside diff comments:
In `@app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt`:
- Around line 65-70: The dynamic receiver registration in EventsReceiver accepts
broadcast actions without sender validation, creating a security risk where
malicious apps can spoof broadcasts. Add the RECEIVER_NOT_EXPORTED flag to the
context.registerReceiver call (for Android 12+) to restrict which apps can send
intents to this receiver. Additionally, modify the processStateIntent method to
validate intent authenticity by implementing a per-message verification
mechanism (such as a nonce or token) that is checked before any state mutation
occurs based on EXTRA_MESSAGE_ID and resultCode, ensuring only legitimate status
broadcasts from the system are processed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b6ad32e-e997-4eab-856d-cade3ab21b0c

📥 Commits

Reviewing files that changed from the base of the PR and between 56f4685 and ef44b22.

📒 Files selected for processing (3)
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt

@capcom6 capcom6 force-pushed the mms/extended-support branch from 36c029e to cc0d93f Compare June 17, 2026 13:14

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/src/main/assets/api/swagger.json (1)

2272-2274: ⚡ Quick win

Document 404 error response body for consistency.

Other endpoints (e.g., DELETE /devices/{id} at lines 1896-1904) return a structured ErrorResponse schema for 404 errors, but this endpoint's 404 response has no content schema defined.

📋 Proposed fix for consistent error handling
          "404": {
-            "description": "Attachment not found"
+            "description": "Attachment not found",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "`#/components/schemas/smsgateway.ErrorResponse`"
+                }
+              }
+            }
          }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/assets/api/swagger.json` around lines 2272 - 2274, The 404
response for this endpoint in the swagger.json file currently only includes a
description without defining a response body schema. To fix this, add a content
schema definition to the 404 response object (at lines 2272-2274) that
references the ErrorResponse schema, matching the same pattern used in other
endpoints like the DELETE /devices/{id} endpoint (referenced around lines
1896-1904). Include the appropriate media type (application/json) and schema
reference to ensure consistent error response documentation across all
endpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 2272-2274: The 404 response for this endpoint in the swagger.json
file currently only includes a description without defining a response body
schema. To fix this, add a content schema definition to the 404 response object
(at lines 2272-2274) that references the ErrorResponse schema, matching the same
pattern used in other endpoints like the DELETE /devices/{id} endpoint
(referenced around lines 1896-1904). Include the appropriate media type
(application/json) and schema reference to ensure consistent error response
documentation across all endpoints.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1d35b668-84f7-4170-8d3f-51b17388caae

📥 Commits

Reviewing files that changed from the base of the PR and between 36c029e and cc0d93f.

📒 Files selected for processing (45)
  • README.md
  • app/src/main/AndroidManifest.xml
  • app/src/main/assets/api/swagger.json
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/StateStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/res/values-zh/strings.xml
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/res/xml/root_preferences.xml
  • docs/MMS.md
✅ Files skipped from review due to trivial changes (4)
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • README.md
  • app/src/main/res/values-zh/strings.xml
  • docs/MMS.md
🚧 Files skipped from review as they are similar to previous changes (38)
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/res/xml/root_preferences.xml
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/StateStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java

@capcom6 capcom6 force-pushed the mms/extended-support branch from cc0d93f to 9744a44 Compare June 22, 2026 01:23

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/src/main/assets/api/swagger.json (1)

43-65: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider adding validation to ensure MMS has content.

smsgateway.MmsMessage has no required fields—a request could technically pass validation with null subject, null text, and an empty/missing attachments array, resulting in an empty MMS. Consider adding a minItems: 1 on attachments or using schema composition to ensure at least one of subject, text, or attachments is provided.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/assets/api/swagger.json` around lines 43 - 65, The
smsgateway.MmsMessage schema currently has no required fields, allowing empty or
null values for all properties. To ensure valid MMS content, add either a
minItems constraint of 1 to the attachments array property to guarantee at least
one attachment is present, or alternatively use schema composition with
oneOf/anyOf to enforce that at least one of the subject, text, or attachments
fields must be provided with non-null values. Choose the approach that best fits
the application's requirements for MMS message validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 43-65: The smsgateway.MmsMessage schema currently has no required
fields, allowing empty or null values for all properties. To ensure valid MMS
content, add either a minItems constraint of 1 to the attachments array property
to guarantee at least one attachment is present, or alternatively use schema
composition with oneOf/anyOf to enforce that at least one of the subject, text,
or attachments fields must be provided with non-null values. Choose the approach
that best fits the application's requirements for MMS message validation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e7ac69ba-3152-4397-818e-27eda9ccc932

📥 Commits

Reviewing files that changed from the base of the PR and between cc0d93f and 9744a44.

📒 Files selected for processing (45)
  • README.md
  • app/src/main/AndroidManifest.xml
  • app/src/main/assets/api/swagger.json
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/StateStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/res/values-zh/strings.xml
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/res/xml/root_preferences.xml
  • docs/MMS.md
✅ Files skipped from review due to trivial changes (4)
  • README.md
  • app/src/main/res/xml/root_preferences.xml
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/res/values-zh/strings.xml
🚧 Files skipped from review as they are similar to previous changes (40)
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/StateStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • docs/MMS.md
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java

@capcom6 capcom6 force-pushed the mms/extended-support branch from 9744a44 to eb09679 Compare June 24, 2026 02:32

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/src/main/assets/api/swagger.json (1)

2272-2274: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add an error body schema to the 404 response for consistency.

The 400 and 401 responses reference smsgateway.ErrorResponse, but 404 returns no content. Aligning it keeps error handling consistent for clients.

♻️ Proposed change
           "404": {
-            "description": "Attachment not found"
+            "description": "Attachment not found",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "`#/components/schemas/smsgateway.ErrorResponse`"
+                }
+              }
+            }
           }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/assets/api/swagger.json` around lines 2272 - 2274, The `404`
response definition for the Attachment not found endpoint is missing a schema
reference, while the `400` and `401` responses properly reference the
smsgateway.ErrorResponse schema. Add a schema property to the `404` response
object that references smsgateway.ErrorResponse, matching the pattern used in
the `400` and `401` response definitions to ensure consistent error response
structure across all error statuses.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/src/main/assets/api/swagger.json`:
- Around line 2272-2274: The `404` response definition for the Attachment not
found endpoint is missing a schema reference, while the `400` and `401`
responses properly reference the smsgateway.ErrorResponse schema. Add a schema
property to the `404` response object that references smsgateway.ErrorResponse,
matching the pattern used in the `400` and `401` response definitions to ensure
consistent error response structure across all error statuses.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c8c2d963-e24b-4dd5-adb1-1bcdb043e4a7

📥 Commits

Reviewing files that changed from the base of the PR and between 9744a44 and eb09679.

📒 Files selected for processing (44)
  • README.md
  • app/src/main/AndroidManifest.xml
  • app/src/main/assets/api/swagger.json
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/res/values-zh/strings.xml
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/res/xml/root_preferences.xml
  • docs/MMS.md
✅ Files skipped from review due to trivial changes (7)
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduContentTypes.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/GenericPdu.java
  • README.md
  • app/src/main/res/xml/root_preferences.xml
  • app/src/main/res/values-zh/strings.xml
  • docs/MMS.md
🚧 Files skipped from review as they are similar to previous changes (35)
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/InvalidHeaderValueException.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/MmsMessage.kt
  • app/src/main/res/xml/file_paths.xml
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MmsException.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSParser.kt
  • app/src/main/java/me/capcom/smsgateway/App.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesRepository.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/CharacterSets.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/WebService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/Module.kt
  • app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayApi.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MessagesReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduBody.java
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/domain/messages/PostMessageRequest.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/MultimediaMessagePdu.java
  • app/src/main/java/me/capcom/smsgateway/receivers/EventsReceiver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/SendReq.java
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/MmsContentObserver.kt
  • app/src/main/java/me/capcom/smsgateway/modules/messages/MessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/InboxRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/data/entities/Message.kt
  • app/src/main/java/me/capcom/smsgateway/modules/receiver/parsers/MMSRetrieveParser.kt
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/me/capcom/smsgateway/domain/MessageContent.kt
  • app/src/main/java/me/capcom/smsgateway/modules/localserver/routes/MessagesRoutes.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduComposer.java
  • app/src/main/java/me/capcom/smsgateway/modules/incoming/IncomingMessagesService.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduPart.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/EncodedStringValue.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/PduHeaders.java
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsAttachmentStorage.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/MmsSender.kt
  • app/src/main/java/me/capcom/smsgateway/modules/mms/pdu/aosp/ContentType.java

@capcom6 capcom6 force-pushed the mms/extended-support branch from eb09679 to 07f1b2d Compare June 24, 2026 03:52
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open for 7 days with no activity.

@github-actions github-actions Bot added the stale label Jul 2, 2026
@capcom6 capcom6 self-assigned this Jul 2, 2026
@capcom6 capcom6 removed the stale label Jul 2, 2026
fluxism and others added 2 commits July 8, 2026 07:55
Add complete MMS handling without requiring default SMS app role:

Send path:
- PduComposer from AOSP for building MMS PDUs
- MmsSender dispatches via platform SmsManager.sendMultimediaMessage
- Outgoing PDU cleanup after send result

Receive path:
- MmsReceiver intercepts WAP_PUSH_RECEIVED and parses M-Notification.ind
- ReceiverService orchestrates MMS retrieval via carrier MMSC
- MMSRetrieveParser decodes retrieved M-Retrieve.conf PDUs
- MmsAttachmentStorage persists parts under files/mms-in/<hash>/

Inbox & API:
- SmsContentObserver polls content://sms for incoming SMS (replaces
  BroadcastReceiver path that carriers can suppress)
- MmsContentObserver watches content://mms for platform-downloaded MMS
- InboxRoutes serves attachment bytes via GET /inbox/{id}/attachments/{partId}
- Webhook payloads include inline base64 attachment data

Infrastructure:
- PingForegroundService made idempotent to avoid crash on rapid restarts
- StateStorage tracks high-water marks for content observers
@capcom6 capcom6 force-pushed the mms/extended-support branch from 07f1b2d to 883ee5d Compare July 8, 2026 00:56
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