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

Always add URL field on media events #680

Closed
wants to merge 5 commits into from

Conversation

surakin
Copy link
Contributor

@surakin surakin commented Dec 29, 2023

@sumnerevans
Copy link
Member

Are stickers not supposed to be encrypted by the spec 😱

@jjj333-p
Copy link

jjj333-p commented Jan 2, 2024

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

Copy link
Member

@tulir tulir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workarounds for bad clients need to be clearly marked. It also needs to be minimal, like using a fake mxc URI or a blank string to avoid it being misinterpreted as a real field. Definitely can't add it to non-sticker events either

@surakin
Copy link
Contributor Author

surakin commented Jan 2, 2024

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
Copy link
Member

tulir commented Jan 2, 2024

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
Copy link
Contributor Author

surakin commented Jan 2, 2024

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

@surakin surakin requested a review from tulir January 2, 2024 09:55
@jjj333-p
Copy link

jjj333-p commented Jan 2, 2024 via email

@surakin
Copy link
Contributor Author

surakin commented Jan 2, 2024

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 🤷

@tulir
Copy link
Member

tulir commented Jan 3, 2024

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

@surakin
Copy link
Contributor Author

surakin commented Jan 3, 2024 via email

@tulir
Copy link
Member

tulir commented Jan 3, 2024

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

@surakin
Copy link
Contributor Author

surakin commented May 23, 2024

Closing in favor of ruma/ruma#1820

@surakin surakin closed this May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Unsupported event: m.sticker Bridge sends invalid stickers
4 participants