MURDER frontend doesn't relays GETMETADATA command correctly to backend

[yaonos]

Description of the issue

While preparing an upgrade from Cyrus-IMAP 2.4.22 to 3.4.5, I discovered an issue with cyradm's info command that does not return any information appart from the backend name when executed on a frontend. There is no issue when this command is executed on a backend.

Diagnosis

While trying to find a solution to this issue, I discovered that the GETMETADATA command that is sent to the frontend is relayed as a GETANNOTATION command to the backends with the GETMETADATA parameters kept as they were received instead of beeing converted to GETANNOTATION compatible parameters.

Here is an example of what is received by the frontend from the cyradm's info command :

---------- cyrus Thu Mar 16 20:57:38 2023

<1678996664<4 GETMETADATA "user.tst" ("/private/*")
>1678996664>4 OK Completed
<1678996664<5 GETMETADATA "user.tst" ("/shared/*")
>1678996664>* METADATA user.tst ("/shared/vendor/cmu/cyrus-imapd/server" "backend_fqdn")
5 OK Completed

And here is what is proxied to the backend by the frontend :

---------- cyrus Thu Mar 16 20:57:44 2023

<1678996664<C01 CAPABILITY
>1678996664>* CAPABILITY IMAP4rev1 LITERAL+ ID ENABLE ACL ANNOTATE-EXPERIMENT-1 BINARY CATENATE CHILDREN CONDSTORE CREATE-SPECIAL-USE ESEARCH ESORT LIST-EXTENDED LIST-MYRIGHTS LIST-STATUS MAILBOX-REFERRALS METADATA MOVE MULTIAPPEND NAMESPACE OBJECTID QRESYNC QUOTA RIGHTS=kxten SAVEDATE SEARCH=FUZZY SORT SORT=DISPLAY SPECIAL-USE STATUS=SIZE THREAD=ORDEREDSUBJECT THREAD=REFERENCES UIDPLUS UNSELECT URLAUTH URLAUTH=BINARY WITHIN ANNOTATEMORE DIGEST=SHA1 LIST-METADATA NO_ATOMIC_RENAME PREVIEW=FUZZY SCAN SORT=MODSEQ SORT=UID THREAD=REFS X-CREATEDMODSEQ X-REPLICATION XLIST XMOVE MUPDATE=mupdate://frontend_fqdn/ LOGINDISABLED AUTH=DIGEST-MD5 AUTH=PLAIN UNAUTHENTICATE COMPRESS=DEFLATE X-QUOTA=STORAGE X-QUOTA=MESSAGE X-QUOTA=X-ANNOTATION-STORAGE X-QUOTA=X-NUM-FOLDERS IDLE
C01 OK Completed
<1678996664<PROXY0 GETANNOTATION "user.tst" (/private/*)
>1678996664>PROXY0 BAD Missing annotation entry
<1678996664<N01 NOOP
>1678996664>N01 OK Completed
<1678996664<PROXY1 GETANNOTATION "user.tst" (/shared/*)
>1678996664>PROXY1 BAD Missing annotation entry
<1678996666<Q01 LOGOUT
>1678996666>* BYE LOGOUT received
Q01 OK Completed

Resolution proposal

I could correct this issue by replacing the GETANNOTATION command sent to the backend in annotate_fetch_proxy() by the GETMETADATA command. I join my patch to this issue in order for you to review this solution. I hope that it does not introduce any new issue.

cyrus-imapd_getmetadata_proxy.patch

[elliefm]

Thanks

@ksmurchison @rsto: what do you make of this? I'm looking at imap_proxy.c and it looks inconsistent in its handling of [GS]ET(METADATA|ANNOTATION)...

• annotate_fetch_proxy() just always uses GETANNOTATION, as @yaonos reports
• annotate_store_proxy() just always uses SETMETADATA
• proxy_part_filldata() looks like it chooses between GETMETADATA/GETANNOTATION by checking whether the backend supports CAPA_METADATA

So that's three different shapes of implementation. I feel like they should converge towards a single choice...

Looks like we already tried to converge towards just always using [GS]ETMETADATA in 43c9f84, but the crucial line of annotate_fetch_proxy() was accidentally missed? Which is then fixed by the patch in OP.

I think it's sufficient to just apply @yaonos's patch, do you agree? If so, I'll also cherry-pick it back as far as 3.2, cause it looks like that's how far back this issue goes.

Or do we want to do something more complicated here, perhaps checking what the backend supports like proxy_part_filldata() does?

[ksmurchison]

@elliefm Yes, I agree that the @yaonos fix is correct. I applied it and bolstered the Cass test in #4441