mautrix / whatsapp

A Matrix-WhatsApp puppeting bridge
https://maunium.net/go/mautrix-whatsapp
GNU Affero General Public License v3.0
1.21k stars 170 forks source link

Always add URL field on media events #680

Closed surakin closed 1 month ago

surakin commented 6 months ago

Fixes https://github.com/mautrix/whatsapp/issues/662 Fixes https://github.com/element-hq/element-x-ios/issues/1047

sumnerevans commented 6 months ago

Are stickers not supposed to be encrypted by the spec :scream:

jjj333-p commented 6 months ago

Are stickers not supposed to be encrypted by the spec 😱

as far as i understand the spec completely ignores this possibility.

i think it makes sense from a userland implementation, you add it to the global im.ponies.emotes or whatever state event as an mxc and then you just link to it and its intended to be a public resource. the difference is that the bridge treats stickers as media to bridge and therefore tries to encrypt it which simply doesnt agree with that thought train. i suppose its just a hole in the spec

surakin commented 6 months ago

As far as I understand the issue, the rust sdk uses the url field when it parses a sticker so it can't be fake or blank.

tulir commented 6 months ago

It must also be using the file object, because otherwise it'd just get an encrypted blob. That's why I assumed it was only validating the presence of url and not actually using it

surakin commented 6 months ago

It must also be using the file object, because otherwise it'd just get an encrypted blob. That's why I assumed it was only validating the presence of url and not actually using it

I just tried using an empty string and stickers don't show up It must be using the file object, yes, but the missing url makes the initial parsing of the sticker event just fail

jjj333-p commented 6 months ago

It’s got to be using the keys from the file object, it’s probably a bug in the sdk

On Mon, Jan 1, 2024 at 11:54 PM Marco Antonio Alvarez < @.***> wrote:

It must also be using the file object, because otherwise it'd just get an encrypted blob. That's why I assumed it was only validating the presence of url and not actually using it

I just tried using an empty string and stickers don't show up It must be using the file object, yes, but the missing url makes the initial parsing of the sticker event just fail

— Reply to this email directly, view it on GitHub https://github.com/mautrix/whatsapp/pull/680#issuecomment-1873810569, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWNJYMB5L2BRS5DOR2IDVGLYMPKNVAVCNFSM6AAAAABBGWXUNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZTHAYTANJWHE . You are receiving this because you commented.Message ID: @.***>

surakin commented 6 months ago

It’s got to be using the keys from the file object, it’s probably a bug in the sdk

More like a bug in the spec, I think. It says that for stickers the content.url field is required and the sdk is honoring that :shrug:

tulir commented 6 months ago

Hmm, I can't reproduce making element x work by just adding the url field

surakin commented 6 months ago

Are you running latest from the develop branch?

On Wed, 3 Jan 2024 at 13:10, Tulir Asokan @.***> wrote:

Hmm, I can't reproduce making element x work by just adding the url field

— Reply to this email directly, view it on GitHub https://github.com/mautrix/whatsapp/pull/680#issuecomment-1875275785, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFG5BGW2LBLWPWF42TPJUTYMVDEFAVCNFSM6AAAAABBGWXUNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZVGI3TKNZYGU . You are receiving this because you authored the thread.Message ID: @.***>

tulir commented 6 months ago

Yes, it tries to render it, but just shows a blank box

surakin commented 1 month ago

Closing in favor of https://github.com/ruma/ruma/pull/1820