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

Fixing attachments #45

Merged
merged 18 commits into from
Mar 8, 2021
Merged

Fixing attachments #45

merged 18 commits into from
Mar 8, 2021

Conversation

golf1052
Copy link
Member

Attachments are currently broken as noted by #39 and signal-csharp/Signal-Windows#209. This commit 8bb307d aimed to move to attachments V2 but while trying to integrate with Signal-Windows I was still getting 0 for every attachment id. This Signal-Android commit signalapp/Signal-Android@37a35e8 appears to be where attachments V3 was introduced. libsignal-service-java needs to disambiguate between V2 and V3 attachments and that is probably why attachments aren't working in Signal-Windows. We are currently unaware of how to disambiguate. This draft PR will keep track of changes/comments while working towards getting attachments working again.

@golf1052
Copy link
Member Author

I've started implementing signalapp/Signal-Android@37a35e8

While implementing that I needed to implement signalapp/Signal-Android@b598431

While implementing that I needed to implement signalapp/libsignal-service-java@a12d506

Now I need to finally implement signalapp/libsignal-service-java@2f59a94 because too much stuff depends on UUID support. I haven't started implementing that yet because it's going to fuck up all the changes I've made so far.

@golf1052
Copy link
Member Author

golf1052 commented Feb 26, 2021

Here are the commits I'll most likely add to this PR (hopefully one commit at a time and not all at once)

Culminating with

There are what looks like bug fixes after the UUID change but I'll think about that when I get to them.

@golf1052
Copy link
Member Author

golf1052 commented Mar 4, 2021

Started working on integrating with Signal-Windows to test that attachments are working. I updated the number references to use E164 in most places but it looks like to send messages I'll actually have to refer to the UUID of the contact I'm sending to.

@golf1052
Copy link
Member Author

golf1052 commented Mar 5, 2021

Got message sending working again. Needed to add a big-endian BinaryReader/Writer to UuidUtil. Also needed to use the UUID to send messages instead of the E.164. This required a database migration in Signal-Windows to save the UUID. Next up is saving the correct stuff for v2 vs v3 attachments.

@golf1052
Copy link
Member Author

golf1052 commented Mar 6, 2021

Attachment downloads now work again. Attachment uploads are failing due to a 400 Bad Request error currently when trying to upload to the CDN

@golf1052
Copy link
Member Author

golf1052 commented Mar 7, 2021

Attachment V2 uploads work (currently buffering the attachment into memory because there's a bug in System.Net.Http.HttpClient). Attachment V3 uploads are still broken.

@golf1052
Copy link
Member Author

golf1052 commented Mar 8, 2021

Attachment V3 uploads now work.

@golf1052 golf1052 marked this pull request as ready for review March 8, 2021 06:31
If the Stream length is shortened to remove the MAC the cipherfile will not be usable again. Instead save the MAC before removing it from the file and add it back on before the Stream is disposed. Also ensure we dispose all Streams correctly.
Part of Add initial support for send/receive on CDN2: signalapp/Signal-Android@37a35e8

Part of Separate message decryption from message processing: signalapp/Signal-Android@b598431

All of Add support for a requiredProtocolVersion: signalapp/libsignal-service-java@a12d506
better support more countries that have phone numbers less than 10 digits (eg Finland, Sweden). the previous version was more restrictive than necessary.

Reflects signalapp/libsignal-service-java@e8229de
Reflects signalapp/Signal-Android@37a35e8

Also includes
- Separate message decryption from message processing: signalapp/Signal-Android@b598431
- Add support for UUIDs: signalapp/libsignal-service-java@2f59a94

Now builds. Tests do not pass yet.
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

Successfully merging this pull request may close these issues.

None yet

1 participant