Download as
Browse Code »

Commit e1d56f4be78aa13f5dc7a200e72095d56322d81c

Authored by Braulio Bhavamitra
2 parents 1cc6c422 8235fc9d
Exists in master and in 6 other branches article_hit_count, checkbox_to_user_can_edit_page, communities_ratings, fix_cloned_article_parent, fix_gallery_image_url, notification_plugin

Merge branch 'metadata-plugin' into 'master' 

metadata: marks urls as html safe

This also fix a core bug of double escaping urls.

See merge request !653
Showing 9 changed files with 36 additions and 14 deletions   Show diff stats
app/models/article.rb
  Diff comments   View file @e1d56f4
... ... @@ -743,9 +743,10 @@ class Article < ActiveRecord::Base
743 743 end
744 744  
745 745 def body_images_paths
746   - require 'uri'
747 746 Nokogiri::HTML.fragment(self.body.to_s).css('img[src]').collect do |i|
748   - (self.profile && self.profile.environment) ? URI.join(self.profile.environment.top_url, URI.escape(i['src'])).to_s : i['src']
  747 + src = i['src']
  748 + src = URI.escape src if self.new_record? # xss_terminate runs on save
  749 + (self.profile && self.profile.environment) ? URI.join(self.profile.environment.top_url, src).to_s : src
749 750 end
750 751 end
751 752  
... ...
plugins/metadata/lib/ext/article.rb
  Diff comments   View file @e1d56f4
... ... @@ -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
  Diff comments   View file @e1d56f4
... ... @@ -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
  Diff comments   View file @e1d56f4
... ... @@ -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
  Diff comments   View file @e1d56f4
... ... @@ -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
  Diff comments   View file @e1d56f4
... ... @@ -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
  Diff comments   View file @e1d56f4
... ... @@ -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
  Diff comments   View file @e1d56f4
... ... @@ -48,6 +48,14 @@ class ContentViewerControllerTest &lt; 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
  Diff comments   View file @e1d56f4
... ... @@ -1497,6 +1497,17 @@ class ArticleTest &lt; 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
... ...