Catch StandardError during Base64 decoding in message encryptor. #51846
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.
Motivation / Background
RubyGems.org got recently facing 500 HTTP responses since user mangled cookies (app uses cookie store) and provided custom value which (thanks to series of edge-cases) ended up triggering
NoMethodError: undefined method unpack1' for nil
raised down fromBase64
class. Per my understanding there is no way to catch this in application and this needs to be fixed in Rails itself.Detail
I have tracked this issue down to
ActiveSupport::Messages::Code
class expectingBase64
failing withArgumentError
only. ButBase64
can fail also with other exceptions when wrong input type provided. In the shared casenil
is actually passed toBase64.strict_decode64
resulting intoNoMethodError
. I have changed rescue to handleStandardError
since similar approach is used few lines bellow indeserialize
method.As an side effect, this tests this unhappy branch of
decode
method which wasn't covered by active support test suite before.I have originally considered updating Base64 to raise
ArgumentError
whennil
passed, but there are other values which can still raise similar error and it would be needed to check if passed object responds to methods needed for Base64 calculation and that's not usually happening in stdlibs.