Replace deprecated iFrame frameborder with border: 0 #33331
Conversation
lib/sanitize_ext/sanitize_config.rb
Outdated
| @@ -136,7 +136,7 @@ module Config | |||
| attributes: { | |||
| 'audio' => %w(controls), | |||
| 'embed' => %w(height src type width), | |||
| 'iframe' => %w(allowfullscreen frameborder height scrolling src width), | |||
| 'iframe' => %w(allowfullscreen height scrolling src style width), | |||
There was a problem hiding this comment.
I think allowing style might actually introduce potential security issues. Not sure this is necessary.
There was a problem hiding this comment.
Should I just leave frameborder as allowed here, for any external embeds that might still use it?
There was a problem hiding this comment.
Allowing style attributes should be safe, although there have been vulnerabilities in our sanitizing library in the past, and limiting the surface area should be best.
And if we do decide to allow style attributes, it probably makes sense to only allow specific properties like border. And keep support for frameborder as well.
There was a problem hiding this comment.
Thanks! To keep this simple, I've reverted this file, and kept the changes focused to the places where Mastodon was authoring the frameborder attribute
There was a problem hiding this comment.
Afaik the html bit you modified still goes through sanitization, so by using style instead, it will get sanitized away and won't work.
There was a problem hiding this comment.
Good call. I added back style, but left frameborder for any legacy content that might need it for the same affect
nschonni
force-pushed
the
remove-frameborder
branch
from
ebd429b to
7b0c884
Compare
nschonni
force-pushed
the
remove-frameborder
branch
from
7b0c884 to
7aafb12
Compare
Not completely sure about the Ruby changes, but looked for the refrences from the failing ESLint PR.
Used the inline style to turn off the border https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#frameborder