From 8ce157b7a3d05d503d80aea46b5dffceb6ee22b2 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Tue, 25 Aug 2026 15:09:59 -0600 Subject: [PATCH 1/2] MOB-98: fix native component events arriving as charlists, not binaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both native bridges (android/jni/mob_nif.zig, ios/mob_nif.m) built mob_send_component_event's event/payload_json via enif_make_string, producing charlists. Mob.ComponentServer decodes payload_json with :json.decode/1, which requires a binary — the component process crashed before handle_event/3 ever ran. Fixed both bridges to emit UTF-8 binaries: Android reuses the existing cstrToBin/3 helper, iOS allocates an ErlNifBinary and copies the bytes. Mob.ComponentServer also now normalizes event/payload_json at the boundary (binary passthrough, charlist -> binary) so a hot-deployed newer BEAM doesn't crash against an older native shell still emitting charlists — the native contract is the real fix, this is compatibility only. Malformed or non-map JSON keeps falling back to %{} instead of crashing the component (previously true only for valid-but-non-map JSON; a genuinely malformed payload raised). Device-verified end-to-end on both platforms: a real tier-2 native component (Compose Button / SwiftUI Button) with a tagged event, traced via :erlang.trace to confirm {:component_event, "tapped", "{}"} arrives as binaries, handle_event/3 fires, and the component/screen stay alive. --- android/jni/mob_nif.zig | 6 +- ios/mob_nif.m | 14 +++- lib/mob/component_server.ex | 32 ++++++-- test/mob/component_server_test.exs | 123 +++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 test/mob/component_server_test.exs diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index 8b1227f..50332ba 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -1458,10 +1458,12 @@ pub export fn mob_send_component_event( const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + // Binaries, not charlists (enif_make_string) — Mob.ComponentServer decodes + // payload_json with :json.decode/1, which requires a binary. const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "component_event"), - erts.enif_make_string(env, event, erts.ERL_NIF_LATIN1), - erts.enif_make_string(env, payload_json, erts.ERL_NIF_LATIN1), + cstrToBin(env, event, std.mem.span(event).len), + cstrToBin(env, payload_json, std.mem.span(payload_json).len), }); var pid = pid_copy; _ = erts.enif_send(null, &pid, env, msg); diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 92d2fe6..1cbb2d8 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -6175,9 +6175,17 @@ void mob_send_component_event(int handle, const char *event, const char *payload enif_mutex_unlock(component_mutex); ErlNifEnv *env = enif_alloc_env(); - ERL_NIF_TERM msg = enif_make_tuple3(env, enif_make_atom(env, "component_event"), - enif_make_string(env, event, ERL_NIF_LATIN1), - enif_make_string(env, payload_json, ERL_NIF_LATIN1)); + // Binaries, not charlists (enif_make_string) — Mob.ComponentServer decodes + // payload_json with :json.decode/1, which requires a binary. + ErlNifBinary event_bin, payload_bin; + enif_alloc_binary(strlen(event), &event_bin); + memcpy(event_bin.data, event, strlen(event)); + enif_alloc_binary(strlen(payload_json), &payload_bin); + memcpy(payload_bin.data, payload_json, strlen(payload_json)); + + ERL_NIF_TERM msg = + enif_make_tuple3(env, enif_make_atom(env, "component_event"), + enif_make_binary(env, &event_bin), enif_make_binary(env, &payload_bin)); enif_send(NULL, &pid, env, msg); enif_free_env(env); } diff --git a/lib/mob/component_server.ex b/lib/mob/component_server.ex index 1082774..729b655 100644 --- a/lib/mob/component_server.ex +++ b/lib/mob/component_server.ex @@ -88,11 +88,8 @@ defmodule Mob.ComponentServer do {:component_event, event, payload_json}, %{module: module, socket: socket, screen_pid: screen_pid, id: id} = state ) do - payload = - case :json.decode(payload_json) do - map when is_map(map) -> map - _ -> %{} - end + event = to_binary(event) + payload = decode_payload(payload_json) {:noreply, new_socket} = module.handle_event(event, payload, socket) send(screen_pid, {:component_changed, id, module}) @@ -108,6 +105,31 @@ defmodule Mob.ComponentServer do {:noreply, %{state | socket: new_socket}} end + # The native contract is binaries (see mob_send_component_event on both + # platforms). This stays TEMPORARILY so a hot-deployed BEAM doesn't crash + # against an older native shell that still emits charlists — MOB-98. + @doc false + @spec to_binary(binary() | charlist()) :: binary() + def to_binary(value) when is_binary(value), do: value + def to_binary(value) when is_list(value), do: List.to_string(value) + + # payload_json arrives as a binary (fixed native contract) or, from an + # older native shell, a charlist — same compat window as to_binary/1 + # above. :json.decode/1 raises on genuinely malformed input rather than + # returning an error tuple; both that and a validly-decoded non-map value + # (e.g. a bare `"5"` or `"null"`) fall back to %{} rather than crashing + # the component process over a bad event payload. + @doc false + @spec decode_payload(binary() | charlist()) :: map() + def decode_payload(json) do + case :json.decode(to_binary(json)) do + map when is_map(map) -> map + _ -> %{} + end + rescue + ErlangError -> %{} + end + @impl GenServer def terminate(reason, %{ module: module, diff --git a/test/mob/component_server_test.exs b/test/mob/component_server_test.exs new file mode 100644 index 0000000..224f564 --- /dev/null +++ b/test/mob/component_server_test.exs @@ -0,0 +1,123 @@ +defmodule Mob.ComponentServerTest do + use ExUnit.Case, async: true + + # Mob.ComponentServer.dispatch/3 (the programmatic Elixir API) bypasses the + # native pipeline entirely — payload arrives as an already-decoded map, not + # JSON. These tests exercise the actual delivery shape the native bridges + # send: handle_info({:component_event, event, payload_json}, ...), where + # event/payload_json may be a binary (the fixed contract, mob 0.7.27+) or a + # charlist (an older native shell, hot-deployed a newer BEAM — MOB-98). + + defmodule Recorder do + use Mob.Component + + def mount(_props, socket) do + {:ok, + socket + |> Mob.Socket.assign(:last_event, nil) + |> Mob.Socket.assign(:last_payload, nil)} + end + + def render(assigns), do: %{last_event: assigns.last_event, last_payload: assigns.last_payload} + + def handle_event(event, payload, socket) do + {:noreply, + socket + |> Mob.Socket.assign(:last_event, event) + |> Mob.Socket.assign(:last_payload, payload)} + end + end + + setup do + start_supervised!({Mob.ComponentRegistry, []}) + + {:ok, pid} = + Mob.ComponentServer.start( + module: Recorder, + id: :r, + screen_pid: self(), + props: %{}, + platform: :no_render + ) + + {:ok, pid: pid} + end + + describe "native :component_event delivery" do + test "accepts a binary event name and binary JSON payload", %{pid: pid} do + send(pid, {:component_event, "tapped", ~s({"index":1})}) + assert_receive {:component_changed, :r, Recorder} + + props = Mob.ComponentServer.render_props(pid) + assert props.last_event == "tapped" + assert props.last_payload == %{"index" => 1} + end + + test "accepts a legacy charlist event name and charlist JSON payload", %{pid: pid} do + send( + pid, + {:component_event, String.to_charlist("tapped"), String.to_charlist(~s({"index":1}))} + ) + + assert_receive {:component_changed, :r, Recorder} + + props = Mob.ComponentServer.render_props(pid) + assert props.last_event == "tapped" + assert props.last_payload == %{"index" => 1} + end + + test "the component always receives a binary event name, never a charlist", %{pid: pid} do + send(pid, {:component_event, String.to_charlist("charlist_event"), "{}"}) + assert_receive {:component_changed, :r, Recorder} + + assert Mob.ComponentServer.render_props(pid).last_event == "charlist_event" + end + + test "malformed JSON falls back to an empty map instead of crashing the component", %{ + pid: pid + } do + send(pid, {:component_event, "bad", "not json"}) + assert_receive {:component_changed, :r, Recorder} + + assert Mob.ComponentServer.render_props(pid).last_payload == %{} + assert Process.alive?(pid) + end + + test "valid but non-map JSON falls back to an empty map", %{pid: pid} do + send(pid, {:component_event, "bad", "5"}) + assert_receive {:component_changed, :r, Recorder} + + assert Mob.ComponentServer.render_props(pid).last_payload == %{} + end + end + + describe "to_binary/1" do + test "a binary passes through unchanged" do + assert Mob.ComponentServer.to_binary("x") == "x" + end + + test "a charlist converts to a binary" do + assert Mob.ComponentServer.to_binary(String.to_charlist("x")) == "x" + end + end + + describe "decode_payload/1" do + test "decodes a binary JSON map" do + assert Mob.ComponentServer.decode_payload(~s({"a":1})) == %{"a" => 1} + end + + test "decodes a charlist JSON map" do + assert Mob.ComponentServer.decode_payload(String.to_charlist(~s({"a":1}))) == %{"a" => 1} + end + + test "falls back to %{} on malformed JSON" do + assert Mob.ComponentServer.decode_payload("not json") == %{} + end + + test "falls back to %{} on valid but non-map JSON" do + assert Mob.ComponentServer.decode_payload("5") == %{} + assert Mob.ComponentServer.decode_payload("null") == %{} + assert Mob.ComponentServer.decode_payload("[1,2]") == %{} + end + end +end From 588c71266fac3003921b0de1ac9a8b3b1028eabc Mon Sep 17 00:00:00 2001 From: GenericJam Date: Tue, 25 Aug 2026 17:35:26 -0600 Subject: [PATCH 2/2] MOB-98 review fixes: crash-safety and allocation-failure handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From code review on PR #83: - Mob.ComponentServer.to_binary/1 raised FunctionClauseError for any input that wasn't a binary or list, with no rescue in the calling handle_info clause — a malformed event/payload shape from native code crashed the component process. Added a catch-all fallback (logs + returns "") and wrapped the list branch's IO.iodata_to_binary call (which itself can raise ArgumentError on malformed content). - Switched List.to_string/1 to IO.iodata_to_binary/1 for the legacy charlist conversion: ERL_NIF_LATIN1 maps codepoint N to byte N, the same as raw-byte iodata — List.to_string/1 would UTF-8-encode any byte > 127 into two bytes, corrupting non-ASCII legacy payloads instead of reproducing them. - ios/mob_nif.m: mob_send_component_event ignored enif_alloc_binary's return value (1=success/0=failure) and computed strlen twice per buffer. Now checks both allocations, releases the first binary if the second fails, and computes each length once. - android/jni/mob_nif.zig: cstrToBin (pre-existing, reused by this PR's new call sites) had the same discarded-return pattern. Falls back to the :nil sentinel this file's other call sites already use for "absent" on allocation failure. - test/mob/component_server_test.exs: setup raced Mob.ComponentRegistry (a fixed-name GenServer) against component_test.exs under async: true — start_supervised! raises on {:already_started, _}. Now tolerates either order. Added tests for the crash-safety fallback and the byte-preservation fix. Device-verified: real native builds (Android emulator + iOS simulator) compile and boot cleanly with these changes. --- android/jni/mob_nif.zig | 10 +++++++-- ios/mob_nif.m | 17 ++++++++++---- lib/mob/component_server.ex | 30 +++++++++++++++++++++++-- test/mob/component_server_test.exs | 36 +++++++++++++++++++++++++++++- 4 files changed, 84 insertions(+), 9 deletions(-) diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index 50332ba..772b535 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -361,10 +361,16 @@ inline fn detachIfAttached(attached: c_int) void { // ── Binary / string helpers ────────────────────────────────────────────── /// Make an `ErlNifBinary` from a C-style {ptr, len} pair and wrap it as a -/// term. BEAM owns the allocated bytes after make_binary returns. +/// term. BEAM owns the allocated bytes after make_binary returns. Falls +/// back to `:nil` on allocation failure — the same sentinel this file's +/// call sites already use for "absent" (see the empty-label/value guards +/// above), so a caller that already tolerates `:nil` doesn't need a new +/// error shape. fn cstrToBin(env: ?*erts.ErlNifEnv, src: [*]const u8, len: usize) erts.ERL_NIF_TERM { var bin: erts.ErlNifBinary = undefined; - _ = erts.enif_alloc_binary(len, &bin); + if (erts.enif_alloc_binary(len, &bin) == 0) { + return erts.atom(env, "nil"); + } @memcpy(bin.data[0..len], src[0..len]); return erts.enif_make_binary(env, &bin); } diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 1cbb2d8..893ca68 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -6177,11 +6177,20 @@ void mob_send_component_event(int handle, const char *event, const char *payload ErlNifEnv *env = enif_alloc_env(); // Binaries, not charlists (enif_make_string) — Mob.ComponentServer decodes // payload_json with :json.decode/1, which requires a binary. + size_t event_len = strlen(event); + size_t payload_len = strlen(payload_json); ErlNifBinary event_bin, payload_bin; - enif_alloc_binary(strlen(event), &event_bin); - memcpy(event_bin.data, event, strlen(event)); - enif_alloc_binary(strlen(payload_json), &payload_bin); - memcpy(payload_bin.data, payload_json, strlen(payload_json)); + if (!enif_alloc_binary(event_len, &event_bin)) { + enif_free_env(env); + return; + } + memcpy(event_bin.data, event, event_len); + if (!enif_alloc_binary(payload_len, &payload_bin)) { + enif_release_binary(&event_bin); + enif_free_env(env); + return; + } + memcpy(payload_bin.data, payload_json, payload_len); ERL_NIF_TERM msg = enif_make_tuple3(env, enif_make_atom(env, "component_event"), diff --git a/lib/mob/component_server.ex b/lib/mob/component_server.ex index 729b655..c9567ff 100644 --- a/lib/mob/component_server.ex +++ b/lib/mob/component_server.ex @@ -4,6 +4,7 @@ defmodule Mob.ComponentServer do # screen gets its own process. Started unlinked (isolated from the screen). use GenServer + require Logger @doc "Start a component process (not linked to the caller)." @spec start(keyword()) :: {:ok, pid()} | {:error, term()} @@ -108,10 +109,35 @@ defmodule Mob.ComponentServer do # The native contract is binaries (see mob_send_component_event on both # platforms). This stays TEMPORARILY so a hot-deployed BEAM doesn't crash # against an older native shell that still emits charlists — MOB-98. + # + # IO.iodata_to_binary/1, not List.to_string/1: the legacy charlist came + # from ObjC's enif_make_string(env, cstr, ERL_NIF_LATIN1), which maps + # codepoint N to byte N — the same as raw-byte iodata, NOT Unicode + # codepoints. List.to_string/1 would UTF-8-encode any byte > 127 into two + # bytes, corrupting non-ASCII legacy payloads instead of reproducing them. + # + # Never raises: this is the component-event boundary from native code, and + # a malformed shape here (however unlikely) must not crash the component + # process over a value it never asked for. @doc false - @spec to_binary(binary() | charlist()) :: binary() + @spec to_binary(term()) :: binary() def to_binary(value) when is_binary(value), do: value - def to_binary(value) when is_list(value), do: List.to_string(value) + + def to_binary(value) when is_list(value) do + IO.iodata_to_binary(value) + rescue + ArgumentError -> log_unexpected_shape(value) + end + + def to_binary(other), do: log_unexpected_shape(other) + + defp log_unexpected_shape(value) do + Logger.warning( + "[mob_component_server] expected a binary or charlist event/payload, got: #{inspect(value)}" + ) + + "" + end # payload_json arrives as a binary (fixed native contract) or, from an # older native shell, a charlist — same compat window as to_binary/1 diff --git a/test/mob/component_server_test.exs b/test/mob/component_server_test.exs index 224f564..18817be 100644 --- a/test/mob/component_server_test.exs +++ b/test/mob/component_server_test.exs @@ -29,7 +29,14 @@ defmodule Mob.ComponentServerTest do end setup do - start_supervised!({Mob.ComponentRegistry, []}) + # Mob.ComponentRegistry registers under a fixed global name. Another + # async test file (component_test.exs) may have already started it — + # start_supervised! would raise on {:already_started, _}, so tolerate + # that instead of racing to be first. + case start_supervised({Mob.ComponentRegistry, []}) do + {:ok, _pid} -> :ok + {:error, {:already_started, _pid}} -> :ok + end {:ok, pid} = Mob.ComponentServer.start( @@ -89,6 +96,14 @@ defmodule Mob.ComponentServerTest do assert Mob.ComponentServer.render_props(pid).last_payload == %{} end + + test "an unexpected event shape doesn't crash the component", %{pid: pid} do + send(pid, {:component_event, :not_a_string, "{}"}) + assert_receive {:component_changed, :r, Recorder} + + assert Mob.ComponentServer.render_props(pid).last_event == "" + assert Process.alive?(pid) + end end describe "to_binary/1" do @@ -99,6 +114,25 @@ defmodule Mob.ComponentServerTest do test "a charlist converts to a binary" do assert Mob.ComponentServer.to_binary(String.to_charlist("x")) == "x" end + + test "an ASCII-only charlist round-trips through List.to_string identically" do + # Sanity check: for the common case (plain ASCII), byte-preserving + # conversion and codepoint-encoding conversion agree. + assert Mob.ComponentServer.to_binary(~c"tapped") == "tapped" + end + + test "a charlist with a byte > 127 is reproduced byte-for-byte, not UTF-8 encoded" do + # ERL_NIF_LATIN1 maps codepoint N to byte N — the raw byte 233, not + # the two-byte UTF-8 encoding of codepoint 233 (é). List.to_string/1 + # would produce <<195, 169>>; the byte-preserving conversion must not. + assert Mob.ComponentServer.to_binary([233]) == <<233>> + end + + test "an unexpected shape (neither binary nor list) falls back to an empty binary" do + assert Mob.ComponentServer.to_binary(:not_a_string) == "" + assert Mob.ComponentServer.to_binary(nil) == "" + assert Mob.ComponentServer.to_binary({1, 2}) == "" + end end describe "decode_payload/1" do