Handle UTF8 strings sent using literal syntax

[arnt]

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 #8772 did not handle that.

Fixes #9212

[ke46138]

Hmm. I built k9 and thunderbird from this pr, but the bug https://github.com/thunderbird/thunderbird-android/issues/9212 remains.

[arnt]

I would guess that's due to a weakness in IMAP: Clients have to cache folder names and guess how long to cache them. There's no mechanism to signal that a refresh is in order. The names will look wrong for a while, short or long. But again, that's guesswork.

But I'll test with my test phone on Monday, during normal working hours.

[arnt]

I'm running this PR now on a test phone. It's still using the cached folder name and will do so for at least half an hour (the value of FOLDER_LIST_STALENESS_THRESHOLD), not sure whether there are other conditions that may extend the cache lifetime. I'll just give it a few hours and see.

[ke46138]

I doubt it has anything to do with caching (if you uninstalled and installed thunderbird). I uninstalled thunderbird and installed the app I built from this pr, added an account and still the bug #9212 remains. I remind you that the bug only appears if you uninstall and install the application (or if you delete the account and create it again), and if you haven't done that, thunderbird uses cached folder names and the bug doesn't appear. I've been testing thunderbird for a few days.

[arnt]

You're right, all that the caching does is slow down my debugging. The mailbox name still wrong even after it issues the IMAP command that I think should fix it (the unit test I included in this PR is copied from a server log). I'll work on it again tomorrow.

[rafaeltonholo]

@arnt, the problem is in the RealImapStore.getFolderDisplayName(serverId): String method / FolderNameCodec class. The server is correctly returning the UTF-8 name (you can observe that via logcat). However, when we try to generate the folder display name, we mess up the encoding.

I guess that will help you with your debugging. Let me know if you need anything else 😊

[arnt]

That took a while, most of it trying to write a unit test. The worst part of my job is when I switch from one unit testing framework to another ;)

There were several things.

• As @rafaeltonholo correctly noticed, getFolderDisplayName() is wrong, which confused me (it wasn't a problem when I first wrote this code — long story). Today's commit should fix that. Thanks @rafaeltonholo.
• createImapResponse() couldn't support UTF8=ACCEPT, which today's commit fixes but I don't like the fix.
• parsing folder names sent by the server as literals (instead of as strings) broke. This caused problems with servers that support UTF8=ACCEPT but avoid using it for output (which makes sense if the server's output code can't see the capabilities). The first commit fixes this.

[arnt]

Wat. I tested that. Can you name a specific folder name and server that broke?

I'm on vacation this week. I'll do my best not to work on this until I'm back (in seven days and a few hours from now). I'm extremely curious though.

[arnt]

Hm. I tested it with a branch that contains quite a bit more code than #8772.

[rafaeltonholo]

Wat. I tested that. Can you name a specific folder name and server that broke?

I'm on vacation this week. I'll do my best not to work on this until I'm back (in seven days and a few hours from now). I'm extremely curious though.

I have used the same name you have in one of your unit tests: Исходящие, but it also fails in all the following cases:

• Ações
• Áêà
• Արխիվ
• أرشيف
• পুৰালেখ
• ⴰⵎⵢⴰⴳ
• ማህደር
• 档案

As you can see in the following screenshot, although the name is fixed, the server_id isn't, causing the issue of not being able to execute actions that are related to that folder.

---

For example, when trying to move a message from INBOX to Исходящие, the app moves the message locally (which is also wrong, but another issue) but fails to execute the move command in the remote:

14:27:59.057 RealImapConnection      V  conn147206245>>> 17 UID MOVE 153 "?????????"
14:27:59.161 ImapResponseParser      V  conn147206245<<<#17# [NO, [TRYCREATE], The destination mailbox could not be found.]
14:27:59.172 MessagingController     E  Failure of command 'move_or_copy' was permanent, removing command
      com.fsck.k9.mail.store.imap.NegativeImapResponseException: Command: UID MOVE 153 "?????????"; response: #17# [NO, [TRYCREATE], The destination mailbox could not be found.]
      at com.fsck.k9.mail.store.imap.ImapResponseParser.readStatusResponse(ImapResponseParser.java:126)
      at com.fsck.k9.mail.store.imap.RealImapConnection.executeSimpleCommand(RealImapConnection.kt:742)
      at com.fsck.k9.mail.store.imap.RealImapConnection.executeSimpleCommand(RealImapConnection.kt:728)
      at com.fsck.k9.mail.store.imap.RealImapConnection.executeCommandWithIdSet(RealImapConnection.kt:765)
      at com.fsck.k9.mail.store.imap.RealImapFolder.moveMessagesUsingMoveExtension(RealImapFolder.kt:346)
      at com.fsck.k9.mail.store.imap.RealImapFolder.moveMessages(RealImapFolder.kt:327)
      at com.fsck.k9.backend.imap.CommandMoveOrCopyMessages.moveOrCopyMessages(CommandMoveOrCopyMessages.kt:59)
      at com.fsck.k9.backend.imap.CommandMoveOrCopyMessages.moveMessages(CommandMoveOrCopyMessages.kt:15)
      at com.fsck.k9.backend.imap.ImapBackend.moveMessages(ImapBackend.kt:96)
      at com.fsck.k9.controller.MessagingController.processPendingMoveOrCopy(MessagingController.java:921)
      at com.fsck.k9.controller.MessagingController.processPendingMoveOrCopy(MessagingController.java:885)
      at com.fsck.k9.controller.MessagingControllerCommands$PendingMoveOrCopy.execute(MessagingControllerCommands.java:67)
      at com.fsck.k9.controller.MessagingController.processPendingCommandsSynchronous(MessagingController.java:744)
      at com.fsck.k9.controller.MessagingController$3.run(MessagingController.java:714)
      at com.fsck.k9.controller.MessagingController.runInBackground(MessagingController.java:234)
      at com.fsck.k9.controller.MessagingController.-$$Nest$mrunInBackground(Unknown Source:0)
      at com.fsck.k9.controller.MessagingController$1.run(MessagingController.java:174)
      at java.lang.Thread.run(Thread.java:1012)

[arnt]

Have the bug fixed now. Wonder whether the code may have worked a while ago, but the corruption persisted via the LocalFolder database… anyway, I'll cut down the code to what's really necessary and submit an update tomorrow. The time is 21:45 here and the wise developer leaves the office.

[kewisch]

Thanks so much @arnt, really appreciate the persistence here!

[arnt]

But another working day has finished, I need to leave five minutes ago, and I still haven't send in the changed code. Need several hours to biject and be sure, and and that time is time I've spent in googledocs instead of here.

[rafaeltonholo]

Hi @arnt, have you had a chance to review the comments above?

[arnt]

Sorry about the latency. I ran out of headspace, there's a limit to how many programming languages, testing frameworks etc I can handle at the same time.

I don't understand when 'gradle testFullDailyUnitTest' runs all tests and when it doesn't. I rebased and now it shows some errors that I didn't see when I wrote the code. I'll deal with them now.

[rafaeltonholo]

Hi @arnt, a unit test is failing. Can you fix it, or would you like me to resolve it so we can merge the changes?

[arnt]

Oops, I misread and didn't notice that. I get many unit test failures, all except one mention that lateinit property logger has not been been initialized and I didn't notice that there's an error has a different cause.

Let me look now.

BTW I'm not patient ­— you are.

[arnt]

The unit test failure appears to be a bug in the unit test. It needs to do a little more mocking. I'm unlikely to get to it tonight, but perhaps during the weekend.

[rafaeltonholo]

Hi @arnt, looks like there is a disconnection between what you are mocking in the unit test and what the method does itself.

In the test case, you are mocking the canSendUTF8QuotedStrings() function to return true, however, the method itself never uses that function, but relies on the FolderNameCode.acceptUtf8Encoding property to encode or not the folder's name.

[arnt]

Hi,

I found out that at the same time, fixed it, and pushed just now. Getting the code to compile on my 16GB laptop was bothersome, so I have not been able to test this on a real device. The unit tests pass (except for the lateinit errors, which seem unrelated to my code).

The code now assumes that if Thunderbird has several IMAP connections to the server, then they have the same capabilities. I'm not 100% sure, but I think I saw a similar assumption elsewhere.

[rafaeltonholo]

Thank you for this contribution!

[coreycb]

Hi, This is a large patch that I think could use some time on daily before we uplift. We can cut another beta next Monday and include this if testing goes well on daily. For now, I'm going to remove the beta uplift label.

[pagebug]

https://github.com/thunderbird/thunderbird-android/releases/tag/THUNDERBIRD_12_0b2 Where is the APK file?

[coreycb]

@pagebug I failed to push uplifts for 12.0b2 so there's nothing new in it. There's a 12.0b3 with uplifts and release notes. We don't have this UTF-8 fix in beta yet. We're aiming for getting it into 12.0b4 next Monday.