Skip absent optional steps when composing structured error messages#2751
Open
ericproulx wants to merge 1 commit into
Open
Skip absent optional steps when composing structured error messages#2751ericproulx wants to merge 1 commit into
ericproulx wants to merge 1 commit into
Conversation
Danger ReportNo issues found. |
305b403 to
8eb2ac7
Compare
`Grape::Exceptions::Base#compose_message` looped over every entry in
`MESSAGE_STEPS` (problem, summary, resolution) and translated each one
individually. For an optional step a message does not define — e.g.
`summary` on `invalid_message_body` — `Grape::Util::Translation#translate`
falls back to returning the scoped key path as a string. That string is
non-blank, so it passed the `detail.present?` guard and leaked verbatim:
Summary:
grape.errors.messages.invalid_message_body.summary
Gate each step on whether the already-resolved message hash defines it.
The resolved hash reflects the :en fallback, so locale fallback for
present steps is preserved, and the existing key-path behaviour for a
fully-unresolvable top-level message is untouched.
Fixes #2748.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8eb2ac7 to
9f464be
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #2748.
Problem
Grape::Exceptions::Base#compose_messageloops over every entry inMESSAGE_STEPS(problem,summary,resolution) and translates each one individually. For an optional step a message doesn't define — e.g.summaryoninvalid_message_body—Grape::Util::Translation#translatefalls back to returning the scoped key path as a string. That string is non-blank, so it passes thedetail.present?guard and leaks verbatim into the rendered message:Fix
Gate each step on whether the already-resolved message hash defines it (
short_message.key?(step.to_sym)). The resolved hash already reflects the:enfallback, so:summary) are skipped instead of leaking the key path;:enlocale fallback for present steps is preserved (verified under a:delocale with no override — still renders Problem + Resolution from:en, no leak);spec/grape/exceptions/base_spec.rb"returns the scoped translation key as a string") is untouched — that path returns before the Hash branch;translateitself is unchanged, so validators andValidationErrorsare unaffected.Tests
Added specs in
spec/grape/exceptions/base_spec.rb: the leak is gone forinvalid_message_body, whilemissing_vendor_option(which legitimately definessummary) still renders all three steps.spec/grape/exceptions/is green (96 examples, 0 failures).Reported by @dcasillano.
🤖 Generated with Claude Code