chibenwa commented on code in PR #2386:
URL: https://github.com/apache/james-project/pull/2386#discussion_r1726641331
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractMailboxProcessor.java:
##########
@@ -444,14 +444,10 @@ protected Optional<MessageRange>
messageRange(SelectedMailbox selected, IdRange
private MessageRange msnRangeToMessageRange(SelectedMailbox selected, long
lowVal, long highVal)
throws MessageRangeException {
- // Take care of "*" and "*:*" values by return the last message in
+ // Take care of "*", "1:*" and "*:*" values by return the last message
in
// the mailbox. See IMAP-289
- if (lowVal == Long.MAX_VALUE && highVal == Long.MAX_VALUE) {
- Optional<MessageUid> last = selected.getLastUid();
- if (!last.isPresent()) {
- throw new MessageRangeException("Mailbox is empty");
- }
- return last.get().toRange();
+ if ((lowVal == Long.MAX_VALUE || lowVal == 1) && highVal ==
Long.MAX_VALUE) {
+ return MessageRange.all();
}
Review Comment:
We got a new bug here!
CF https://datatracker.ietf.org/doc/html/rfc3501 ABNF
```
seq-number = nz-number / "*"
; message sequence number (COPY, FETCH, STORE
; commands) or unique identifier (UID COPY,
; UID FETCH, UID STORE commands).
; * represents the largest number in use. In
; the case of message sequence numbers, it is
; the number of messages in a non-empty mailbox.
; In the case of unique identifiers, it is the
; unique identifier of the last message in the
; mailbox or, if the mailbox is empty, the
; mailbox's current UIDNEXT value.
; The server should respond with a tagged BAD
; response to a command that uses a message
; sequence number greater than the number of
; messages in the selected mailbox. This
; includes "*" if the selected mailbox is empty.
```
`1:*` -> Messages from first to last IE `MessageRange.all() (we are good)
But `*:*` means range from last message to last message IE
`last.get().toRange()`
So we actually need two separate ifs: one handling `1:*` and one handling
`*:*`
Also we MAY which:
- [ ] To ensure MPT tests `C: A0 FETCH *:*`
- [ ] To test the behaviour of `C: A0 UID FETCH *:*`
- [ ] Technically speacking a lone `seq-number` is a valid `sequence-set`
so we may which to ensure `C: A0 FETCH *` and `C: A0 UID FETCH *` are nicely
handled - and tested.
--
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]
