Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fixes strings being interpolated multiple times
Similarly to #599, I've observed issues issues where untrusted user input that includes interpolation patterns gets unintentionally interpolated and leads to bogus `I18n::MissingInterpolationArgument` exceptions. This happens when multiple lookups are required for a key to be resolved, which is common when resolving defaults, or resolving a key that itself resolves to a Symbol. As an example let's consider these translations, common for Rails apps: ```yaml en: activerecord: errors: messages: taken: "%{value} has already been taken" ``` If the `value` given to interpolate ends up containing interpolation characters, and Rails specifies default keys (as [described here](https://guides.rubyonrails.org/i18n.html#error-message-scopes)), resolving those defaults will cause a `I18n::MissingInterpolationArgument` to be raised: ```rb I18n.t('activerecord.errors.models.organization.attributes.name.taken', value: '%{dont_interpolate_me}', default: [ :"activerecord.errors.models.organization.taken", :"activerecord.errors.messages.taken" ] ) ``` Raises: ``` I18n::MissingInterpolationArgument: missing interpolation argument :dont_interpolate_me in "%{dont_interpolate_me}" ({:value=>"%{dont_interpolate_me}"} given) ``` Instead of this, we'd expect the translation to resolve to: ``` %{dont_interpolate_me} has already been taken ``` This behaviour is caused by the way that recursive lookups work: whenever a key can't be resolved to a string directly, the `I18n.translate` method is called either to walk through the defaults specified, or if a Symbol is matched, to try to resolve that symbol. This results in interpolation being executed twice for recursive lookups... once on the pass that finally resolves to a string, and again on the original call to `I18n.translate`. A "proper" fix here would likely revolve around decoupling key resolution from interpolation... it feels odd to me that the `resolve_entry` method calls `I18n.translate`... however I see this as a fundamental change beyond the scope of this fix. Instead I'm proposing to add a new reserved key `skip_interpolation` that gets passed down into every recursive call of `I18n.translate` and instructs the method to skip interpolation. This ensures that only the initial `I18n.translate` call is the one that gets its string interpolated.
- Loading branch information