Fix crash on \c/\M- escapes with an invalid byte on a UTF-8 source#1092
Open
nabsei wants to merge 1 commit into
Open
Fix crash on \c/\M- escapes with an invalid byte on a UTF-8 source#1092nabsei wants to merge 1 commit into
nabsei wants to merge 1 commit into
Conversation
…on a UTF-8 source read_post_meta_or_ctrl_char, slash_c_char and slash_m_char all called .ord on @escape without controlling its encoding. @escape usually carries the source buffer's own encoding (eg. UTF-8 by default), and either that byte directly, or the result of encode_escape() applying the \c/\M- bit operations to it and force_encoding-ing the result back to the source's encoding, can be invalid on its own in that encoding (eg. \c\xFF, whose \x escape decodes to byte 0xFF). Parsing such a literal from a UTF-8 source then crashed with an unhandled "ArgumentError: invalid byte sequence in UTF-8" instead of the existing, already-handled :invalid_encoding diagnostic that similar escapes like \M-a already raise gracefully. Read @escape's byte value with String#b instead, since a raw byte is what \c/\M- escapes actually operate on. The existing lexer-level test for these escapes only exercised an ascii-8bit source, which never hit this; added a parser-level test using the common, UTF-8-by-default case that reproduces the crash. Relates to rubocop/rubocop#15457 Co-authored-by: Claude <noreply@anthropic.com>
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
1 participant
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.
What
Parsing a control/meta escape (
\c,\M-) whose target byte isn't valid on its own in the source's encoding — eg.\c\xFFfrom a UTF-8 source — crashes with an unhandledArgumentError: invalid byte sequence in UTF-8, instead of the existing, already-handled:invalid_encodingdiagnostic that similar escapes like\M-aalready raise gracefully.Reported downstream at rubocop, where a Ruby source file simply containing
/\c\xFF/as a regex literal (part of MRI's own regexp test suite) crashes rubocop entirely: rubocop/rubocop#15457Why
read_post_meta_or_ctrl_char,slash_c_charandslash_m_charall call.ordon@escapewithout controlling its encoding.@escapeusually carries the source buffer's own encoding (UTF-8 by default), and either that byte directly, or the result ofencode_escape()applying the\c/\M-bit operations to it andforce_encoding-ing the result back to the source's encoding, can be invalid on its own in that encoding.\cand\M-escapes operate on the raw byte value of their target, not on a Unicode codepoint, so this reads@escape's value viaString#binstead, which never raises regardless of whether the byte happens to be valid in the source's encoding.Verification
test_control_meta_escape_chars_in_regexp_from_utf8_source) covering all 7\c/\M-/\C-+\xFFcombinations from a UTF-8 source (the common case) — confirmed it reproduces the exact crash on unmodified code and passes with the fix.test_meta_control_hex_escaped_char, targeting an old Ruby version) and the existing parser-level test (test_control_meta_escape_chars_in_regexp__since_31) both still pass unmodified — the latter only exercises anascii-8bit-forced source, which is why it never caught this in the first place.\ca,\x41,\x7f) and already-gracefully-erroring ones (\M-a,\M-\C-a) are unaffected.I'm not marking this as closing rubocop/rubocop#15457 directly since that repo depends on a released version of this gem; leaving it linked for context instead.
Disclosure: this PR was prepared with the assistance of Claude Code (Anthropic). The investigation, root-cause diagnosis, fix, and verification steps described above were done and checked by me.