throneless-tech / libsignal-service-javascript

A javascript library for basic interaction with the Signal messaging service, adapted from Signal-Desktop.
GNU General Public License v3.0
57 stars 21 forks source link

Either wrong name or type in MessageReceiver.cacheRemoveBatch(items) #20

Closed panzi closed 3 years ago

panzi commented 3 years ago

https://github.com/throneless-tech/libsignal-service-javascript/blob/08da9fb5d858d479218225ffae2f74295e2399fb/src/MessageReceiver.js#L531

removeUnprocessed(id) receives a single id, not an array of items. Is the parameter name wrong or should the body of the function be the following?

  async cacheRemoveBatch(items) {
    for (const item of items) {
      await this.store.removeUnprocessed(item.id);
    }
  }

Context: I'm currently in the process of reverse engineering types so I can actually can implement a correct storage backend. A naive approach didn't work, because of all the ArrayBuffer fields in the objects. JSON.stringify() just converts those to "{}" and simply recursively scanning the passed object and encoding as .toString("binary") is no good either, since PostgreSQL does not like strings that aren't valid unicode. And I remember some JSON parsers choke on nil bytes in strings too, even though a nil byte is be valid unicode. In general .toString("binary") is a hack that will lead to headache, so instead I try to cleanly convert to/from base64.

jheretic commented 3 years ago

Hi @panzi, good catch. I think that must have been missed when I pulled in some upstream updates to batch processing in the most recent release. I think changing cacheRemoveBatch() keeps removeUnprocessed more consistent with the rest of the storage API. If you want to open a PR for that, I'll merge it, otherwise I can make that fix real quick and publish out a patch release.

Some of the storage API types are definitely a bit idiosyncratic, and it was many of the projects using this at the time I ported it were using things more along the lines of generic key-value stores for their storage than something like Postgres, so thanks for slogging through that. If it helps, there are some conversion utilities ported from the upstream project in src/helpers.js, like jsonThing, that may or may not assist with stringifying deep structures with weird buffer types.

panzi commented 3 years ago

Don't have time for a pull request right now. Working on our own Signal Gateway (AGPL, not yet published) right now. In that context I started to write types for the parts of this library that are relevant to me. WIP version of just those types, in case you're interested: https://gist.github.com/panzi/ec3a9e610ccd49c84eaea362e547f4f5 (This will of course be published as AGPLv3 once somewhat finished.)

jheretic commented 3 years ago

No worries, I'll just make the change and push it up then. That's cool! FYI the next version of this library will be Typescript (since upstream Signal-Desktop moved to Typescript), so hopefully that'll be helpful to you when it's ready.

panzi commented 3 years ago

It will be helpful, thank you. Btw. is there no method to update the account's profile and avatar?

jheretic commented 3 years ago

It's probably possible to do. There are a number of different SyncMessage events from upstream that I haven't fully explored. But there isn't, say, a method exposed on AccountManager that does that. Next version, hopefully.

jheretic commented 3 years ago

Fixed in 18698abee4065cdfbf1aa842c845834e010c7e06

panzi commented 3 years ago

About profile/avatar: Do you know any way to do it, even not via API? Because when I verify the official Signal app it will unverify the node app, right? So how would I even give our chatbot a nice username+avatar?

panzi commented 3 years ago

Or even forget setting a name, how do I get the name? How do I decrypt the "name" field of a profile? I tried to do the following:

        const identityKey = await connection.sender.store.getIdentityKeyPair();
        const name = await crypto.decryptProfileName(profile.name, identityKey.privKey);
        profile.name = {
            encrypted: profile.name,
            given:  Buffer.from(name.given).toString("UTF-8"),
            family: name.family !== null ? Buffer.from(name.family).toString("UTF-8") : null,
        };

But that always just gives me { given: "", family: null } (if the name is set by the user, otherwise the "name" field is null already.)

panzi commented 3 years ago

Background: We develop a chatbot that users can to collaborate on certain things. I need to be able to name the participants. Otherwise everyone is called "Unknown User", which would make it all useless. I'd also like to display the other user's avatars, but I have even less hope on figuring out how to decrypt that.