ReviewOS

also looking at this

craft-native/craft

fix(ios): answer the shapes the spec answers, now that a bare false survives

#105
Merged chris wants to merge fix/ios-reply-shape-parity into main
7 files +423 -82
packages/ios/templates/CraftApp.swiftmodified+13-4
renamed throughout · 13 hidden lines
Changes to packages/ios/templates/CraftApp.swift
@@ -1093,11 +1093,11 @@ struct CraftWebView: UIViewRepresentable { renamed throughout
10931093 // MARK: - Siri Shortcuts
10941094 case "registerSiriShortcut":
10951095 if let phrase = body["phrase"] as? String,
1096 let action = body["action"] as? String {
1096 let action = body["shortcutAction"] as? String {
10971097 registerSiriShortcut(phrase: phrase, action: action, callbackId: callbackId)
10981098 }
10991099 case "removeSiriShortcut":
1100 if let action = body["action"] as? String {
1100 if let action = body["shortcutAction"] as? String {
11011101 removeSiriShortcut(action: action, callbackId: callbackId)
11021102 }
11031103
Changes to packages/ios/templates/CraftApp.swift
@@ -2194,7 +2194,15 @@ struct CraftWebView: UIViewRepresentable {
21942194 register: function(phrase, action) {
21952195 var self = window.craft;
21962196 var id = 'cb_' + (++self._callbackId);
2197 window.webkit.messageHandlers.craft.postMessage({action: 'registerSiriShortcut', phrase: phrase, action: action, callbackId: id});
2197 // `shortcutAction`, not `action`. This object had two
2198 // keys called `action` — the message's own, and the
2199 // shortcut's — and the later one wins, so every call
2200 // arrived labelled with the shortcut's identifier
2201 // instead of 'registerSiriShortcut'. No switch arm
2202 // matched, nothing answered, and these wrappers park in
2203 // `_callbacks` with no timeout: the promise never
2204 // settled at all.
2205 window.webkit.messageHandlers.craft.postMessage({action: 'registerSiriShortcut', phrase: phrase, shortcutAction: action, callbackId: id});
21982206 return new Promise(function(resolve, reject) {
21992207 self._callbacks[id] = {resolve: resolve, reject: reject};
22002208 });
@@ -2202,7 +2210,8 @@ struct CraftWebView: UIViewRepresentable {
22022210 remove: function(action) {
22032211 var self = window.craft;
22042212 var id = 'cb_' + (++self._callbackId);
2205 window.webkit.messageHandlers.craft.postMessage({action: 'removeSiriShortcut', action: action, callbackId: id});
2213 // The same duplicate key as `register` above.
2214 window.webkit.messageHandlers.craft.postMessage({action: 'removeSiriShortcut', shortcutAction: action, callbackId: id});
22062215 return new Promise(function(resolve, reject) {
22072216 self._callbacks[id] = {resolve: resolve, reject: reject};
22082217 });
packages/zig/src/bridge_mobile_display.zigmodified+315-47
Changes to packages/zig/src/bridge_mobile_display.zig
@@ -3,8 +3,12 @@ const builtin = ;
33const capabilities = ;
44const bridge_error = ;
55const objc_runtime = ;
6const ios_async = ;
7const compat_mutex = ;
68
79const objc = objc_runtime.objc;
10const Id = objc.id;
11const is_darwin = builtin.target.os.tag.isDarwin();
812
913/// The `mobile` display group: the app icon's badge, the idle timer, and the
1014/// two orientation actions.
@@ -44,14 +48,16 @@ const objc = objc_runtime.objc;
4448/// which WebKit delivers on the main thread. The hop is redundant, and there is
4549/// no dispatch-block machinery in this repo to reproduce it with.
4650///
47/// **No badge authorization.** Swift calls
48/// `requestAuthorizationWithOptions:` and rejects with "Badge permission
49/// denied". Reporting that from Zig needs a *capturing* block — every block in
50/// this repo is static and non-capturing — so it is not attempted here. The
51/// consequence is stated rather than hidden: Zig sets the number and proves
52/// UIKit stored it, which is **not** proof that the springboard draws it. An
53/// unauthorized app gets `{"count":N}` and an invisible badge, and will never
54/// see "Badge permission denied" or the first-run prompt Swift triggers.
51/// **Badge authorization, as the spec does it.** `setBadge` and `clearBadge`
52/// call `requestAuthorizationWithOptions:` first and reject with
53/// `PERMISSION_DENIED` when iOS says no — the spec's "Badge permission denied".
54/// This used to be skipped with the note that it needed a *capturing* block,
55/// which no block in the repo was. That stopped being true when
56/// `bridge_mobile_auth.zig` shipped per-slot global blocks whose slot index is
57/// baked in at comptime; the same shape is used here. Without it Zig set the
58/// number, proved UIKit stored it, and answered `true` for a badge an
59/// unauthorised app never draws — a fabricated success, and the one this file
60/// was still producing after every other module had stopped.
5561pub const A = struct {
5662 pub const set_badge = "setBadge";
5763 pub const clear_badge = "clearBadge";
@@ -105,15 +111,21 @@ pub const DisplayBridge = struct {
105111 }
106112 }
107113
108 /// `{"count":N}` in, `{"count":<what UIKit now reports>}` out.
114 /// `{"count":N}` in, bare `true` out — what the spec resolves.
109115 ///
110 /// The reply is an object, not the bare `true` Swift resolves:
111 /// `craft-bridge.js` settles with `payload || {}`, so any falsy scalar
112 /// arrives at the page as `{}` and carries nothing. An object also lets the
113 /// value be checked, which a `true` never could.
116 /// This answered `{"count":<observed>}` until now, to work around
117 /// `craft-bridge.js` settling with `payload || {}`: a falsy scalar reached
118 /// the page as `{}` and carried nothing. That is fixed at its source
119 /// (`_orEmpty`), so the wrapper is gone and pages reading `=== true` work
120 /// again.
121 ///
122 /// The observed count goes to the log rather than the page. It was the one
123 /// thing the object shape bought, and the spec never offered it — a page
124 /// that wants the badge back has no API for it in either implementation,
125 /// so inventing one here would be a second divergence rather than a fix.
114126 fn setBadge(self: *Self, data: []const u8) !void {
115127 const requested = try badgeCountFrom(self.allocator, data);
116 try self.replyBadge(A.set_badge, try applyBadge(requested));
128 try requestBadgeAuthorization(self.allocator, A.set_badge, requested);
117129 }
118130
119131 /// The same path as `setBadge` with a fixed 0, so the two cannot drift.
@@ -121,15 +133,14 @@ pub const DisplayBridge = struct {
121133 /// `clearBadge` sends no payload at all, so there is nothing to parse:
122134 /// `payloadOf` hands this `{}` and the count is not the caller's to choose.
123135 fn clearBadge(self: *Self) !void {
124 try self.replyBadge(A.clear_badge, try applyBadge(0));
125 }
126
127 fn replyBadge(self: *Self, action: []const u8, observed: i64) !void {
128 var buf: [48]u8 = undefined;
129 bridge_error.sendResultToJS(self.allocator, action, try renderBadgeReply(&buf, observed));
136 try requestBadgeAuthorization(self.allocator, A.clear_badge, 0);
130137 }
131138
132 /// `{"enabled":<bool>}` in, `{"enabled":<what UIKit now reports>}` out.
139 /// `{"enabled":<bool>}` in, the bare observed boolean out.
140 ///
141 /// The spec resolves the `enabled` the page sent; this resolves what UIKit
142 /// reports, which is the same value whenever the write took. Bare rather
143 /// than `{"enabled":…}` for the reason recorded on `replyBadge`.
133144 ///
134145 /// Swift also stores `isKeepingAwake`, which nothing in the template ever
135146 /// reads. It is not ported: a variable with no reader cannot be wrong, and
@@ -140,11 +151,10 @@ pub const DisplayBridge = struct {
140151 const requested = try keepAwakeFrom(self.allocator, data);
141152 const observed = try applyKeepAwake(requested);
142153
143 var buf: [32]u8 = undefined;
144154 bridge_error.sendResultToJS(
145155 self.allocator,
146156 A.set_keep_awake,
147 try renderKeepAwakeReply(&buf, observed),
157 if (observed) "true" else "false",
148158 );
149159 }
150160
@@ -185,12 +195,213 @@ pub const DisplayBridge = struct {
185195/// Split out because a test that re-typed the format string would pass no
186196/// matter what the handler sent — it would be asserting that `std.fmt` works.
187197/// These are the strings the page actually receives.
188fn renderBadgeReply(buf: []u8, observed: i64) ![]const u8 {
189 return std.fmt.bufPrint(buf, "", .{observed});
198/// What `setBadge` and `clearBadge` resolve. A bare `true`, matching the spec.
199const badge_applied = "true";
200
201// =============================================================================
202// Badge authorization: ask, then set, then answer — on the threads the spec
203// uses for each.
204// =============================================================================
205
206/// `UNAuthorizationOptionBadge`. `UNUserNotificationCenter.h:23` in the
207/// iPhoneSimulator 26.5 SDK: `UNAuthorizationOptionBadge = (1 << 0)`. Pinned
208/// by a test that cites the same line, after a neighbouring module once
209/// guessed a `1 << 2` that named a different option entirely.
210const un_authorization_option_badge: c_ulong = 1 << 0;
211
212/// Ask iOS for badge authorization, and set the badge only once it says yes.
213///
214/// The spec's order exactly: `requestAuthorization(options: .badge)`, then
215/// `applicationIconBadgeNumber = count` inside `DispatchQueue.main.async` on
216/// grant, or reject on refusal. The count is validated *before* anything is
217/// asked, so a malformed call fails as it always did without ever triggering
218/// the first-run prompt.
219///
220/// When UserNotifications is not in the process — the `zig-slice` fixture
221/// links only UIKit, WebKit and Foundation — there is nothing to ask, and the
222/// badge is set directly as before, with a warning saying so. That path cannot
223/// be reached by a generated app: `project.yml` links UserNotifications
224/// unconditionally.
225fn requestBadgeAuthorization(allocator: std.mem.Allocator, action: []const u8, count: i64) !void {
226 if (!is_darwin) return error.UnsupportedPlatform;
227 _ = std.math.cast(c_long, count) orelse return bridge_error.BridgeError.InvalidParameter;
228
229 const UNCenterClass = objc.objc_getClass("UNUserNotificationCenter") orelse {
230 std.log.warn(
231 "{s}: UserNotifications is not in this process, so authorization cannot be " ++
232 "requested; setting the badge directly, which an unauthorised app will not draw",
233 .{action},
234 );
235 const observed = try applyBadge(count);
236 std.log.info("{s}: badge is now {d}", .{ action, observed });
237 bridge_error.sendResultToJS(allocator, action, badge_applied);
238 return;
239 };
240
241 // `currentNotificationCenter` RAISES — not nil — in a process with no
242 // bundle identifier, which is every test runner. Same guard as
243 // `bridge_mobile_permissions.zig` and `bridge_mobile_notifcancel.zig`.
244 try requireBundleIdentifier();
245
246 const sel_current = objc.sel_registerName("currentNotificationCenter") orelse
247 return error.SelectorNotFound;
248 const center = objc.msgSendId(UNCenterClass, sel_current) orelse return error.NoNotificationCenter;
249 const sel = objc.sel_registerName("requestAuthorizationWithOptions:completionHandler:") orelse
250 return error.SelectorNotFound;
251
252 const ticket = ios_async.acquire(action) orelse return poolFull(action);
253 errdefer ios_async.abandon(ticket);
254 publishPendingCall(ticket, action, count);
255
256 const Fn = *const fn (Id, objc.SEL, c_ulong, *anyopaque) callconv(.c) void;
257 const func: Fn = @ptrCast(&objc.objc_msgSend);
258 func(center, sel, un_authorization_option_badge, replyBlock(ticket));
190259}
191260
192fn renderKeepAwakeReply(buf: []u8, observed: bool) ![]const u8 {
193 return std.fmt.bufPrint(buf, "", .{observed});
261fn requireBundleIdentifier() !void {
262 const NSBundle = objc.objc_getClass("NSBundle") orelse return error.ClassNotFound;
263 const sel_main = objc.sel_registerName("mainBundle") orelse return error.SelectorNotFound;
264 const bundle = objc.msgSendId(NSBundle, sel_main) orelse return error.NoBundleIdentifier;
265 const sel_ident = objc.sel_registerName("bundleIdentifier") orelse return error.SelectorNotFound;
266 if (objc.msgSendId(bundle, sel_ident) == null) return error.NoBundleIdentifier;
267}
268
269fn poolFull(action: []const u8) bridge_error.BridgeError {
270 std.log.warn("{s} refused: all {d} async slots in flight", .{ action, ios_async.max_in_flight });
271 return bridge_error.BridgeError.InvalidParameter;
272}
273
274/// What a slot's block will answer. The ticket is stored, not rebuilt from the
275/// index: a ticket carries a generation, and inventing one at reply time would
276/// defeat the check that stops a late completion answering the slot's next
277/// occupant. `action` is one of the two static names in `A`, never allocated.
278const PendingBadge = struct {
279 ticket: ios_async.Ticket,
280 action: []const u8,
281 count: i64,
282};
283
284var pending_badges: [ios_async.max_in_flight]?PendingBadge = @splat(null);
285var pending_mutex: compat_mutex.Mutex = .{};
286
287fn publishPendingCall(ticket: ios_async.Ticket, action: []const u8, count: i64) void {
288 pending_mutex.lock();
289 defer pending_mutex.unlock();
290 pending_badges[ticket.index] = .{ .ticket = ticket, .action = action, .count = count };
291}
292
293/// Read and clear. Clearing is what makes a second fire a no-op instead of a
294/// second reply.
295fn takePendingCall(index: u5) ?PendingBadge {
296 pending_mutex.lock();
297 defer pending_mutex.unlock();
298 const call = pending_badges[index];
299 pending_badges[index] = null;
300 return call;
301}
302
303const BlockDescriptor = extern struct {
304 reserved: c_ulong = 0,
305 size: c_ulong,
306};
307
308/// `void (^)(BOOL, NSError *)` — `requestAuthorizationWithOptions:`'s
309/// completion. Not `ios_async.boolErrorBlock`: that one answers the page
310/// `"granted"`/`"denied"`, and this one has a badge to set in between.
311const ReplyBlock = extern struct {
312 isa: ?*anyopaque,
313 flags: c_int,
314 reserved: c_int = 0,
315 invoke: *const anyopaque,
316 descriptor: *const BlockDescriptor,
317};
318
319/// 1 << 28. A global block is never copied, so it can be handed to an API that
320/// escapes it with no heap copy and no descriptor lifetime.
321const BLOCK_IS_GLOBAL: c_int = 1 << 28;
322const reply_block_descriptor = BlockDescriptor{ .size = @sizeOf(ReplyBlock) };
323
324extern var _NSConcreteGlobalBlock: anyopaque;
325
326fn makeReplyInvoke(comptime index: u5) *const anyopaque {
327 const S = struct {
328 fn invoke(_: *const ReplyBlock, granted: bool, err: Id) callconv(.c) void {
329 authorizationFired(index, granted, err);
330 }
331 };
332 return @ptrCast(&S.invoke);
333}
334
335fn makeReplyBlocks() [ios_async.max_in_flight]ReplyBlock {
336 var out: [ios_async.max_in_flight]ReplyBlock = undefined;
337 for (&out, 0..) |*b, i| {
338 b.* = .{
339 .isa = &_NSConcreteGlobalBlock,
340 .flags = BLOCK_IS_GLOBAL,
341 .invoke = makeReplyInvoke(@intCast(i)),
342 .descriptor = &reply_block_descriptor,
343 };
344 }
345 return out;
346}
347
348var reply_blocks: [ios_async.max_in_flight]ReplyBlock =
349 if (is_darwin) makeReplyBlocks() else undefined;
350
351fn replyBlock(ticket: ios_async.Ticket) *anyopaque {
352 return @ptrCast(&reply_blocks[ticket.index]);
353}
354
355extern "c" fn dispatch_async_f(
356 queue: *anyopaque,
357 context: ?*anyopaque,
358 work: *const fn (?*anyopaque) callconv(.c) void,
359) void;
360extern var _dispatch_main_q: anyopaque;
361
362/// Runs on whatever queue UserNotifications chose.
363///
364/// A refusal is answered from here — `ios_async` hops to the main queue
365/// itself. A grant is not: the badge has to be *set* on the main thread
366/// before the answer is true, which is what Swift's `DispatchQueue.main.async`
367/// is for in this position. So the grant path hops first and answers after.
368/// The pending entry survives the hop and is taken on the far side, which is
369/// what makes a double fire harmless: the second hop finds nothing.
370fn authorizationFired(index: u5, granted: bool, err: Id) void {
371 if (!is_darwin) return;
372
373 if (!granted) {
374 const call = takePendingCall(index) orelse {
375 std.log.warn("badge authorization fired for slot {d} with no call recorded; ignored", .{index});
376 return;
377 };
378 _ = err;
379 std.log.info("{s}: badge authorization denied", .{call.action});
380 ios_async.deliverErrorCode(call.ticket, bridge_error.BridgeError.PermissionDenied);
381 return;
382 }
383
384 // `index + 1`, never a null context: `ios_async` passes its slot the same
385 // way and for the same reason.
386 dispatch_async_f(&_dispatch_main_q, @ptrFromInt(@as(usize, index) + 1), applyBadgeOnMain);
387}
388
389/// Main thread. Authorized, so set the badge and say so.
390fn applyBadgeOnMain(context: ?*anyopaque) callconv(.c) void {
391 if (!is_darwin) return;
392 const index: u5 = @intCast(@intFromPtr(context) - 1);
393 const call = takePendingCall(index) orelse {
394 std.log.warn("badge grant reached the main queue for slot {d} with no call recorded; ignored", .{index});
395 return;
396 };
397
398 const observed = applyBadge(call.count) catch |e| {
399 std.log.warn("{s}: authorised, but the badge could not be set: {s}", .{ call.action, @errorName(e) });
400 ios_async.deliverErrorCode(call.ticket, bridge_error.BridgeError.NativeCallFailed);
401 return;
402 };
403 std.log.info("{s}: badge is now {d}", .{ call.action, observed });
404 ios_async.deliverJson(call.ticket, badge_applied);
194405}
195406
196407// =============================================================================
@@ -630,27 +841,25 @@ test "the orientation actions refuse, and say which kind of refusal it is" {
630841 );
631842}
632843
633test "replies are objects carrying a value, not bare scalars" {
634 // `craft-bridge.js` settles with `payload || {}`. A bare `false` — which
635 // `setKeepAwake(false)` would resolve, following Swift — arrives at the
636 // page as `{}` and loses the answer entirely.
844test "replies are the bare scalars the spec resolves" {
845 // These were objects — `{"count":N}` and `{"enabled":<bool>}` — to work
846 // around `craft-bridge.js` settling with `payload || {}`, which turned a
847 // bare `false` into a truthy `{}`. That is fixed at its source, so the
848 // wrapper is gone and the shapes match the spec again: a page doing
849 // `(await craft.setKeepAwake(false)) === false` gets `false`, and one doing
850 // `(await craft.clearBadge()) === true` gets `true`.
637851 //
638 // These call the same renderers the handlers do, so changing a key name or
639 // dropping the object wrapper fails here instead of reading as coverage.
640 var buf: [48]u8 = undefined;
641
642 try testing.expectEqualStrings("{\"count\":7}", try renderBadgeReply(&buf, 7));
643 try testing.expectEqualStrings("{\"count\":0}", try renderBadgeReply(&buf, 0));
644 try testing.expectEqualStrings("{\"enabled\":false}", try renderKeepAwakeReply(&buf, false));
645 try testing.expectEqualStrings("{\"enabled\":true}", try renderKeepAwakeReply(&buf, true));
646
647 // The stack buffers the handlers hand these are 48 and 32 bytes; the widest
648 // reply either can produce has to fit, or a legitimate count turns into a
649 // `NoSpaceLeft` the page reads as a native failure.
650 var badge_buf: [48]u8 = undefined;
651 try testing.expect((try renderBadgeReply(&badge_buf, std.math.minInt(i64))).len <= badge_buf.len);
652 var keep_buf: [32]u8 = undefined;
653 try testing.expect((try renderKeepAwakeReply(&keep_buf, false)).len <= keep_buf.len);
852 // Pinned as the literal bytes the handlers send, so re-wrapping either
853 // reply fails here rather than reading as coverage.
854 try testing.expectEqualStrings("true", badge_applied);
855
856 // Parsed, not just compared, so a literal that is not valid JSON — `True`,
857 // or a stray quote — cannot pass.
858 for ([_][]const u8{ badge_applied, "true", "false" }) |json| {
859 const parsed = try std.json.parseFromSlice(std.json.Value, testing.allocator, json, .{});
860 defer parsed.deinit();
861 try testing.expect(parsed.value == .bool);
862 }
654863}
655864
656865test "UIKit work is refused off Darwin rather than attempted" {
@@ -658,3 +867,62 @@ test "UIKit work is refused off Darwin rather than attempted" {
658867 try testing.expectError(error.UnsupportedPlatform, applyBadge(1));
659868 try testing.expectError(error.UnsupportedPlatform, applyKeepAwake(true));
660869}
870
871test "the badge authorization option is the header's bit, not a guess" {
872 // `UNUserNotificationCenter.h:23`: `UNAuthorizationOptionBadge = (1 << 0)`.
873 // A neighbouring module once pinned a guessed `1 << 2` for a HealthKit
874 // option and shipped a crash; this cites the line instead.
875 try testing.expectEqual(@as(c_ulong, 1), un_authorization_option_badge);
876 try testing.expect(un_authorization_option_badge != 1 << 1); // Sound
877 try testing.expect(un_authorization_option_badge != 1 << 2); // Alert
878}
879
880test "each badge slot's reply block is global and has its own invoke" {
881 if (!is_darwin) return error.SkipZigTest;
882 for (&reply_blocks) |*b| {
883 try testing.expectEqual(&_NSConcreteGlobalBlock, b.isa);
884 try testing.expectEqual(BLOCK_IS_GLOBAL, b.flags);
885 try testing.expectEqual(@sizeOf(ReplyBlock), @as(usize, @intCast(b.descriptor.size)));
886 }
887 try testing.expect(reply_blocks[0].invoke != reply_blocks[1].invoke);
888}
889
890test "a pending badge call is taken exactly once" {
891 // The property a double-fired completion relies on: the second `take`
892 // finds nothing, so it replies to nobody instead of answering twice.
893 const ticket = ios_async.Ticket{ .index = 3, .generation = 42 };
894 publishPendingCall(ticket, A.set_badge, 7);
895 const first = takePendingCall(3) orelse return error.TestUnexpectedResult;
896 try testing.expectEqual(@as(i64, 7), first.count);
897 try testing.expectEqualStrings(A.set_badge, first.action);
898 try testing.expectEqual(@as(u32, 42), first.ticket.generation);
899 try testing.expect(takePendingCall(3) == null);
900}
901
902test "a bad count is refused before any authorization is requested" {
903 // Ordering, not just parsing: the parse failure has to come first, on
904 // every platform, so a malformed call never triggers the first-run prompt.
905 var bridge = DisplayBridge.init(testing.allocator);
906 defer bridge.deinit();
907 try testing.expectError(
908 bridge_error.BridgeError.InvalidParameter,
909 bridge.handleMessage(A.set_badge, "{\"count\":\"three\"}"),
910 );
911}
912
913test "setBadge is refused on the host rather than resolved without asking" {
914 // There is no UIKit and no bundle identifier in a test binary, so the
915 // only correct outcome is a refusal. Which refusal depends on what the
916 // host has loaded — UserNotifications absent takes one path, present
917 // takes another — so this pins the shape (an error, never a reply) and
918 // not the name. A resolve here would be the exact fabricated success this
919 // change exists to remove.
920 var bridge = DisplayBridge.init(testing.allocator);
921 defer bridge.deinit();
922 if (bridge.handleMessage(A.set_badge, "{\"count\":3}")) |_| {
923 return error.TestUnexpectedResult;
924 } else |_| {}
925 if (bridge.handleMessage(A.clear_badge, "{}")) |_| {
926 return error.TestUnexpectedResult;
927 } else |_| {}
928}
packages/zig/src/bridge_mobile_health.zigmodified+25-5
Changes to packages/zig/src/bridge_mobile_health.zig
@@ -1085,11 +1085,20 @@ fn routeInserted(index: u5, success: bool, err: Id) void {
10851085 const chain = call.workout orelse return;
10861086
10871087 if (!success) {
1088 // Rejects, as the spec does. Resolving here reported a workout with
1089 // `"routeId":""`, which is the shape a *successful* finish with a nil
1090 // route produces — so a page could not tell "the route saved and had no
1091 // id" from "the route did not save at all", and would file the workout
1092 // as complete with its locations silently missing.
1093 //
1094 // The id is not lost, it is logged: a fabricated success costs more
1095 // than a rejection, and this module refuses one everywhere else.
10881096 logNSError(A.save_health_workout, err);
1089 // Swift rejects. The workout is saved regardless, so the id is
1090 // reported rather than lost — losing it would leave a page unable to
1091 // reference a workout that exists.
1092 replyWithWorkout(call.ticket, chain, null);
1097 std.log.info(
1098 "saveHealthWorkout: route insert failed; the workout itself was saved as {s}",
1099 .{uuidStringOf(chain.workout) orelse "<no uuid>"},
1100 );
1101 ios_async.deliverErrorCode(call.ticket, bridge_error.BridgeError.NativeCallFailed);
10931102 chain.deinit();
10941103 return;
10951104 }
@@ -1112,8 +1121,15 @@ fn routeFinished(index: u5, route: Id, err: Id) void {
11121121 defer chain.deinit();
11131122
11141123 if (err != null) {
1124 // Same reasoning as `routeInserted`: the spec rejects on a finish
1125 // error, and resolving would be indistinguishable from the nil-route
1126 // success below.
11151127 logNSError(A.save_health_workout, err);
1116 replyWithWorkout(call.ticket, chain, null);
1128 std.log.info(
1129 "saveHealthWorkout: route finish failed; the workout itself was saved as {s}",
1130 .{uuidStringOf(chain.workout) orelse "<no uuid>"},
1131 );
1132 ios_async.deliverErrorCode(call.ticket, bridge_error.BridgeError.NativeCallFailed);
11171133 return;
11181134 }
11191135 replyWithWorkout(call.ticket, chain, route);
@@ -1124,6 +1140,10 @@ fn routeFinished(index: u5, route: Id, err: Id) void {
11241140/// Swift emits the second shape only when a route was finished, and its
11251141/// `routeId` is `route?.uuid.uuidString ?? ""` — so a finished-but-nil route
11261142/// carries an empty string rather than a missing key.
1143///
1144/// Only reached on success now. The two failure stages reject instead, because
1145/// an empty `routeId` here already means "finished, no id" and cannot also mean
1146/// "did not finish".
11271147fn replyWithWorkout(ticket: ios_async.Ticket, chain: WorkoutChain, route: Id) void {
11281148 const allocator = std.heap.c_allocator;
11291149
packages/zig/src/bridge_mobile_siri.zigmodified+9-6
Changes to packages/zig/src/bridge_mobile_siri.zig
@@ -295,7 +295,10 @@ fn readFields(
295295 else => return BridgeError.InvalidJSON,
296296 };
297297
298 const action = try requiredString(allocator, object, "action");
298 // `shortcutAction`, not `action`: the envelope's own `action` names the
299 // bridge action, and the page used to send both under that one key. See
300 // the wrapper in `CraftApp.swift`.
301 const action = try requiredString(allocator, object, "shortcutAction");
299302 errdefer allocator.free(action);
300303
301304 const phrase = if (want.phrase) try requiredString(allocator, object, "phrase") else null;
@@ -459,7 +462,7 @@ test "both fields are required for a registration, and neither hangs" {
459462 // missing phrase replies nothing at all.
460463 var fields = try readFields(
461464 testing.allocator,
462 "{\"action\":\"openVault\",\"phrase\":\"open my vault\"}",
465 "{\"shortcutAction\":\"openVault\",\"phrase\":\"open my vault\"}",
463466 .{ .phrase = true },
464467 );
465468 defer fields.deinit(testing.allocator);
@@ -468,7 +471,7 @@ test "both fields are required for a registration, and neither hangs" {
468471
469472 try testing.expectError(BridgeError.MissingData, readFields(
470473 testing.allocator,
471 "{\"action\":\"openVault\"}",
474 "{\"shortcutAction\":\"openVault\"}",
472475 .{ .phrase = true },
473476 ));
474477 try testing.expectError(BridgeError.MissingData, readFields(
renamed throughout · 21 hidden lines
Changes to packages/zig/src/bridge_mobile_siri.zig
@@ -478,18 +481,18 @@ test "both fields are required for a registration, and neither hangs" { renamed throughout
478481 ));
479482 try testing.expectError(BridgeError.InvalidParameter, readFields(
480483 testing.allocator,
481 "{\"action\":\"\",\"phrase\":\"p\"}",
484 "{\"shortcutAction\":\"\",\"phrase\":\"p\"}",
482485 .{ .phrase = true },
483486 ));
484487 try testing.expectError(BridgeError.InvalidParameter, readFields(
485488 testing.allocator,
486 "{\"action\":3,\"phrase\":\"p\"}",
489 "{\"shortcutAction\":3,\"phrase\":\"p\"}",
487490 .{ .phrase = true },
488491 ));
489492}
490493
491494test "a removal needs only the action" {
492 var fields = try readFields(testing.allocator, "{\"action\":\"openVault\"}", .{ .phrase = false });
495 var fields = try readFields(testing.allocator, "{\"shortcutAction\":\"openVault\"}", .{ .phrase = false });
493496 defer fields.deinit(testing.allocator);
494497 try testing.expectEqualStrings("openVault", fields.action);
495498 try testing.expect(fields.phrase == null);
packages/zig/src/bridge_mobile_system.zigmodified+30-16
Changes to packages/zig/src/bridge_mobile_system.zig
@@ -63,16 +63,23 @@ pub const capability_actions = [_]capabilities.ActionDecl{
6363///
6464/// Named rather than written inline at each `sendResultToJS`, so the test that
6565/// pins their shape reads the same bytes the handlers send instead of a copy
66/// of them that can drift. Every one is a JSON *object* carrying a boolean:
67/// `craft-bridge.js` resolves with `payload || {}`, so a bare `false` and an
68/// empty reply are the same value to the page — a fabricated success wearing
69/// the shape of an honest one.
66/// of them that can drift.
67///
68/// Bare booleans, because that is what the spec resolves — `openURL` and
69/// `requestReview` answer `true`, and `share` answers the `completed` flag
70/// from the activity handler. These were objects until now, and the reason is
71/// worth recording: `craft-bridge.js` settled with `payload || {}`, which
72/// turns a truthful `false` into a truthy `{}`, so a bare boolean could not
73/// carry a negative answer. Wrapping it was the workaround. The bug is fixed
74/// at its source (`_orEmpty`), so the wrapper is gone and a cancelled share
75/// reads as `false` again instead of `{completed:false}`, which every page
76/// written against the spec reads as success.
7077const R = struct {
71 const opened = "{\"opened\":true}";
72 const not_opened = "{\"opened\":false}";
73 const completed = "{\"completed\":true}";
74 const not_completed = "{\"completed\":false}";
75 const requested = "{\"requested\":true}";
78 const opened = "true";
79 const not_opened = "false";
80 const completed = "true";
81 const not_completed = "false";
82 const requested = "true";
7683
7784 const all = [_][]const u8{ opened, not_opened, completed, not_completed, requested };
7885};
@@ -1031,19 +1038,26 @@ test "every reply is a JSON object naming what happened, never a bare boolean" {
10311038 // `craft-bridge.js` resolves with `payload || {}`, so a bare `false` and an
10321039 // empty reply are the same value to the page.
10331040 for (R.all) |json| {
1034 const parsed = try std.json.parseFromSlice(std.json.Value, testing.allocator, json, .{});
1041 const parsed = try std.json.parseFromSlice(std.json.Value, testing.allocator, json, .{
1042 // A bare `true` is a JSON fragment, not a document. The spec
1043 // serialises with `.fragmentsAllowed` for the same reason.
1044 .allocate = .alloc_if_needed,
1045 });
10351046 defer parsed.deinit();
1036 try testing.expect(parsed.value == .object);
1037 try testing.expectEqual(@as(usize, 1), parsed.value.object.count());
10381047
1039 // And the member is a boolean the caller can branch on, not a string or
1040 // a number that reads as truthy whatever it says.
1041 var it = parsed.value.object.iterator();
1042 try testing.expect(it.next().?.value_ptr.* == .bool);
1048 // A boolean the caller can branch on — not a string, not a number, and
1049 // not an object, which is what these were while `payload || {}` made a
1050 // bare `false` unrepresentable.
1051 try testing.expect(parsed.value == .bool);
10431052 }
10441053
10451054 // Non-vacuity: every assertion above is satisfied by an empty list.
10461055 try testing.expectEqual(@as(usize, 5), R.all.len);
10471056 try testing.expect(!std.mem.eql(u8, R.opened, R.not_opened));
10481057 try testing.expect(!std.mem.eql(u8, R.completed, R.not_completed));
1058
1059 // The negative answers really are `false`, which is the whole point of the
1060 // change: an object here read as success to every page that branched on it.
1061 try testing.expectEqualStrings("false", R.not_opened);
1062 try testing.expectEqualStrings("false", R.not_completed);
10491063}
packages/zig/src/bridge_mobile_vision.zigmodified+20-2
Changes to packages/zig/src/bridge_mobile_vision.zig
@@ -416,15 +416,33 @@ fn shapeResults(allocator: std.mem.Allocator, kind: Kind, request: Id) ![]u8 {
416416 const count = try arrayCount(results);
417417 const limit = if (kind == .classify) @min(count, max_classifications) else count;
418418
419 // `written`, not `i`, decides the separator. A recognition observation with
420 // no candidate is skipped rather than emitted, exactly as the spec does, so
421 // the reply can be shorter than the observation list — and keying the comma
422 // off the loop index would then leave a leading or doubled one and hand the
423 // page invalid JSON.
424 var written: usize = 0;
419425 var i: usize = 0;
420426 while (i < limit) : (i += 1) {
421427 const observation = try arrayObjectAt(results, i);
422 if (i != 0) try out.append(allocator, ',');
428 const mark = out.items.len;
429 if (written != 0) try out.append(allocator, ',');
423430 switch (kind) {
424431 .classify => try appendClassification(allocator, &out, observation),
425432 .detect => try appendDetection(allocator, &out, observation),
426 .recognize => try appendRecognizedText(allocator, &out, observation),
433 // The one skippable case. Swift reads `topCandidates(1).first` and
434 // drops the observation when it is nil; rejecting the whole call
435 // instead loses every line that *did* recognise, which is the
436 // answer the page asked for.
437 .recognize => appendRecognizedText(allocator, &out, observation) catch |err| switch (err) {
438 error.NoCandidates => {
439 out.shrinkRetainingCapacity(mark);
440 continue;
441 },
442 else => return err,
443 },
427444 }
445 written += 1;
428446 }
429447
430448 try out.append(allocator, ']');
packages/zig/src/js/craft-bridge.jsmodified+11-2
Changes to packages/zig/src/js/craft-bridge.js
@@ -65,6 +65,15 @@
6565 }
6666 }
6767
68 // `payload || {}` was wrong for every action whose answer is a bare boolean:
69 // it turned a truthful `false` into `{}`, which is truthy, so a page reading
70 // `if (await craft.share(...))` took the success branch on a cancelled share.
71 // Only genuinely absent payloads become `{}`; `false`, `0` and `""` are
72 // answers and survive.
73 function _orEmpty(payload) {
74 return payload === undefined || payload === null ? {} : payload
75 }
76
6877 // Native's answer to one call. `id` is the `i` we sent, echoed back; a reply
6978 // raised outside any dispatch has none and falls back to the action queue.
7079 window.__craftBridgeResult = function (action, payload, id) {
@@ -77,7 +86,7 @@
7786 // call's own, with nothing to indicate anything had gone wrong.
7887 if (!e) return
7988 _forget(e)
80 if (e.resolve) e.resolve(payload || {})
89 if (e.resolve) e.resolve(_orEmpty(payload))
8190 return
8291 }
8392
@@ -87,7 +96,7 @@
8796 if (!q || q.length === 0) return
8897 const e = q[0]
8998 _forget(e)
90 if (e.resolve) e.resolve(payload || {})
99 if (e.resolve) e.resolve(_orEmpty(payload))
91100 }
92101
93102 // Native calls this when an action fails.