#856 ✓resolved
Tomoaki Hayasaka

"``<ht>``" result is html-escaped

Reported by Tomoaki Hayasaka | November 17th, 2010 @ 07:06 PM | in Hobo 1.3 (Rails 3)

rails-3.0.2, i18n-0.4.2, hobo-1.3.0.pre17

Result of "<ht>" is marked non html_safe in some cases. Especially with i18n-0.4.2, it is always marked so. For example, a page fragment:

<ht key="no.translations.available"><span>span</span></ht>

will be "translated" to "<span>span</span>" (it's desired) and it is marked "not html_safe" (not desired), users will see "<span>span</span>" in the browser. This sort of fragments are used at many places in rapid.

With i18n-0.4.1, this issue was hidden if neither translation nor interpolation was done, because options[:default] is returned as is, and html_safe flag is preserved. But anyway, if either translation or interpolation was done, the issue arises.

Fix:

diff --git a/hobo/lib/hobo/helper/translations.rb b/hobo/lib/hobo/helper/translations.rb
index cccd81d..8e91f9e 100644
--- a/hobo/lib/hobo/helper/translations.rb
+++ b/hobo/lib/hobo/helper/translations.rb
@@ -36,7 +36,7 @@ module Hobo
           if key.has_key?(:default) && !key[:default].blank?
             Rails.logger.warn "hobo-i18n: 'default' should not be used as an attribute on the ht-tag. If used, then you need to make sure that the tags inner-contents are not used. These are normally treated as defaults automatically, but if there is a default attribute then that inner-content will be hidden from this method - and will not be replaced with the translation found."
           end
-          defaults = options[:default];
+          defaults = options[:default]
           # Swap key and options, remove options[:key]
           options = key
           key = options.delete(:key) # returns value for options[:key] as well as deleting it
@@ -87,7 +87,7 @@ module Hobo
           key_prefix ? translation.to_str+key_prefix : translation
         else
           "translation invalid: #{key}"
-        end
+        end.html_safe
       end
 
     end

However this is just a quick fix. Following Rails3's TranslationHelper#translate convention will likely to be a right way.

Comments and changes to this ticket

  • Bryan Larsen

    Bryan Larsen November 19th, 2010 @ 09:42 PM

    • State changed from “new” to “open”
    • Milestone set to “Hobo 1.3 (Rails 3)”
    • Milestone order changed from “197920” to “0”
  • Domizio Demichelis

    Domizio Demichelis November 20th, 2010 @ 12:50 AM

    Tomoaki,
    I don't think that the default from an ht tag should be automatically considered html_safe.

    What if:

    <ht key="no.translations.available">
  <an-unsafe-user-tag>a default</an-unsafe-user-tag>
</ht>
    The goal of auto escaping is protecting from accidentally usafe output, and if we mark it as html_safe wouldn't we bypass that protection?

    I think that the user should explicitly return a html_safe string, so what about:

    <ht key="no.translations.available">
  <%= raw '<span>span</span>' %>
</ht>
    What do you think?
  • Domizio Demichelis

    Domizio Demichelis November 20th, 2010 @ 01:49 AM

    I found a minute to check it and I am confused about what you mean, because all the content of the ht tag is parsed as dryml content so the tag returns a html_safe string per se (because it's recognized as a static tag).

    And if you mean other user defined tag (instead of span), then your tag shoud be responsable of eventually escaping and return a html_safe string of its result.

    Am I missing something?

  • Domizio Demichelis

    Domizio Demichelis November 21st, 2010 @ 06:23 PM

    • State changed from “open” to “wontfix”

    I am in the middle of a mayor refactoring of the ht method and the hobo i18n stuff and I finally found what you are meaning :-).

    The example you supplied is the wrong example to show the behaviour, anyway, here are my conclusions:

    I18n.translate marks as html_safe any key that ends with (\b|.|_)html and as unsafe anything else, regardless what is in the interpolated variables.

    That sucks, because it should respect the html_safe status of the interpolated variables (escaping them or not), and the ending html should indicate ONLY the html_safe status of the string itself.

    The current behaviour is potentially dangerous and really annoying specially for dynamically created variables (which could be safe or not), so I will post the problem in the i18n-rails forum, and let's see what will hapen.

    Meanwhile, although it sucks, we better stick with the rails conventions, so we will use a 'html' ending key and we will have it explicitly marked as safe. That is far better than outputting everything as html_safe, which is potentially much more dangerous than the current behaviour.

    In conclusion, the ht method will use the translate helper and the I18n.translate will be patched in order to give us more info about the used keys (not only those used with ht) but the patch will NOT change the behaviour with respect to html_safe and html ending keys (which are indeed in the translate helper).

    If you have any translation key that includes html code, you have to name it with an ending _html and it will be marked html_safe.

  • Domizio Demichelis

    Domizio Demichelis November 22nd, 2010 @ 07:05 PM

    (from [eeddeb6c44e7f005fd5f3ee7b363cd86cdef6539]) I18n refactory [#856]

    • refactory of Hobo::Helper::Translations
    • the use of ht method/tag is reserved for keys starting with a model name
    • the first key passed to ht must be a model name, not a tableized name
    • added show_translation_keys config option that allows to show the used keys right in the output
    • added patches to I18n and TranslationHelper
    • use of translate helper instead I18n.t where possible https://github.com/tablatom/hobo/commit/eeddeb6c44e7f005fd5f3ee7b36...
  • Domizio Demichelis

    Domizio Demichelis November 23rd, 2010 @ 03:29 PM

    (from [cb5297afd253b6763ce2bd031cff95e6efc79e46]) fixed a security breach of ActionView::Helpers::TranslationHelper.translate [#856]

  • Domizio Demichelis

    Domizio Demichelis November 23rd, 2010 @ 05:24 PM

    • State changed from “wontfix” to “resolved”

    Tomoaki,
    despite what I stated before, I changed my mind and now it is safely behaving like you want, without suffixing the key with 'html'. Anyway you must pass html_safe interpolation variables if they include html code that you want in the output.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

People watching this ticket

Tags

Referenced by

Pages