Repository navigation
Backport rbs 4.1 fixes for the supported range - #230
Merged
Merged
Conversation
rbs before 4.1 declares `Resolv#initialize` as taking a single `Resolv::Hosts | Resolv::DNS`, but the method's runtime contract is an *array* of resolvers (the default is `[Hosts.new, DNS.new]`, and `each_address` iterates the argument). Against that signature the idiomatic `Resolv.new([Resolv::Hosts.new, dns])` has no accepting overload and `call.argument-type-mismatch` false-fires — measured on Mastodon's `app/lib/request.rb:296`, the one diagnostic that the rbs 4.1.0 bump (#225) removed from the corpus A/B. Upstream fixed the signature in 4.1 (ruby/rbs#2960). The gemspec still supports `>= 3.0, < 5.0`, so this overlay backports the corrected overload for the older releases, following the StringScanner#[] precedent: appended via `| ...`, so on 4.1 it duplicates the upstream overload at no cost, and `resolv` is already in DEFAULT_LIBRARIES so the reopen never introduces the class on its own. Verified on rbs 4.0.3 against Mastodon: without the overlay the FP fires (88 diagnostics on request.rb), with it the FP alone disappears (87). Drop the file when the rbs floor reaches 4.1.
Rigor hands three kinds of externally-controlled content to `RBS::Parser.parse_signature`: project `signature_paths:` files, the quarantine detector's re-parse of the same files, and plugin-synthesized virtual RBS (which can echo project bytes). None of them checked the encoding first, and the parser's behaviour on invalid UTF-8 is bad on every release in the supported range, in two different ways: - On rbs 4.1 (the pinned version), `RBS::Parser.magic_comment`'s regex raises a bare `ArgumentError` before the lexer runs. That is not a `ParsingError`, so it sails through every existing rescue and aborts `rigor check` with a stack trace. Found live: the new virtual-RBS spec crashed through `parseable_rbs?` exactly this way. - On the older releases the gemspec supports (`>= 3.0, < 5.0`), the C lexer could infinite-loop or abort the process on an invalid byte in a comment (fixed upstream in ruby/rbs#2973 / #2983) — a hang no rescue can catch, which is precisely the failure mode the quarantine's fail-soft rescues cannot absorb. The guard is one predicate (`invalid_encoding?`) run before every parse of external content, on every rbs version: a project file is quarantined with the existing loud warning (the note carries the path itself, since a ParsingError message embeds its own `path:line:` prefix and the warn composer prints only that element), and a virtual contribution is skipped like a parse failure. Rigor-synthesized strings (namespace stubs, sig-gen validity checks of its own output) stay unguarded — their bytes come from already-parsed names. Verified across the range: full suite on rbs 4.1.0, and the `spec/rigor/environment` compat set on 4.0.3 and 3.10.4 — the CI job's exact scope, run locally per the 0.3.1 lesson.
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.
Follow-up to #225, from the "what can be backported for rbs 3.x/4.0 users" review. The gemspec supports
rbs >= 3.0, < 5.0; these two changes bring 4.1's fixes to the rest of that range.1. Resolv#initialize core overlay
rbs before 4.1 types
Resolv#initializeas taking a single resolver, but the runtime contract is an array — so the idiomaticResolv.new([Resolv::Hosts.new, dns])false-firedcall.argument-type-mismatch. Upstream fixed the signature in 4.1 (ruby/rbs#2960); the overlay backports the corrected overload, following theStringScanner#[]precedent (appended via| ..., duplicate-at-no-cost on 4.1,resolvalready inDEFAULT_LIBRARIES).Proof on rbs 4.0.3 against Mastodon
app/lib/request.rb: without the overlay 88 diagnostics including the Resolv FP at :296; with it 87, the FP alone gone.2. Invalid-UTF-8 quarantine before the parser
Writing the spec for this exposed that the hazard is live on the pinned rbs too, in a different shape:
RBS::Parser.magic_comment's regex raises a bareArgumentErroron invalid bytes — not aParsingError, so it escapes every existing rescue and abortsrigor checkwith a stack trace. (Observed directly: the new virtual-RBS spec crashed throughparseable_rbs?before the guard existed.)The guard is one
valid_encoding?predicate run before every parse of externally-controlled content (project sig files, the quarantine detector, virtual RBS). A bad project file lands in the existing loud quarantine warning; a bad virtual contribution is skipped like a parse failure. Rigor-synthesized strings stay unguarded.The same pattern applies to sig-gen's re-parse of existing project
.rbs(layout_index.rb/writer.rb) — deliberately out of scope here (different command path, different UX call on how to refuse); flagged as a follow-up task.Verification
make verifygreen on pinned rbs 4.1.0 (8,289 examples),git diff --checkclean.spec/rigor/environment(therbs-compatCI job's exact scope) run locally on rbs 4.0.3 and rbs 3.10.4: 195 examples, 0 failures each — both ends of the range reproduced before push, per the 0.3.1 lesson.