Add support for RFCs 5530 and 6855. #8772
Conversation
|
We need to make sure mailbox names are properly decoded. My understanding of RFC 6855 is that LIST responses won't use the modified UTF-7 encoding for mailbox names when The RFC text reads ambiguous to me (emphasis mine):
That makes it sound like the server can use mUTF-7 or UTF-8 and the client has to deal with it. However, I don't see how a client could distinguish a mUTF-7 encoded string from a UTF-8-encoded string that happens to also be a valid, non-trivial mUTF-7 encoding. |
|
Excellent point. I don't think that was raised at all during the work on 653x/685x. I think it warrants a late change to 6855bis... I'll try to make the document conform to the majority of client code. |
|
We discussed the proper course of action for RFC 9755 (which was published this week, after a very long auth48 phase, guess why). The general opinion is that if you enable utf8, utf8 is what you get and what you need to send. I updated this PR accordingly. I have a feeling that I'll need to check a lot of software. |
|
I force-pushed again because I forgot to run The code looks mostly okay in my eyes, except that I found the testing on COPY/MOVE a little sparse. Some people prefer orthogonal testing, others like a little redundancy. I tried do match your style. Let me know if I missed. |
|
@arnt Thanks, I'm not too familiar with the addressed RFCs yet. But from a high level view, it looks good. We want to avoid adding more Java code to the project and maybe you could convert it? Otherwise just ping me and I'll patch this pull-request. |
|
That's about EnabledResponse, right? I converted it and force-pushed now. Hope I got it right, I'm not really fluent in Kotlin. Looks like a really nice language, but I still don't know it well enough to write elegant new code from scratch. (BTW, while I'll try to install and test properly on-device, I haven't for this force-push. Realistic on-device testing requires the tip of mime4j, plus some SMTP code that I have locally.) |
|
The remaining failures are in PGP code, which I didn't touch and don't aspire to fix. |
|
Hi, would you mind reviewing again? I'd really like to get to the testable stage. |
|
Hi @arnt, apologies we're a bit short staffed right now. Wolf is away for a bit, cketti is out, and new Android team member(s) are still onboarding. I'll see what we can do here, but worst case it might have to wait until Wolf is back in May. |
|
Makes sense, thanks for explaining. |
2db6a43 to
1c1dcf3
Compare
5530 (ENABLE) lets a client tell the server which extensions it would like to use, and lets the server tell the client which extensions is has enabled. Most IMAP extensions don't need to be enabled, but UTF8=ACCEPT does. 9755 (UTF8=ACCEPT) requires four things of clients: 1. The client uses ENABLE (added with tests) 2. The client accepts UTF8 strings (worked already, this adds testing) 3. The client cannot use certain SEARCH syntax (tb already did not) 4. The client uses UTF8 for folder names instead of mUTF7 This commit contains a couple of unit tests that do nothing right now, they merely guard against possible future breakage.
|
Thanks. Wonder why that test broke. I'm a little uncertain. A new mime4j release is necessary for real thing, but as I recall this part of the code (#8772 as opposed to #8770) ought to work without it. It won't really do anything, of course — mail from my friends at 中国互联网络信息中心.中国 will now be rejected after the IMAP stage instead of by the IMAP stage. (#8772 is just one part. cketti preferred to change one subsystem at a time, for a progression of states where nothing is broken that worked before, and then finally step to a state where the new functionality is user-visible and usable. That final step needs a new mime4j release.) |
|
@arnt Okay, then this is good to go. Test test broke as the imap connection now always tries to call enable, but the mock wasn't setup for these additional steps. |
|
Thanks for your contribution! Your pull request has been merged and will be part of Thunderbird 12. We appreciate the time and effort you put into improving Thunderbird. If you haven’t already, you’re welcome to join our Matrix chat for contributors. It’s where we discuss development and help each other out. https://matrix.to/#/#tb-android-dev:mozilla.org |
|
Oh great, thanks! What next, SMTP? I'll take me a few days, I have to review a 72-page document and am forcing myself to write no code until the review is done. |
RFC 6855/9755 permits IMAP servers to send UTF8 in strings such as "this", but
does not ban sending literals such as {4}
this
instead, and some servers do, and thunderbird#8772 did not handle that.
Fixes thunderbird#9212
RFC 6855/9755 permits IMAP servers to send UTF8 in strings such as "this", but
does not ban sending literals such as {4}
this
instead, and some servers do, and thunderbird#8772 did not handle that.
Fixes thunderbird#9212
5530 (ENABLE) lets a client tell the server which extensions it would like to use, and lets the server tell the client which extensions is has enabled. Most IMAP extensions don't need to be enabled, but UTF8=ACCEPT does.
6855 (UTF8=ACCEPT) requires three things of clients:
6855 also allows some optimisations that this change does not contain:
This change doesn't do either of those, because they add complexity (and unit tests) but no functionality. Some IMAP commands become three or seven or even ten bytes smaller. Who cares?
RFC 6855 differs modestly from its replacement, currently in the RFC-editor's queue. This implementation sides with the newer RFC when there's any difference.
This commit contains a couple of unit tests that do nothing right now, they merely guard against possible future breakage.