Commit 8235fc9d9cb92ae0d98145ed4f5e79dedcd6c929
1 parent
fb028d5a
Exists in
master
and in
29 other branches
metadata: marks urls as html safe
Showing
9 changed files
with
36 additions
and
14 deletions
Show diff stats
app/models/article.rb
| ... | ... | @@ -742,9 +742,10 @@ class Article < ActiveRecord::Base |
| 742 | 742 | end |
| 743 | 743 | |
| 744 | 744 | def body_images_paths |
| 745 | - require 'uri' | |
| 746 | 745 | Nokogiri::HTML.fragment(self.body.to_s).css('img[src]').collect do |i| |
| 747 | - (self.profile && self.profile.environment) ? URI.join(self.profile.environment.top_url, URI.escape(i['src'])).to_s : i['src'] | |
| 746 | + src = i['src'] | |
| 747 | + src = URI.escape src if self.new_record? # xss_terminate runs on save | |
| 748 | + (self.profile && self.profile.environment) ? URI.join(self.profile.environment.top_url, src).to_s : src | |
| 748 | 749 | end |
| 749 | 750 | end |
| 750 | 751 | ... | ... |
plugins/metadata/lib/ext/article.rb
| ... | ... | @@ -12,9 +12,9 @@ class Article |
| 12 | 12 | end, |
| 13 | 13 | title: proc{ |a, plugin| "#{a.title} - #{a.profile.name}" }, |
| 14 | 14 | image: proc do |a, plugin| |
| 15 | - img = a.body_images_paths | |
| 16 | - img = "#{a.profile.environment.top_url}#{a.profile.image.public_filename}" if a.profile.image if img.blank? | |
| 17 | - img ||= MetadataPlugin.config[:open_graph][:environment_logo] rescue nil if img.blank? | |
| 15 | + img = a.body_images_paths.map! &:html_safe | |
| 16 | + img = "#{a.profile.environment.top_url}#{a.profile.image.public_filename}".html_safe if a.profile.image if img.blank? | |
| 17 | + img ||= MetadataPlugin.config[:open_graph][:environment_logo].html_safe rescue nil if img.blank? | |
| 18 | 18 | img |
| 19 | 19 | end, |
| 20 | 20 | see_also: [], |
| ... | ... | @@ -31,10 +31,10 @@ class Article |
| 31 | 31 | card: 'summary', |
| 32 | 32 | description: proc do |a, plugin| |
| 33 | 33 | description = a.body.to_s || a.environment.name |
| 34 | - CGI.escapeHTML(plugin.helpers.truncate(plugin.helpers.strip_tags(description), length: 200)) | |
| 34 | + plugin.helpers.truncate plugin.helpers.strip_tags(description), length: 200 | |
| 35 | 35 | end, |
| 36 | 36 | title: proc{ |a, plugin| "#{a.title} - #{a.profile.name}" }, |
| 37 | - image: proc{ |a, plugin| a.body_images_paths }, | |
| 37 | + image: proc{ |a, plugin| a.body_images_paths.map! &:html_safe }, | |
| 38 | 38 | } |
| 39 | 39 | |
| 40 | 40 | metadata_spec namespace: :article, key_attr: :property, tags: { | ... | ... |
plugins/metadata/lib/ext/product.rb
| ... | ... | @@ -14,8 +14,8 @@ class Product |
| 14 | 14 | description: proc{ |p, plugin| ActionView::Base.full_sanitizer.sanitize p.description }, |
| 15 | 15 | |
| 16 | 16 | image: proc do |p, plugin| |
| 17 | - img = "#{p.environment.top_url}#{p.image.public_filename}" if p.image | |
| 18 | - img = "#{p.environment.top_url}#{p.profile.image.public_filename}" if img.blank? and p.profile.image | |
| 17 | + img = "#{p.environment.top_url}#{p.image.public_filename}".html_safe if p.image | |
| 18 | + img = "#{p.environment.top_url}#{p.profile.image.public_filename}".html_safe if img.blank? and p.profile.image | |
| 19 | 19 | img ||= MetadataPlugin.config[:open_graph][:environment_logo] rescue nil if img.blank? |
| 20 | 20 | img |
| 21 | 21 | end, | ... | ... |
plugins/metadata/lib/ext/profile.rb
| ... | ... | @@ -5,8 +5,8 @@ class Profile |
| 5 | 5 | metadata_spec namespace: :og, tags: { |
| 6 | 6 | type: proc{ |p, plugin| plugin.context.params[:og_type] || MetadataPlugin.og_types[:profile] || :profile }, |
| 7 | 7 | image: proc do |p, plugin| |
| 8 | - img = "#{p.environment.top_url}#{p.image.public_filename}" if p.image | |
| 9 | - img ||= MetadataPlugin.config[:open_graph][:environment_logo] rescue nil if img.blank? | |
| 8 | + img = "#{p.environment.top_url}#{p.image.public_filename}".html_safe if p.image | |
| 9 | + img ||= MetadataPlugin.config[:open_graph][:environment_logo].html_safe rescue nil if img.blank? | |
| 10 | 10 | img |
| 11 | 11 | end, |
| 12 | 12 | title: proc{ |p, plugin| if p.nickname.present? then p.nickname else p.name end }, | ... | ... |
plugins/metadata/lib/ext/uploaded_file.rb
| ... | ... | @@ -13,7 +13,7 @@ class UploadedFile |
| 13 | 13 | plugin.og_url_for url |
| 14 | 14 | end, |
| 15 | 15 | title: proc{ |u, plugin| u.title }, |
| 16 | - image: proc{ |u, plugin| "#{u.environment.top_url}#{u.public_filename}" if u.image? }, | |
| 16 | + image: proc{ |u, plugin| "#{u.environment.top_url}#{u.public_filename}".html_safe if u.image? }, | |
| 17 | 17 | description: proc{ |u, plugin| u.abstract || u.title }, |
| 18 | 18 | } |
| 19 | 19 | ... | ... |
plugins/metadata/lib/metadata_plugin/base.rb
| ... | ... | @@ -50,7 +50,8 @@ class MetadataPlugin::Base < Noosfero::Plugin |
| 50 | 50 | Array(values).each do |value| |
| 51 | 51 | value = value.call(object, plugin) if value.is_a? Proc rescue nil |
| 52 | 52 | next if value.blank? |
| 53 | - r << tag(:meta, key_attr => key, value_attr => CGI.escape_html(value.to_s)) | |
| 53 | + value = h value unless value.html_safe? | |
| 54 | + r << tag(:meta, {key_attr => key, value_attr => value.to_s}, false, false) | |
| 54 | 55 | end |
| 55 | 56 | end |
| 56 | 57 | end | ... | ... |
plugins/metadata/lib/metadata_plugin/url_helper.rb
| ... | ... | @@ -7,7 +7,8 @@ module MetadataPlugin::UrlHelper |
| 7 | 7 | def og_url_for options |
| 8 | 8 | options.delete :port |
| 9 | 9 | options[:host] = self.og_domain |
| 10 | - Noosfero::Application.routes.url_helpers.url_for options | |
| 10 | + url = Noosfero::Application.routes.url_helpers.url_for options | |
| 11 | + url.html_safe | |
| 11 | 12 | end |
| 12 | 13 | |
| 13 | 14 | def og_profile_url profile | ... | ... |
plugins/metadata/test/functional/content_viewer_controller_test.rb
| ... | ... | @@ -48,6 +48,14 @@ class ContentViewerControllerTest < ActionController::TestCase |
| 48 | 48 | assert_tag tag: 'meta', attributes: { property: 'og:image', content: /\/images\/x.png/ } |
| 49 | 49 | end |
| 50 | 50 | |
| 51 | + should 'escape utf8 characters correctly' do | |
| 52 | + a = TinyMceArticle.create(name: 'Article to be shared with images', body: 'This article should be shared with all social networks <img src="/images/ç.png" />', profile: profile) | |
| 53 | + | |
| 54 | + get :view_page, profile: profile.identifier, page: [ a.name.to_slug ] | |
| 55 | + assert_tag tag: 'meta', attributes: { property: 'og:image', content: /\/images\/%C3%A7.png/ } | |
| 56 | + end | |
| 57 | + | |
| 58 | + | |
| 51 | 59 | should 'render not_found page properly' do |
| 52 | 60 | assert_equal false, Article.exists?(:slug => 'non-existing-page') |
| 53 | 61 | assert_nothing_raised do | ... | ... |
test/unit/article_test.rb
| ... | ... | @@ -1497,6 +1497,17 @@ class ArticleTest < ActiveSupport::TestCase |
| 1497 | 1497 | assert_includes a.body_images_paths, 'http://test.com/noosfero.png' |
| 1498 | 1498 | end |
| 1499 | 1499 | |
| 1500 | + should 'escape utf8 characters correctly' do | |
| 1501 | + Environment.any_instance.stubs(:default_hostname).returns('noosfero.org') | |
| 1502 | + a = build TinyMceArticle, profile: @profile | |
| 1503 | + a.body = 'Noosfero <img src="http://noosfero.com/cabeça.png" /> ' | |
| 1504 | + assert_includes a.body_images_paths, 'http://noosfero.com/cabe%C3%A7a.png' | |
| 1505 | + | |
| 1506 | + # check if after save (that is, after xss_terminate run) | |
| 1507 | + a.save! | |
| 1508 | + assert_includes a.body_images_paths, 'http://noosfero.com/cabe%C3%A7a.png' | |
| 1509 | + end | |
| 1510 | + | |
| 1500 | 1511 | should 'get absolute images paths in article body' do |
| 1501 | 1512 | Environment.any_instance.stubs(:default_hostname).returns('noosfero.org') |
| 1502 | 1513 | a = build TinyMceArticle, :profile => @profile | ... | ... |