Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

user contact customizations (e.g. nickGivenName) on Android are discarded after linked signal-cli receive is run #1678

Closed
brad2014 opened this issue Jan 16, 2025 · 10 comments
Milestone

Comments

@brad2014
Copy link

brad2014 commented Jan 16, 2025

This is related to (and may be a duplicate of) #1664, but I think there's something else going on.

After modifying nickGivenName and note for a contact on Android, some set of circumstances (perhaps the combination of signal-cli receive and a change in contact's profile.lastUpdateTimestamp) is causing the Android's nickGivenName (not just the signal-cli local store for the contact) to revert to null.

Separately, the local store for the "note" is set to null (but, oddly, this does not propagate to the Android). This may be a separate bug.

Signal-cli Version: 0.13.11, running against a linked "device" associated with an Signal master device on Android.

Actual behavior:

  1. After setting the nickGivenName on an Android master device for a non-system contact, and running signal-cli receive on a linked device, notice that the nickGivenName on the Android reverts to null.
  2. Also the note field on the linked device is set to null (while it is not cleared on the Android).
  3. Other odd local storage updates, noted below (may be fine, I just don't understand them)

Expected behavior:

  1. "signal-cli receive" should be "read only", I expect it to not change contact storage on the signal server or master device.
  2. After "signal-cli receive" the note field on the stored contact should match that on the master device.

To reproduce:

  1. Set nickGivenName and note on Android for a non-contact-list contact
  2. Run and save listContacts on client
  3. Run signal-cli receive on the client. Notice that after signal-cli receive, nickGivenName is not set, but note is (i.e. bug nickGivenName and nickFamilyName not retrieved for non-contact recipient #1664):
  4. Run and save listContacts on client and diff against previous saved version
--- listContacts-pre-receive.json	2025-01-16 11:30:14.465151652 +0100
+++ listContacts-post-receive.json	2025-01-16 11:30:26.733065458 +0100
@@ -7596,7 +7596,7 @@
     "nickName": null,
     "nickGivenName": "[REDACTED]", <-- matches profile contact givenName
     "nickFamilyName": null,                  <-- bug #1664 
-    "note": "[REDACTED]".                  <-- correct previous note value
+    "note": "[REDACTED]"                    <-- correct current note value (as expected)
     "color": "ultramarine",
     "isBlocked": false,
     "isHidden": false,
  1. Send a test message to the contact on the Android
  2. Something causes contact's profile.lastUpdateTimestamp to change (not sure how to force that)
  3. Re-run listContacts, signal-cli receive, listContacts on the client.

After the second receive the local contact store for signal-cli is changed thus:

--- listContacts2-pre-receive.json	2025-01-16 11:32:56.147015651 +0100
+++ listContacts2-post-receive.json	2025-01-16 11:33:08.683927564 +0100
@@ -7594,17 +7594,17 @@
     "givenName": null,
     "familyName": null,
     "nickName": null,
-    "nickGivenName": "[REDACTED]: <-- matches profile givenName, not what was set on Android in step 1
+    "nickGivenName": null,                 <-- bug #1664, reported fixed
     "nickFamilyName": null,
-    "note": "[REDACTED]",                <-- correct note value, matching Android
+    "note": null,                                   <-- This is wrong. The note should not be set to null
     "color": "ultramarine",
     "isBlocked": false,
     "isHidden": false,
-    "messageExpirationTime": 0,
+    "messageExpirationTime": 2419200,  <-- this is the correct setting on Android, not sure why it was not in signal-cli storage
     "profileSharing": true,
-    "unregistered": true,
+    "unregistered": false,  <-- this is the correct setting on Android, not sure why it was not in signal-cli storage
     "profile": {
-      "lastUpdateTimestamp": 1736434811527,
+      "lastUpdateTimestamp": 1737023579346,  <-- not sure what caused this update from 2025-1-9 to 2025-1-16, since rest of profile did not change
       "givenName": "[REDACTED]",
       "familyName": null,
       "about": null,
@brad2014
Copy link
Author

brad2014 commented Jan 16, 2025

Note: I'm hypothesizing that the combination of a profile.lastUpdateTimestamp change plus the "signal-cli receive" run is the cause of undoing the user change to the contact on the Android master, but I could be getting the cause wrong. This is proving hard to reproduce.

I'm also noticing that other stored contact parameters (e.g. contact.blocked, perhaps) are reverting on Android (I set them, and a little while later my change is mysteriously undone) on , and I'm wondering if my periodically running "signal-cli receive" on the linked device might be involved in causing it.

@AsamK
Copy link
Owner

AsamK commented Jan 16, 2025

Might be related to #1549
Can you try it with the latest development build? https://github.com/AsamK/signal-cli/actions/runs/12776836588

@brad2014
Copy link
Author

brad2014 commented Jan 16, 2025

Version: signal-cli 0.13.12-SNAPSHOT

I ran the experiment in the transcript below Using the latest development build you mention, signal-cli 0.13.12-SNAPSHOT. It creates a new link, edits an android contact, sends an android message, capturing the linkContacts state before and after each change. I notice:

  1. "WARN RetrieveStickerPackJob warning" does not have a bug filed against it.
  2. Between listContacts-3 and listContacts-4.json, the nickGivenName is updated, confirming fix of nickGivenName and nickFamilyName not retrieved for non-contact recipient #1664 .
  3. Between listContacts-4.json and listContacts-5.json, the profile.lastUpdateTimestamp changed, and the nickGivenName has reverted to null, reconfirming this bug (user contact customizations (e.g. nickGivenName) on Android are discarded after linked signal-cli receive is run #1678) on the SNAPSHOT release.

(I will note that, while the experiment is in progress, the Android is also linked to a receive-only signal-cli 0.13.11 device and a quiescent signal-desktop 7.38.0 device, though neither had any obvious outbound activity that I know of. If we want to test without the 0.13.11 device on the line, which I'm happy to do, it will need to wait until 0.13.12 is released.)

I hope this is helpful.

Transcript (edited/redacted):

% ./signal-cli --config . link -n signaltest # establish a fresh linked device
% ./signal-cli --config . --output=json listContacts -a | jq 'sort_by(.uuid,.number)' > listContacts-1.json
% ./signal-cli --config . --output=json receive | tee -a msg.out 
WARN  RetrieveStickerPackJob - Failed to retrieve sticker pack REDACTED: Class org.asamk.signal.manager.storage.stickerPacks.JsonStickerPack$JsonSticker[] is instantiated reflectively but was never registered.Register the class by adding "unsafeAllocated" for the class in reflect-config.json. (through reference chain: org.asamk.signal.manager.storage.stickerPacks.JsonStickerPack["stickers"])
[error repeats for each sticker pack]
% ./signal-cli --config . --output=json listContacts -a | jq 'sort_by(.uuid,.number)' > listContacts-3.json
% # on android, set the nickGivenName and nickFamily name for contact
% ./signal-cli --config . --output=json receive | tee -a msg.out
% ./signal-cli --config . --output=json listContacts -a | jq 'sort_by(.uuid,.number)' > listContacts-4.json
% diff -y listContacts-3.json listContacts-4.json
... snip ...
  {                                                               {
    "number": null,                                                 "number": null,
    "uuid": "REDACTED UUID",                             "uuid": "REDACTED UUID",
    "username": null,                                               "username": null,
    "name": "",                                                     "name": "",
    "givenName": null,                                              "givenName": null,
    "familyName": null,                                             "familyName": null,
    "nickName": null,                                               "nickName": null,
    "nickGivenName": null,                                    |     "nickGivenName": "[REDACTED- correctly reflects Android update] ",
    "nickFamilyName": null,                                   |     "nickFamilyName": "[REDACTED- correctly reflects Android update]",
    "note": null,                                                   "note": null,
    "color": "indigo",                                              "color": "indigo",
    "isBlocked": false,                                             "isBlocked": false,
    "isHidden": false,                                              "isHidden": false,
    "messageExpirationTime": 0,                                     "messageExpirationTime": 0,
    "profileSharing": true,                                         "profileSharing": true,
    "unregistered": false,                                          "unregistered": false,
    "profile": {                                                    "profile": {
      "lastUpdateTimestamp": 0,                                       "lastUpdateTimestamp": 0,
      "givenName": "[REDACTED]",                                      "givenName": "[REDACTED]",
      "familyName": null,                                             "familyName": null,
      "about": null,                                                  "about": null,
      "aboutEmoji": null,                                             "aboutEmoji": null,
      "hasAvatar": false,                                             "hasAvatar": false,
      "mobileCoinAddress": null                                       "mobileCoinAddress": null
    }                                                               }
  },                                                              },
... end-snip ...
% # on Android, send a test message to the user 
% ./signal-cli --config . --output=json receive | tee -a msg.out
% ./signal-cli --config . --output=json listContacts -a | jq 'sort_by(.uuid,.number)' > listContacts-5.json
% diff -y listContacts-4.json listContacts-5.json
... snip ...
  {                                                               {
    "number": null,                                                 "number": null,
    "uuid": "[REDACTED UUID]",                             "uuid": "[REDACTED UUID]",
    "username": null,                                               "username": null,
    "name": "",                                                     "name": "",
    "givenName": null,                                              "givenName": null,
    "familyName": null,                                             "familyName": null,
    "nickName": null,                                               "nickName": null,
    "nickGivenName": "[REDACTED]",                            |     "nickGivenName": null,
    "nickFamilyName": "[REDACTED]",                           |     "nickFamilyName": null,
    "note": null,                                                   "note": null,
    "color": "indigo",                                              "color": "indigo",
    "isBlocked": false,                                             "isBlocked": false,
    "isHidden": false,                                              "isHidden": false,
    "messageExpirationTime": 0,                               |     "messageExpirationTime": 604800,
    "profileSharing": true,                                         "profileSharing": true,
    "unregistered": false,                                          "unregistered": false,
    "profile": {                                                    "profile": {
      "lastUpdateTimestamp": 0,                               |       "lastUpdateTimestamp": 1737054356413,
      "givenName": "[REDACTED]",                                      "givenName": "[REDACTED]",
      "familyName": null,                                             "familyName": null,
      "about": null,                                          |       "about": "",
      "aboutEmoji": null,                                     |       "aboutEmoji": "",
      "hasAvatar": false,                                             "hasAvatar": false,
      "mobileCoinAddress": null                                       "mobileCoinAddress": null
    }                                                               }
  },                                                              },
... end-snip ...
% grep [REDACTED UUID] msg.out
{"envelope":{"source":"[REDACTED]","sourceNumber":"[REDACTED]","sourceUuid":"[REDACTED]","sourceName":"[REDACTED]","sourceDevice":1,"timestamp":1737054345938,"serverReceivedTimestamp":1737054343614,"serverDeliveredTimestamp":1737054355794,"syncMessage":{"sentMessage":{"destination":"[REDACTED UUID]","destinationNumber":null,"destinationUuid":"[REDACTED] UUID","timestamp":1737054345938,"message":"Test - please ignore","expiresInSeconds":604800,"viewOnce":false}}},"account":"[REDACTED]"}
{"envelope":{"source":"[REDACTED UUID]","sourceNumber":null,"sourceUuid":"[REDACTED UUID]","sourceName":"[REDACTED - profile name]","sourceDevice":1,"timestamp":1737054347130,"serverReceivedTimestamp":1737054347317,"serverDeliveredTimestamp":1737054355794,"receiptMessage":{"when":1737054347130,"isDelivery":true,"isRead":false,"isViewed":false,"timestamps":[1737054345938]}},"account":"[REDACTED]"}

@AsamK
Copy link
Owner

AsamK commented Jan 17, 2025

I think it's the other signal-cli instance, that's modifying the storage in between. Let's retest after the next release.

@brad2014
Copy link
Author

I have upgraded signal-cli to 0.3.12.

This appears to be fixed. nickGivenName and nickFamilyName are now set correctly on signal-cli storage, and Android do not appear to revert after a message is sent.

Closing.

@brad2014
Copy link
Author

"WARN RetrieveStickerPackJob warning" does not have a bug filed against it.

Note: fixed in c25468a

@brad2014 brad2014 reopened this Jan 23, 2025
@brad2014
Copy link
Author

brad2014 commented Jan 23, 2025

I have upgraded signal-cli to 0.3.12.

This appears to be fixed. nickGivenName and nickFamilyName are now set correctly on signal-cli storage, and Android do not appear to revert after a message is sent.

Closing.

Reopening [edit: various typos fixed]

Ugh. Unfortunately this is still happening on 0.3.12. Signal contacts with no phone number are having their nickGivenName and nickFamilyName reverting to null, and that is then deleting the nickname set on the Android.

Here's what I'm seeing:

  1. The linked devices are signal Android 7.29.4, signal-desktop (mac) 7.39.0, and signal-cli 0.3.12.

  2. Every signal contact that has the following properties has its nickGivenName and nickFamilyName set to null by signal-cli receive, and those null values are subsequently picked up by the other linked devices (e.g. the Android also throws away the nickname that was previously set there):

select(.uuid != null and .number == null and .profileSharing == true and .unregistered == false)

Nothing about those contacts has changed in any of my devices, and no messages referencing their contact uuids have been recorded by "signal-cli receive" during the interval between the setting of the nickname on Android and the reversion.

  1. The signal-cli receive command that causes the reversion reports this message:
{"envelope":{"source":"[REDACTED]","sourceNumber":"[REDACTED]","sourceUuid":"[REDACTED]","sourceName":"[REDACTED]","sourceDevice":1,"timestamp":1737622524854,"serverReceivedTimestamp":1737622522463,"serverDeliveredTimestamp":1737622808061,"syncMessage":{"type":"CONTACTS_SYNC"}},"account":"[REDACTED]"}

The previous syncMessage with type CONTACTS_SYNC was 4 days earlier, and the contact nicknames were stable and correct that entire time.

  1. Oddly the .note field customized on Android is also set to null by signal-cli, though that change is not fed back to the Android. Upon the next signal-cli receive, the .note field reappears in the signal-cli storage, even though there was no subsequent CONTACTS_SYNC syncMessage received. What caused the .note storage to be updated?
--- contacts.json	2025-01-23 09:00:05.507334826 +0000
+++ contacts.json.new	2025-01-23 09:00:19.765239313 +0000
@@ -736,8 +736,8 @@
     "givenName": null,
     "familyName": null,
     "nickName": null,
-    "nickGivenName": "[REDACTED]",
-    "nickFamilyName": "[REDACTED]",
+    "nickGivenName": null, <-- bug: the nickname provided previously by Android signal is discarded here then on all devices
+    "nickFamilyName": null,
     "note": null,
     "color": "green", <-- oddly this chat customization I did on Android is preserved
     "isBlocked": false,
@@ -6757,9 +6757,9 @@
     "givenName": null,
     "familyName": null,
     "nickName": null,
-    "nickGivenName": "[REDACTED]",
-    "nickFamilyName": "[REDACTED]",
-    "note": "[REDACTED]", 
+    "nickGivenName": null,
+    "nickFamilyName": null,
+    "note": null,   <-- note that customized note is also discarded,
     "color": "green", <-- but customized chat color is not
     "isBlocked": false,
     "isHidden": false,
//// an hour later, signal-receive is re-run
--- /home/brad/Documents/Messages/signal/contacts.json	2025-01-23 10:00:05.496407277 +0000
+++ /home/brad/Documents/Messages/signal/contacts.json.new	2025-01-23 10:00:17.931325020 +0000
@@ -6759,7 +6759,7 @@
     "nickName": null,
     "nickGivenName": null,  <-- Android by this time also has discarded its nickname
     "nickFamilyName": null,
-    "note": null,
+    "note": "[REDACTED]",  <-- oddly, note was not discarded on Android, and somehow found its way back to signal-cli storage
     "color": "green",
     "isBlocked": false,
     "isHidden": false,

@AsamK
Copy link
Owner

AsamK commented Jan 23, 2025

Thanks for the detailed report.
Looks like signal-cli discard the existing contact information when it receives a contact sync message. The contact sync message contains the color, but doesn't contain the nickname and note. That explains the observed behavior.
The note and nickname fields are handled differently by Signal-Android, that's why the note is not discarded:
https://github.com/signalapp/Signal-Android/blob/7dd1fc09c0a54dbb6c0e4a73d86471fc65ebc470/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.kt#L233
Signal-Android then stores the note again in the server-side storage and signal-cli retrieves it.

Will investigate, why the contact sync message handling doesn't work correctly for users without a number.

@brad2014
Copy link
Author

brad2014 commented Jan 23, 2025

Cool. So the fact (confirmed) that any linked device can edit a note but not delete a note is a signal-android bug - I'll file that one there.

@AsamK AsamK closed this as completed in b8d8413 Jan 23, 2025
@AsamK AsamK added this to the next-version milestone Jan 23, 2025
@brad2014
Copy link
Author

You rock! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants