Fixes strings being interpolated multiple times #699
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similarly to #599, I've observed issues issues where untrusted user input that includes interpolation patterns gets unintentionally interpolated and leads to bogus
I18n::MissingInterpolationArgumentexceptions.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:
If the
valuegiven to interpolate ends up containing interpolation characters, and Rails specifies multiple default keys (as described here) aI18n::MissingInterpolationArgumentwill be raised:Instead of this, we'd expect the translation to resolve to:
This behaviour is caused by the way that recursive lookups work: whenever a key can't be resolved to a string directly the
I18n.translatemethod 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
I18n.translatepass that finally resolves to a string, and again on the original call toI18n.translate.A "proper" fix here would likely revolve around decoupling key resolution from interpolation. It feels odd to me that the
resolve_entrymethod callsI18n.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_interpolationthat gets passed down into every recursive call ofI18n.translateand instructs the method to skip interpolation.This ensures that only the initial
I18n.translatecall is the one that gets its string interpolated.