fix(gif): Restrict gif rendering to Giphy only #14988
Conversation
hristoterezov
force-pushed
the
gif-whitelist
branch
from
a3798d0
to
59c12b5
Compare
|
Pl also drop the proxy sample from resources/ |
|
Missing check for disableThirdPartyRequests |
hristoterezov
force-pushed
the
gif-whitelist
branch
from
59c12b5
to
0d899f9
Compare
Done! |
What do you mean? Where? I've added bunch of isGifEnabled checks and they include disableThirdPartyRequests check + if giphy is enabled trough config or dynamic branding and some more checks. I think those are enough? |
|
The shared video test is failing but it seems we have a general issue with it because even vanilla jitsi-meet PR makes it fail. Tested manually and it works. I think we can merge this PR! |
config.js
Outdated
| // rating: 'pg', | ||
| // // The proxy server url for giphy requests in the web app. | ||
| // proxyUrl: 'https://giphy-proxy.example.com', | ||
| // rating: 'pg' |
There was a problem hiding this comment.
Nit: leave the trailing comma, so adding new options doesn't result in a syntax error.
react/features/gifs/function.any.ts
Outdated
| return false; | ||
| } | ||
|
|
||
| return Boolean(hostname) && Boolean(GIF_HOST_WHITELIST.find(h => h === hostname)); |
There was a problem hiding this comment.
Since it's oonly 1 domain, it's more resilient to do an equaality check. This way any prototype pollution bug doesn't render this protection useless.
| /** | ||
| * Whether the gifs are enabled or not. | ||
| */ | ||
| gifEnabled?: boolean; |
There was a problem hiding this comment.
Can you modify isGifEnabled to always return a boolean, it makes no sense that it can return undefined. This way it can default to false I guess, rather than haveing to do it everywhere.
There was a problem hiding this comment.
it actually always returns boolean.
Fixed!
hristoterezov
force-pushed
the
gif-whitelist
branch
from
0d899f9
to
ac4391b
Compare
hristoterezov
force-pushed
the
gif-whitelist
branch
from
ac4391b
to
07971cd
Compare
No description provided.