Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Add fallback to group avatar in case thumbnail not found on server #1485

Closed
wants to merge 3 commits into from

Conversation

agent3bood
Copy link

No description provided.

@agent3bood
Copy link
Author

@turt2live
Copy link
Member

I would almost expect this to be fixed on the server side to be honest. The thumbnailer seems broken, but if it can't be fixed then this seems like a relatively okay solution to me. (Although I have no authority on this aspect and I haven't tested it myself).

@agent3bood
Copy link
Author

Agree, fixing it on tbe server is the way to go, but this fix is adding a fallback incase thumbnail not found on the server.

@agent3bood agent3bood closed this Oct 16, 2017
@agent3bood agent3bood reopened this Oct 16, 2017
@agent3bood
Copy link
Author

Also this issue
element-hq/element-web#2581

@agent3bood agent3bood changed the title fix .ico room avatar Add fallback to group avatar in case thumbnail not found on server Oct 18, 2017
@lukebarnard1
Copy link
Contributor

lukebarnard1 commented Oct 25, 2017

So this potentially allows for an SVG to show be shown as the room avatar? We shouldn't be showing any SVGs afaik. Better for this to be done on the server I think.

@ara4n
Copy link
Member

ara4n commented Nov 3, 2017

yeah, i see the intention here, but the fact that it displays the original SVG if a thumbnail is missing (resolving element-hq/element-web#2581) is not really ideal - as Luke says, we go to pains to not currently show any user-generated SVGs, as it is trivial to sculpt malicious SVGs (e.g. billion lol attacks). If anyone is wrangling thumbnailing of SVGs it should be the sender client (or possibly the server)'s responsibility.

Sorry for not being able to merge :(

@ara4n ara4n closed this Nov 3, 2017
@agent3bood
Copy link
Author

Thanks all for your comments, so this issue is a backend problem not the client.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants