Hi,
On 21/08/2024 10:09, Benoit TELLIER wrote:
Hello Wojtek,
I would expect UID FETCH 1:* XXX on an empty mailbox to return OK
Kinda:
C: A0 UID FETCH 1:* UID
S: A0 OK FETCH completed.
Thank you
The way to go regarding non compliance to IMAP syntax/grammar/logic would be:
- Try to get the exact IMAP exchange. Evolution is especially handy regarding
this as allows turning on advanced IMAP debugging that actually drops the IMAP
conversation to a log file.
Indeed: https://gitlab.gnome.org/GNOME/evolution/-/wikis/Debugging: `CAMEL_DEBUG=imapx:io evolution
>& logfile`
- That exchange can actually be reused in order to write a MPT test that actually acts
as an "integration test"
This is veeeery conveniet
- Then we need to address the faulty logic in protocols/imap . Unit tests may
be written if relevant.
- Open a JIRA ticket to track the defect
https://issues.apache.org/jira/browse/JAMES-4060
- Open a PR referencing the JIRA issue in the title + commit messages (starts
with JAMES-XXXX) for automatic linking
Here, it seems rather straightforward, here is the MPT output I came up with
that reproduces the issue (sorry I was curious!) :
https://github.com/apache/james-project/pull/2385
A followup PR build upon yours due to GH limitations:
https://github.com/apache/james-project/pull/2386
Regarding the fix, I believe it is rather the way FetchProcessor handles an
empty range.
- currently it considers empty ranges as bad input. While it MAY be
legitimate with commands that alter state (COPY MOVE STORE EXPUNGE REPLACE) it
is definitly inappropriate for FETCH.
- It likely rather should skip the given range rather than throwing an
exception in FetchProcessor line 205.
- (in the recent IMAP work, we mostly ported the existing code without
questionning its actual logic and we likely missed that one-line test coverage
to uncover this issue).
Do you want to get a look at the issue?
Tried my best, let's see how you deem the result :)
Ps: SELECT is mandatory to work with a folder content. TB is necessarily doing
it, though it may be clever enough to infer from the SELECT command (or the
STATUS counters) that the folder is empty and thus that opening it may not be
needed.
Indeed TB is smarter and after select and empty mailbox it doesn't issue FETCH (it's also possible
to debug TB in the same way as Evolution it turns out).
it would be handy if the log would contain complete received command, even
for learning/understanding better what's going on/what was received (for
example UID fetch lacks actual parameters)
Regarding actual IMAP logging...
The MDC logging context is set with precious debugging information including
user supplied parameters. You may need to configure your logging output to
include MDC to benefit from it. Would it fit your needs?
Hmm... it's not set everywhere, and especially missing for IMAP commands IMHO (but can't see nice
way to improve it)
I agree the "GOT Tag" messages are useless and may be removed.
I made a couple of tweaks to the commands output and removed individual ones in favour of the
"final" one which IMHO has a bit more sense and is more informative.
AbstractProcessor (org.apache.james.imap.processor.base.AbstractProcessor) have
a debug log logging the command along side the associated MDC, which shall be
enough... Though this is the interpretation of IMAP exchanges by James and not
the actual IMAP exchanges.
However that is true that I did often ended up tweaking the IMAP network layer
to extract the IMAP exchange. Having a standard ready to use way to do this
would be handy I believe.
The IMAP parsers are read ahead (reads data as needed) thus getting the exact
user input is tricky, but if needed we could add traces onto
NettyImapRequestLineReader and drop a copy of the buffer as a log. For what
james sends we sould put a trace into ChannelImapResponseWriter:write. In oth
case we shall beware of eager evaluation of the logging input as computing the
traces content may be expensive.
Do you want to get a shot at this too?--
Best regards,
Benoit TELLIER
Yeah, outputting socket data in case of IMAP seems quite daunting as each command/processor can read
more data as it needs. Though for my needs and fixing this issue the exchange I got seems enough.
And there is still possibility to get the complete exchange from the client perspective
wk.
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
