From 8235fc9d9cb92ae0d98145ed4f5e79dedcd6c929 Mon Sep 17 00:00:00 2001 From: Braulio Bhavamitra Date: Wed, 12 Aug 2015 20:27:37 -0300 Subject: [PATCH] metadata: marks urls as html safe --- app/models/article.rb | 5 +++-- plugins/metadata/lib/ext/article.rb | 10 +++++----- plugins/metadata/lib/ext/product.rb | 4 ++-- plugins/metadata/lib/ext/profile.rb | 4 ++-- plugins/metadata/lib/ext/uploaded_file.rb | 2 +- plugins/metadata/lib/metadata_plugin/base.rb | 3 ++- plugins/metadata/lib/metadata_plugin/url_helper.rb | 3 ++- plugins/metadata/test/functional/content_viewer_controller_test.rb | 8 ++++++++ test/unit/article_test.rb | 11 +++++++++++ 9 files changed, 36 insertions(+), 14 deletions(-) diff --git a/app/models/article.rb b/app/models/article.rb index bdb304d..9d974e4 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -742,9 +742,10 @@ class Article < ActiveRecord::Base end def body_images_paths - require 'uri' Nokogiri::HTML.fragment(self.body.to_s).css('img[src]').collect do |i| - (self.profile && self.profile.environment) ? URI.join(self.profile.environment.top_url, URI.escape(i['src'])).to_s : i['src'] + src = i['src'] + src = URI.escape src if self.new_record? # xss_terminate runs on save + (self.profile && self.profile.environment) ? URI.join(self.profile.environment.top_url, src).to_s : src end end diff --git a/plugins/metadata/lib/ext/article.rb b/plugins/metadata/lib/ext/article.rb index ac53071..e864363 100644 --- a/plugins/metadata/lib/ext/article.rb +++ b/plugins/metadata/lib/ext/article.rb @@ -12,9 +12,9 @@ class Article end, title: proc{ |a, plugin| "#{a.title} - #{a.profile.name}" }, image: proc do |a, plugin| - img = a.body_images_paths - img = "#{a.profile.environment.top_url}#{a.profile.image.public_filename}" if a.profile.image if img.blank? - img ||= MetadataPlugin.config[:open_graph][:environment_logo] rescue nil if img.blank? + img = a.body_images_paths.map! &:html_safe + img = "#{a.profile.environment.top_url}#{a.profile.image.public_filename}".html_safe if a.profile.image if img.blank? + img ||= MetadataPlugin.config[:open_graph][:environment_logo].html_safe rescue nil if img.blank? img end, see_also: [], @@ -31,10 +31,10 @@ class Article card: 'summary', description: proc do |a, plugin| description = a.body.to_s || a.environment.name - CGI.escapeHTML(plugin.helpers.truncate(plugin.helpers.strip_tags(description), length: 200)) + plugin.helpers.truncate plugin.helpers.strip_tags(description), length: 200 end, title: proc{ |a, plugin| "#{a.title} - #{a.profile.name}" }, - image: proc{ |a, plugin| a.body_images_paths }, + image: proc{ |a, plugin| a.body_images_paths.map! &:html_safe }, } metadata_spec namespace: :article, key_attr: :property, tags: { diff --git a/plugins/metadata/lib/ext/product.rb b/plugins/metadata/lib/ext/product.rb index a6ff72e..b2edcb2 100644 --- a/plugins/metadata/lib/ext/product.rb +++ b/plugins/metadata/lib/ext/product.rb @@ -14,8 +14,8 @@ class Product description: proc{ |p, plugin| ActionView::Base.full_sanitizer.sanitize p.description }, image: proc do |p, plugin| - img = "#{p.environment.top_url}#{p.image.public_filename}" if p.image - img = "#{p.environment.top_url}#{p.profile.image.public_filename}" if img.blank? and p.profile.image + img = "#{p.environment.top_url}#{p.image.public_filename}".html_safe if p.image + img = "#{p.environment.top_url}#{p.profile.image.public_filename}".html_safe if img.blank? and p.profile.image img ||= MetadataPlugin.config[:open_graph][:environment_logo] rescue nil if img.blank? img end, diff --git a/plugins/metadata/lib/ext/profile.rb b/plugins/metadata/lib/ext/profile.rb index 401a8ed..72ad40d 100644 --- a/plugins/metadata/lib/ext/profile.rb +++ b/plugins/metadata/lib/ext/profile.rb @@ -5,8 +5,8 @@ class Profile metadata_spec namespace: :og, tags: { type: proc{ |p, plugin| plugin.context.params[:og_type] || MetadataPlugin.og_types[:profile] || :profile }, image: proc do |p, plugin| - img = "#{p.environment.top_url}#{p.image.public_filename}" if p.image - img ||= MetadataPlugin.config[:open_graph][:environment_logo] rescue nil if img.blank? + img = "#{p.environment.top_url}#{p.image.public_filename}".html_safe if p.image + img ||= MetadataPlugin.config[:open_graph][:environment_logo].html_safe rescue nil if img.blank? img end, title: proc{ |p, plugin| if p.nickname.present? then p.nickname else p.name end }, diff --git a/plugins/metadata/lib/ext/uploaded_file.rb b/plugins/metadata/lib/ext/uploaded_file.rb index cb42251..ea884ae 100644 --- a/plugins/metadata/lib/ext/uploaded_file.rb +++ b/plugins/metadata/lib/ext/uploaded_file.rb @@ -13,7 +13,7 @@ class UploadedFile plugin.og_url_for url end, title: proc{ |u, plugin| u.title }, - image: proc{ |u, plugin| "#{u.environment.top_url}#{u.public_filename}" if u.image? }, + image: proc{ |u, plugin| "#{u.environment.top_url}#{u.public_filename}".html_safe if u.image? }, description: proc{ |u, plugin| u.abstract || u.title }, } diff --git a/plugins/metadata/lib/metadata_plugin/base.rb b/plugins/metadata/lib/metadata_plugin/base.rb index cfa9f5e..d95602f 100644 --- a/plugins/metadata/lib/metadata_plugin/base.rb +++ b/plugins/metadata/lib/metadata_plugin/base.rb @@ -50,7 +50,8 @@ class MetadataPlugin::Base < Noosfero::Plugin Array(values).each do |value| value = value.call(object, plugin) if value.is_a? Proc rescue nil next if value.blank? - r << tag(:meta, key_attr => key, value_attr => CGI.escape_html(value.to_s)) + value = h value unless value.html_safe? + r << tag(:meta, {key_attr => key, value_attr => value.to_s}, false, false) end end end diff --git a/plugins/metadata/lib/metadata_plugin/url_helper.rb b/plugins/metadata/lib/metadata_plugin/url_helper.rb index ed8a735..e19903d 100644 --- a/plugins/metadata/lib/metadata_plugin/url_helper.rb +++ b/plugins/metadata/lib/metadata_plugin/url_helper.rb @@ -7,7 +7,8 @@ module MetadataPlugin::UrlHelper def og_url_for options options.delete :port options[:host] = self.og_domain - Noosfero::Application.routes.url_helpers.url_for options + url = Noosfero::Application.routes.url_helpers.url_for options + url.html_safe end def og_profile_url profile diff --git a/plugins/metadata/test/functional/content_viewer_controller_test.rb b/plugins/metadata/test/functional/content_viewer_controller_test.rb index 0b48971..9c136be 100644 --- a/plugins/metadata/test/functional/content_viewer_controller_test.rb +++ b/plugins/metadata/test/functional/content_viewer_controller_test.rb @@ -48,6 +48,14 @@ class ContentViewerControllerTest < ActionController::TestCase assert_tag tag: 'meta', attributes: { property: 'og:image', content: /\/images\/x.png/ } end + should 'escape utf8 characters correctly' do + a = TinyMceArticle.create(name: 'Article to be shared with images', body: 'This article should be shared with all social networks ', profile: profile) + + get :view_page, profile: profile.identifier, page: [ a.name.to_slug ] + assert_tag tag: 'meta', attributes: { property: 'og:image', content: /\/images\/%C3%A7.png/ } + end + + should 'render not_found page properly' do assert_equal false, Article.exists?(:slug => 'non-existing-page') assert_nothing_raised do diff --git a/test/unit/article_test.rb b/test/unit/article_test.rb index 68ae33f..785bb50 100644 --- a/test/unit/article_test.rb +++ b/test/unit/article_test.rb @@ -1497,6 +1497,17 @@ class ArticleTest < ActiveSupport::TestCase assert_includes a.body_images_paths, 'http://test.com/noosfero.png' end + should 'escape utf8 characters correctly' do + Environment.any_instance.stubs(:default_hostname).returns('noosfero.org') + a = build TinyMceArticle, profile: @profile + a.body = 'Noosfero ' + assert_includes a.body_images_paths, 'http://noosfero.com/cabe%C3%A7a.png' + + # check if after save (that is, after xss_terminate run) + a.save! + assert_includes a.body_images_paths, 'http://noosfero.com/cabe%C3%A7a.png' + end + should 'get absolute images paths in article body' do Environment.any_instance.stubs(:default_hostname).returns('noosfero.org') a = build TinyMceArticle, :profile => @profile -- libgit2 0.21.2