Repository navigation
Compile non-strict templates Ractor-locally - #62
Closed
etiennebarrie wants to merge 110 commits into
Closed
etiennebarrie wants to merge 110 commits into
etiennebarrie wants to merge 110 commits into
Conversation
Unused since 45955ae replaced the generic clause-merge loop in `Relation::Merger` with explicit where/having merges and a replacement rule for the from clause.
…kground_connection` The parameter has been unused since the method was introduced in 5eab03f; the wait is intentionally unbounded.
Unused since cf7db80 decoupled `HashLookupTypeMap` from `TypeMap`; no caller passes an argument.
The instance variable has never been read since it was added in fe92250.
Its last callers were removed in 6b1515a.
Its last callers were removed in fd24e5b when the adapters moved to the `cast_result` path.
After `bin/rails new --skip-test`, a freshly generated app fails its own `bin/rubocop` (and the generated CI lint step) with a `Layout/EmptyLinesAroundBlockBody` offense in `config/ci.rb`, because the blank line separating the security checks from the test group is left behind at the end of the "Checks" group when no test group is generated. Skipped steps no longer leave stray blank lines behind: the check steps render without blank lines between them, and the blank line before the "Tests" group is only rendered together with the group itself. Every skip combination that keeps RuboCop in the app now renders a `config/ci.rb` that passes it with zero offenses. Same class as rails#58164, which fixed the routes injected by the authentication generator for the same cop family.
Only `cast` and `serialize` were exercised by Active Model's own suite. Line coverage over `activemodel/test/cases/` showed `type`, `binary?`, the `Data` branch of `cast`, `changed_in_place?`, and `Data#hex` all unreached. Adds coverage for: * `type` and `binary?`. * `serialize` returning `nil` for `nil`, and a `Binary::Data` otherwise. * `cast` unwrapping a `Binary::Data` back to its binary-encoded string. * `changed_in_place?` comparing the deserialized old value. * `Data#hex`, `#to_s`/`#to_str`, and comparison against a raw string. This brings `active_model/type/binary.rb` to full line coverage.
Since 26e78a2, ActiveSupport::TimeZone instances are frozen and Ractor-shareable so that TimeWithZone methods can be called from a non-main Ractor. However, any TimeWithZone method going through `formatted_offset(false)` -- `#to_s` (including string interpolation), `#rfc2822`, and `to_fs(:rfc822)` -- still raised: Ractor::IsolationError: can not access non-shareable objects in constant ActiveSupport::TimeZone::UTC_OFFSET_WITHOUT_COLON by non-main ractor. The `UTC_OFFSET_WITHOUT_COLON` constant is built with `String#tr`, whose return value is not covered by the frozen_string_literal magic comment, so the constant referenced a mutable string. Freezing it makes the constant shareable, and these methods now return the same strings on a non-main Ractor as on the main one.
etiennebarrie
force-pushed
the
actionview-eager-load-templates
branch
from
August 4, 2026 15:42
4d95b86 to
b262f80
Compare
etiennebarrie
force-pushed
the
actionview-frozen-non-strict-templates
branch
from
August 4, 2026 15:43
7f94f80 to
0940ce5
Compare
gmcgibbon
reviewed
Aug 5, 2026
|
|
||
| # Do not escape templates of these mime types. | ||
| class_attribute :escape_ignore_list, default: ["text/plain"] | ||
| class_attribute :escape_ignore_list, default: ["text/plain"].freeze |
Member
There was a problem hiding this comment.
This was solved upstream with an after_initialize freeze I believe.
| # freeze from leaking into other tests. | ||
| ActiveSupport::Ractors.make_shareable(Mime.instance_variable_get(:@lookup_by_extension)) | ||
|
|
||
| r = ActionView::FileSystemResolver.new(dir) |
Member
There was a problem hiding this comment.
Suggested change
| r = ActionView::FileSystemResolver.new(dir) | |
| resolver = ActionView::FileSystemResolver.new(dir) |
Minor comments. I think we can open these upstream.
The collector's on_change handler only added methods, so Mime::Type.unregister left a stale collector method behind. Calling it dispatched custom(Mime[:sym], ...) with a nil mime, which ActionController's Collector#custom turns into Type.new(nil) and a confusing Mime::Type::InvalidMimeType (nil is not a valid MIME type). Removing the method routes the call through Collector#method_missing instead, which raises a NoMethodError explaining the format must be registered as a MIME type first. No deprecation cycle: the stale method already raised, only the error message improves. Re-registering the type regenerates the method.
The lambda captures only the subclass, created after the assignment completes, so it can be made shareable.
etiennebarrie
force-pushed
the
actionview-eager-load-templates
branch
from
August 6, 2026 10:33
b262f80 to
93519a3
Compare
etiennebarrie
force-pushed
the
actionview-frozen-non-strict-templates
branch
from
August 6, 2026 10:33
0940ce5 to
4f4d19a
Compare
etiennebarrie
force-pushed
the
actionview-eager-load-templates
branch
from
August 6, 2026 14:05
93519a3 to
9414c43
Compare
etiennebarrie
force-pushed
the
actionview-frozen-non-strict-templates
branch
from
August 6, 2026 14:05
4f4d19a to
d4c0aa3
Compare
…rails#58209) `ActionText::Content#to_markdown` produces invalid CommonMark when a block-level element is nested inside an `<a>`. HTML allows an anchor to wrap block-level content because `<a>` has a transparent content model, but CommonMark defines link text as "a sequence of zero or more inline elements" (https://spec.commonmark.org/0.31.2/#links), so there is no Markdown representation of a link whose text is a block. The problem lives in `visit_a` in `actiontext/lib/action_text/markdown_conversion.rb`, which assumes its children produce inline output: "[#{inner}](#{encode_href(href)})" The block visitors violate that assumption: `visit_pre` emits a fenced code block, and `visit_p`, `visit_blockquote`, the heading visitors, and the list visitors all append a paragraph-separating blank line. A blank line inside link text severs the link, because Markdown resolves block structure before it parses inline links. CommonMark provides no way to encode a block-level element inside a link, so the best the conversion can do is approximate the intent of the original HTML. This change keeps the link and degrades its contents to inline text. `visit_a` now flattens its children's joined output onto a single line, replacing each run of newlines with a space, before wrapping it in link syntax. When the anchor does not produce a link because the `href` is missing or disallowed, the content passes through unchanged with its block structure intact. For example, `<a href="https://example.com"><pre>puts 1</pre></a>` previously produced [``` puts 1 ``` ](https://example.com) in which the fenced code block and the blank line sever the link. Now it produces [``` puts 1 ``` ](https://example.com) which is a valid inline code span inside a valid link, and renders as a link whose text is the code `puts 1`. Other block elements flatten the same way, keeping their markers as literal link text: `<a href="https://example.com"><h1>heading</h1></a>` produces `[# heading ](https://example.com)`.
`ActiveRecord::NestedAttributes#raise_nested_attributes_record_not_found!`
interpolates the user-controlled `id` from nested params directly into the
`RecordNotFound` message:
raise RecordNotFound.new("Couldn't find #{model} with ID=#{record_id} for #{self.class.name} with ID=#{id}", ...)
`record_id` comes straight from `posts_attributes[].id` and is not escaped.
Rails maps `RecordNotFound` to a generic `404` and never renders this
message, so this is not exploitable through the framework itself. The risk
is limited to an application that logs the message: an `id` carrying ANSI
escape sequences is interpreted when the log is later viewed in a terminal,
which lets an attacker forge, recolor, or hide log output.
This is the same class of issue as CVE-2025-55193
(GHSA-76r7-hhxj-r776), which added `.inspect`
to the ids in the `RecordNotFound` messages raised by `find` in `core.rb`
and `finder_methods.rb`. That fix missed the nested attributes call site.
This change calls `.inspect` on `record_id` before interpolating it. String
ids are now quoted and their control bytes are escaped to inert text, while
integer ids are unchanged.
For example, a nested `id` of `"\e[31mHello\e[0m"` (containing raw ANSI
escape bytes) previously passed through to the message verbatim, where a
terminal viewing the log would interpret it. It now renders as the quoted,
inert literal `ID="\e[31mHello\e[0m"`.
The second interpolation, `#{id}`, is the parent record's own persisted
primary key rather than reflected request input, so it is left unchanged,
consistent with the parent fix in dd6fcc4.
HackerOne user ylchen originally reported this behavior. The report was closed
as informative because it was not exploitable.
…holder_generation Use bind parameters for array-form arguments in `find_by_sql` / `count_by_sql`
[Fix rails#40642] When a length constraint (:minimum or :is) was given as a proc, resolve_value was only called inside the branch guarded by the value being present. For a nil value with a :minimum (or :is) constraint the raw proc leaked unresolved into errors_options[:count] and was later invoked with the message options hash during I18n interpolation, raising NoMethodError instead of producing the expected "is too short" message. Resolve the check value once, before building the error, so the error message always receives a concrete count. Currently-working paths are unchanged since resolve_value is idempotent for concrete integers and the proc is still invoked exactly once per check.
This caused 8-1-stable builds to get stuck.
…-type-coverage Add test coverage for `ActiveModel::Type::Binary`
ProxyLogger can only filter by severity, so silencing one noisy message
from a gem means raising the level and losing every message below it.
SomeLibrary.logger = ActiveSupport::ProxyLogger.new(Rails.logger)
.ignore(/Noisy/)
Regexps are matched against the message, strings are matched literally.
Patterns are compiled into a single Regexp.union at registration time,
and matching only happens after the severity check, so proxies without
patterns are unaffected.
add now takes Logger#add's explicit signature rather than forwarding
`...`, because the message has to be resolved before it can be matched.
…ger-ignore-patterns Allow ActiveSupport::ProxyLogger to ignore messages by pattern
Fix LengthValidator crash with proc minimum and nil value
`insert`, `update`, `delete` correspond to their SQL DML verbs, but `create` reads like DDL (`create_table`, `create_database`) rather than the `INSERT` it actually performs. The alias is confusing and offers no value over calling `insert` directly.
…insert Deprecate `create` alias of `insert` in connection adapters
Non-main Ractors build their notifier from the recorded subscriptions snapshot, but assigning the snapshot replaced the string subscribers defaulting map with a plain frozen Hash: all_listeners_for concatenates the list for the instrumented name with the other subscribers, so any name with no string subscriber returned nil and crashed the first instrumentation of an unsubscribed event inside a Ractor. Rebuild the defaulting Concurrent::Map and dup the lists when assigning.
rack/ractorize freezes public Rack constants which can't yet be made frozen due to backwards compatibility. However, they will need to be frozen to serve requests inside non-main Ractors.
…napshot-defaults Keep the string subscribers default when rebuilding a Fanout
`RoutesReloader#execute` draws and finalizes the routes, but since be08bdc it leaves the reloader reporting that they are not loaded. The reloader hook installed by the application finisher calls it that way, so the next url helper draws every route a second time and runs the `after_routes_loaded` hooks again.
…-loaded-state Mark the routes as loaded when the routes reloader runs standalone
…mission Omit optional route segments given empty strings
Followup: rails#58412 `false` is `blank?` but we do want to keep it.
UnboundTemplate#freeze required strict locals because a single template can serve every locals set only when the template ignores render-time locals. Without strict locals, one template exists per locals set, which cannot be known at boot: freeze now keeps the unbound shell frozen and shareable, and bind_locals builds unfrozen templates on demand in a Ractor-local two-level cache (store_if_absent, keyed by the unbound template then by normalized locals). Each Ractor compiles a locals set at most once, mirroring the per-process behavior of the unfrozen cache. The Ractor test renders through the public Template#render: with the notifier subscriptions recorded as a shareable snapshot, non-main Ractors rebuild their own notifier and both compile! and render instrument normally. With non-strict templates freezable, ractorize! now also freezes every file system resolver after eager loading and compiling it, making the template caches shareable for stock applications.
etiennebarrie
force-pushed
the
actionview-frozen-non-strict-templates
branch
from
August 11, 2026 10:10
455508e to
823c395
Compare
github-actions
Bot
added
activerecord
activejob
actionpack
actiontext
activemodel
activestorage
docs
labels
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
16 participants
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 #57, allows using non-strict templates from Ractors (mostly, we need AS::Notifications to be shareable).
While this is not advisable, this helps on the path to view rendering ractor safety without requiring strict-locals templates.