kpumuk / meta-tags

Search Engine Optimization (SEO) for Ruby on Rails applications.
MIT License
2.73k stars 275 forks source link

Special chars are encoded #103

Closed chr1s1 closed 8 years ago

chr1s1 commented 8 years ago

Hi,

I experienced a weird behaviour of the meta_tags title:

If i have a title like "This is a awesome title & it is short" then it will be transformed to "This is a awesome title & it is short" which is not what I want as browsers like chrome actually show the &. I tried to put an decode function around the title, but nothing seems to work. Any idea where this comes from and how I am able to change it?

Chris

chr1s1 commented 8 years ago

updated description as & was transformed to &

danielfriis commented 8 years ago

+1

ishields commented 8 years ago

+1 Any solution to this problem? I reverted to 2.0.0 and this problem doesn't occur.

phlegx commented 8 years ago

+1 same problem in keyword and description if I use e.g. John's

Put this code in meta_tags.rb file under config/initializers and restart your app:

module MetaTags
  # Module contains helpers that normalize text meta tag values.
  module TextNormalizer

    def self.truncate(string, limit = nil, natural_separator = ' ')
      string = helpers.truncate(string, length: limit, separator: natural_separator, omission: '', escape: false) if limit
      string
    end

    def self.normalize_keywords(keywords)
      return '' if keywords.blank?
      keywords = cleanup_strings(keywords).each(&:downcase!).map(&:html_safe)
      separator = strip_tags MetaTags.config.keywords_separator
      keywords = truncate_array(keywords, MetaTags.config.keywords_limit, separator)
      safe_join(keywords, separator)
    end

  end
end

This works only for version 2.1.0!!!

The problem is, that the strings gets encoded twice. Truncate gets as parameter an escaped string and don't needs to escape the string again. safe_join needs all keywords as html_safe because the keywords has been encoded with cleanup_strings just before.

miharekar commented 8 years ago

@phlegx that doesn't solve the title problem. You have #85 and #105 open for that

prosanelli commented 8 years ago

This is now an issue for everyone that wants to upgrade to the Rails 4.2.5.1 security patch

phlegx commented 8 years ago

@mrfoto why? My title is shown correctly with the above code. I take a look at the linked issues. Thx @prosanelli, here more about the patch http://weblog.rubyonrails.org/2016/1/25/Rails-5-0-0-beta1-1-4-2-5-1-4-1-14-1-3-2-22-1-and-rails-html-sanitizer-1-0-3-have-been-released/

Rails rails-html-sanitizer gem (included in Rails 4.2 and above) has a security alert. This gem includes used method strip_tags.

Put this code in meta_tags.rb file under config/initializers and restart your app to test:

module MetaTags
  # Module contains helpers that normalize text meta tag values.
  module TextNormalizer

    def self.strip_tags(string)
      ERB::Util.html_escape helpers.sanitize(string)
    end

  end
end
siegy22 commented 8 years ago

@phlegx So the solution would be chaning https://github.com/Elektron1c97/meta_tags-rails/blob/master/lib/meta_tags-rails/text_normalizer.rb#L74 <= this line

to

def self.strip_tags
   ERB::Util.html_escape helpers.sanitize(string)
end

?

kpumuk commented 8 years ago

Should be resolved in https://github.com/kpumuk/meta-tags/pull/120

phlegx commented 7 years ago

Hi! In the new version 2.4.0 I get this description and this keywords as example:

Description tag:

Don&#39;t ...

Keywords Tag

keyword1, don&amp;#39;t

The description tag is ok but not the keyword because & of &#39; gets re-sanitized. Any idea?

phlegx commented 7 years ago

I found that the following code at text_normalizer.rb#L98 re-encodes the keywords:

def self.safe_join(array, sep = $,)
    helpers.safe_join(array, sep)
end

This function is called by text_normalizer.rb#L57.

def self.normalize_keywords(keywords)
      return '' if keywords.blank?
      keywords = cleanup_strings(keywords).each(&:downcase!)
      separator = strip_tags MetaTags.config.keywords_separator

      keywords = truncate_array(keywords, MetaTags.config.keywords_limit, separator)
      # variable keywords should be an array of raw elements because cleanup is always done on line 59.
      safe_join(keywords, separator)
end
phlegx commented 7 years ago

Hi @siegy22 and @kpumuk! I use latest version 2.4.0. I have solved the problem described in the last comment with this code:

module MetaTags

  module TextNormalizer

    def self.normalize_keywords(keywords)
      return '' if keywords.blank?
      keywords = cleanup_strings(keywords).each(&:downcase!)
      separator = strip_tags MetaTags.config.keywords_separator
      keywords = truncate_array(keywords, MetaTags.config.keywords_limit, separator)
      # Replace safe_join with simple join.
      # safe_join produces from don&#39;t the re-escaped string don&amp;#39;t
      keywords.join(separator)
    end

  end

  class Tag

    def render(view)
      # Method tag don't accepts symbols as option values.
      attributes.each { |k, v| attributes[k] = v.to_s if v.is_a?(Symbol) }
      # Keywords don't should be re-escaped.
      escape = attributes[:name] == 'keywords' ? false : true
      # Method tag re-escapes keywords values.
      # tag(name, options = nil, open = false, escape = true)
      view.tag(name, attributes, false, escape)
    end

  end

end

Whit this changes I have all meta tags (title, description, keywords, etc.) escaped in the right way. This is not an elegant way but it shows how the problem can be solved. The attributes as a symbol can be changed in file renderer.rb using a string instead of a symbol e.g.:

render_with_normalization(tags, 'description')
render_with_normalization(tags, 'keywords')