prosgarz35 opened a new pull request, #3236:
URL: https://github.com/apache/james-project/pull/3236
## Summary
This pull request addresses several RFC compliance issues, race conditions,
and determinism inconsistencies found during protocol verification of SMTP and
JMAP RFC-8621 implementations.
---
## Changes
### 1. SMTP: Remove extraneous trailing parenthesis in EHLO greeting (RFC
5321 §4.1.1.1)
- **File:**
`protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/EhloCmdHandler.java`
- **Issue:** The EHLO greeting response string formatted the client remote
address with an unbalanced closing parenthesis `])` instead of `]`:
```
250 hostname Hello client [192.168.1.1])
```
- **Fix:** Changed `.append("])")` to `.append("]")`, conforming strictly to
RFC 5321 §4.1.1.1 syntax.
---
### 2. JMAP RFC-8621: Ensure atomicity in `Email/set update` and eliminate
race window (RFC 8621 §3)
- **File:**
`server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSetUpdatePerformer.scala`
- **Issue:** Previously, `updateSingleMessage` only used atomic execution
when both mailbox IDs and flags were updated simultaneously (`isMailboxUpdate
&& isFlagUpdate`). In the `else` branch, flag updates and mailbox ID updates
were executed in a chained non-atomic fashion (`updateFlags(...).flatMap(... =>
updateMailboxIds(...))`), introducing a partial-update failure window and
potential data race between concurrent updates.
- **Fix:**
- Branching is now strictly separated into dedicated, atomic store
operations:
- Combined update: calls `messageIdManager.updateEmail(...)` (single
atomic operation).
- Flags-only update: calls `messageIdManager.setFlagsReactive(...)`
directly.
- Mailbox-only update: calls
`messageIdManager.setInMailboxesReactive(...)` directly.
- Removed redundant non-atomic helper methods `updateFlags` and
`updateMailboxIds`.
---
### 3. JMAP RFC-8621: Deterministic message copy selection for multi-mailbox
emails (RFC 8621 §4.1)
- **File:**
`server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Email.scala`
- **Issue:** When an email exists across multiple mailboxes, `message._2`
contains a `Seq[MessageResult]` whose order depends on the backend store driver
and is not guaranteed. View factories (`EmailMetadataViewFactory`,
`EmailHeaderViewFactory`, `EmailFullViewFactory`, `EmailFastViewReader`, and
`EmailFastViewWithAttachmentsMetadataReader`) previously called `.headOption`,
which could lead to non-deterministic selection of the representative message
metadata (such as `size` and `receivedAt`).
- **Fix:** Added `Email.pickFirstMessage(messages: Seq[MessageResult])`
which deterministically sorts the message results by internal date timestamp
and mailbox ID (`messages.sortBy(m => (m.getInternalDate.getTime,
m.getMailboxId.serialize())).headOption`). All view factories now use this
deterministic selection helper.
---
### 4. JMAP RFC-8621: Fix typo in `validateTextBody` error message
- **File:**
`server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSet.scala`
- **Issue:** `validateTextBody` raised an `IllegalArgumentException` stating
`"Expecting htmlBody type to be text/html"` when validating a non-`text/plain`
part due to copy-paste from `validateHtmlBody`.
- **Fix:** Updated the error message to accurately state `"Expecting
textBody type to be text/plain"`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
