BlueBubblesApp / bluebubbles-server

Server for forwarding iMessages to clients within the BlueBubbles App ecosystem
https://bluebubbles.app
Apache License 2.0
540 stars 47 forks source link

Messages with multiple attachments will only return one of the attachments #547

Closed tnvu closed 1 year ago

tnvu commented 1 year ago

I think I found the issue --

MessageRepository.getAttachment() accepts an attachment GUID but discards the attachment prefix to perform a lookup based on the "actual" guid. It then queries the database and then calls query.getOne() which return the same attachment every single time.

zlshames commented 1 year ago

I think I found the issue --

MessageRepository.getAttachment() accepts an attachment GUID but discards the attachment prefix to perform a lookup based on the "actual" guid. It then queries the database and then calls query.getOne() which return the same attachment every single time.

This is code on the server, so it is a server bug?

tnvu commented 1 year ago

Yeah, I think so. I initially found the behavior on the android client and wasn't sure what was going on. I brought up the issue in both #android-support and #server-support and was told to file a client bug.

I eventually dug into the code to see what was going on and found the problem. Not sure how to change it to a server bug.

zlshames commented 1 year ago

Moved. I'll fix it

tnvu commented 1 year ago

Thanks. I submitted a possible solution here

https://github.com/BlueBubblesApp/bluebubbles-server/pull/546

I haven't tested it yet because I can't get things to build on my system.

zlshames commented 1 year ago

PR merged. Should be good now. Hard to officially test, but it was working normally

tnvu commented 1 year ago

You can test by sending a message with multiple attachments. It should return individual images and not the same image multiple times

On Mon, Jul 17, 2023, 6:12 PM Zach Shames @.***> wrote:

PR merged. Should be good now. Hard to officially test, but it was working normally

— Reply to this email directly, view it on GitHub https://github.com/BlueBubblesApp/bluebubbles-server/issues/547#issuecomment-1639124824, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIRCLCJLOTCLIIL267PVXDXQXPJRANCNFSM6AAAAAAZ63YR6M . You are receiving this because you authored the thread.Message ID: @.***>

zlshames commented 1 year ago

The issue was not fixed. And I think the changes broke something more than I thought. Gonna have to revert some of the code and retry it

zlshames commented 1 year ago

Fixed again. It's cuz the for loop used "in" instead of "of" and it caused the list of GUIDs to improperly be iterated over. It was iterating over the indexes, not the values.

Just JavaScript things 😁