Closed stedmanblake closed 7 years ago
@krnjn I removed some strip_tags
calls in the second commit. I split this into two commits so that you can remove the second if you disagree. Here's my rationale; please verify!
There's no way to keep strip_tags
from encoding special characters, (e.g. '&' => '&').
Sources:
https://github.com/rails/rails-html-sanitizer/issues/28
https://github.com/rails/rails-html-sanitizer/commit/49dfc1584c5b8e35a4ffabf8356ba3df025e8d3f
The only affected text fields (for which I removed strip_tags
) are coming from the server. We sanitized them prior to persisting them, so I don't believe there could be any malicious tags in need of stripping by this point.
The other option is to keep strip_tags
and then decode special characters. This just seemed needlessly complicated given point 2 above.
Thanks!
Thanks for the lift on this @stedmanblake - I refactored things down to a single partial to handle all the meta tags for these links per the docs here: https://dev.twitter.com/cards/getting-started#twitter-cards-and-open-graph
I see what you're saying about strip_tags
but it's more used for stripping the HTML tags rather than sanitizing anything for safety (e.g. saving characters by removing the HTML mark up for the preview in different sharing options).
I'm only passing up the specific contributions' title / description if they exist, otherwise I default to the title / description of the challenge. Let me know if you have questions about it, but this looks good for me!
Sounds good! Thanks for the update.
1st commit
shared/cards/_twitter.html.erb
to add meta tags.application.html.erb
._share_links
, and_share_popover
. These are specific to the current Challenge / Stage. • To test: https://cards-dev.twitter.com/validator (NB: it doesn't work yet; Twitter needs to review and whitelist us.)2nd commit
&
problem by removing thestrip_tags()
calls in_share_links
and_share_popover
. • It's not possible avoid encoding special characters when usingstrip_tags
: https://github.com/rails/rails-html-sanitizer/commit/49dfc1584c5b8e35a4ffabf8356ba3df025e8d3f • I don't think we don't need to worry about security for these text fields, because they were sanitized prior to being saved on the server.