Fix Encoding::CompatibilityError in the cable message browser - #136
Fix Encoding::CompatibilityError in the cable message browser#136Aberen wants to merge 2 commits into
Conversation
Solid Cable 4.0+ stores channel names and payloads in a binary (bytea) column, so the views receive ASCII-8BIT (BINARY) strings. Appending a BINARY value that contains non-ASCII bytes (e.g. the "→" found in Turbo Stream payloads) to the UTF-8 output buffer — which already holds non-ASCII text from the layout — raises Encoding::CompatibilityError and 500s the channel page. - Add to_utf8_text helper: re-tag the bytes as UTF-8 (lossless; the underlying data is UTF-8 source text) before rendering. - Apply it where cable channel names and payloads are rendered. - Align the spec dummy schema with the real solid_cable 4.0.2 schema (binary channel/payload) so the suite actually exercises the BINARY path. - Add regression specs: non-ASCII payload and non-ASCII channel name both render (200, arrow present). Without the fix both fail with Encoding::CompatibilityError.
There was a problem hiding this comment.
🟡 Changes recommended
PostgreSQL bytea search filters still use LIKE and require an adapter-appropriate fix and coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Addresses UTF-8 rendering errors for binary-backed Solid Cable channel names and payloads.
Changes:
- Adds UTF-8 normalization before rendering cable data.
- Aligns the dummy schema with Solid Cable 4.x binary columns.
- Adds regression specs and changelog documentation.
File summaries
| File | Summary | Review status |
|---|---|---|
spec/requests/solid_stack_web/cable_messages_spec.rb |
Adds non-ASCII payload and channel regression coverage. | Add /cable page-level regression coverage. |
spec/dummy/db/schema.rb |
Models Cable fields as binary columns. | Critical (4 votes): Fix PostgreSQL bytea search filters and cover that path. |
CHANGELOG.md |
Documents the encoding fix. | No final comments. |
app/views/solid_stack_web/cable/index.html.erb |
Normalizes channel names before rendering. | Nit (4 votes): Add non-ASCII /cable regression coverage. |
app/views/solid_stack_web/cable_messages/index.html.erb |
Normalizes channel names and payloads. | No final comments. |
app/helpers/solid_stack_web/application_helper.rb |
Adds UTF-8 text normalization. | No final comments. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <td class="sqw-monospace sqw-truncate" title="<%= to_utf8_text(channel[:channel]) %>"> | ||
| <%= link_to to_utf8_text(channel[:channel]), cable_channel_messages_path(channel[:channel_hash]), class: "sqw-link" %> |
There was a problem hiding this comment.
Done — added two /cable regression specs (commit 838d9d3):
- "renders a non-ASCII channel name without error" — broadcasts a
sports → finalchannel and asserts 200 + the name appears. - "filters non-ASCII channel names by substring" — broadcasts the same and searches with
q="→", asserting the match appears and a non-matching channel does not.
These exercise both the channel cell rendering (to_utf8_text(channel[:channel])) and the channel_rows LIKE search path on the binary column. Full suite: 419 examples, 0 failures.
| t.binary "channel", null: false | ||
| t.binary "payload", null: false |
There was a problem hiding this comment.
I tested this against a real PostgreSQL 18.6 database with the actual bytea schema and the conclusion is the opposite:
-
PostgreSQL ships a native
~~(LIKE) operator forbytea. Inpg_operator:~~(bytea,bytea) bytea byteaIt's core, not an extension. The text→bytea implicit cast resolves
channel LIKE 'text'. -
The exact query shape the controllers emit works on bytea:
SELECT count(*) FROM solid_cable_messages WHERE channel LIKE '%display%' -- 13 SELECT count(*) FROM solid_cable_messages WHERE channel LIKE '%zzz%' -- 0 (negative match correct) SELECT count(*) FROM (VALUES (E'room → live'::bytea)) x(c) WHERE c LIKE '%→%' -- 1 (non-ASCII) SELECT count(*) FROM solid_cable_messages WHERE channel NOT LIKE '%zzz%' -- 13So the browser search does not 500 on PG.
-
The proposed fix actually regresses search.
CAST(channel AS TEXT)on a bytea column produces the hex escape form, not the original UTF-8 text:SELECT CAST(channel AS TEXT) FROM solid_cable_messages LIMIT 1; -- \x333a646973706c61795f6c697665 SELECT count(*) FROM solid_cable_messages WHERE CAST(channel AS TEXT) LIKE '%display%'; -- 0Applying
CAST(... AS TEXT)would make every search return zero rows. If a cast is ever genuinely needed (e.g. to search by a decoded UTF-8 substring across adapters), the correct PG function isconvert_from(channel, 'UTF8'), which does match:SELECT count(*) FROM solid_cable_messages WHERE convert_from(channel,'UTF8') LIKE '%display%'; -- 13 -
The suite runs on SQLite (see
spec/dummy/config/database.yml), where BLOB LIKE also works (verified:channel LIKE '%→%'→ 1). So the SQLite suite does surface this path; it just doesn't distinguish~~(bytea,bytea)from~~(text,text).
I'd rather keep the controllers as-is (no cast) since it works on both adapters. If you'd prefer, I can add a CI job that runs the cable specs against a real Postgres service so the bytea path is exercised on PG too — let me know.
Closes the coverage gap noted in review: the /cable channel-list view now has specs that render a non-ASCII channel name and filter by one, exercising the binary channel column's rendering and LIKE search paths.
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review comments were supplied.
Review details
Suppressed comments (1)
app/helpers/solid_stack_web/application_helper.rb:32
- Because these columns are explicitly binary,
force_encoding("UTF-8")is only safe for valid UTF-8 bytes; binary storage does not enforce that. If a malformed or legacy payload/channel (for example, bytes such as\xFF) is present, this helper returns an invalid UTF-8 string and Rails escaping/truncation can still raise while rendering. Scrub after re-tagging (or otherwise handle invalid sequences) so this boundary remains safe for all rows while preserving valid UTF-8 bytes.
value.to_s.dup.force_encoding("UTF-8")
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
The Solid Cable channel browser 500s whenever a payload or channel name
contains non-ASCII characters (e.g. the
→that appears in Turbo Streampayloads). The page raises:
Root cause
Solid Cable 4.0+ stores
channelandpayloadin a binary (bytea)column, so ActiveRecord hands the view a string with
ASCII-8BIT(BINARY) encoding. The view interpolates it straight into the UTF-8
output buffer. Appending a BINARY string that carries non-ASCII bytes to
a UTF-8 buffer that already holds non-ASCII text (the layout header does)
raises
Encoding::CompatibilityError. ASCII-only payloads never triggerit, which is why the existing suite didn't catch it.
Changes
app/helpers/solid_stack_web/application_helper.rb— addto_utf8_text, which re-tags the bytes as UTF-8 (lossless; theunderlying data is UTF-8 source text) via
dup.force_encoding("UTF-8").app/views/solid_stack_web/cable_messages/index.html.erbandapp/views/solid_stack_web/cable/index.html.erb— render channel namesand payloads through
to_utf8_text.spec/dummy/db/schema.rb— align the dummy schema with the realsolid_cable 4.0.2 schema (
binaryforchannel/payload) so thesuite actually exercises the BINARY path.
spec/requests/solid_stack_web/cable_messages_spec.rb— add regressionspecs for non-ASCII payloads and non-ASCII channel names.
Verification
rubocop: 116 files, no offenses.the binary schema + new specs kept, both new examples fail with the
exact
Encoding::CompatibilityErrorabove.Notes
force_encodingis used deliberately rather thanencode: the byteabytes are already valid UTF-8 source text (they are HTML/Turbo Stream
documents, not a different encoding), so re-tagging is correct and
lossless.
dupkeeps the ActiveRecord attribute string unmutated.