ReviewOS
Reload

also looking at this

craft-native/craft

fix(ios): answer a malformed argument instead of leaving the page waiting

#246
Merged glennmichael123 wants to merge fix/ios-invalid-argument into main
2 files +136 -0
packages/ios/templates/CraftApp.swiftmodified+26-0
@@ -863,6 +863,8 @@ struct CraftWebView: UIViewRepresentable {
863863 case "setBadge":
864864 if let count = body["count"] as? Int {
865865 setBadgeCount(count, callbackId: callbackId)
866 } else {
867 rejectCallback(callbackId, error: "setBadge needs a whole number of badges", code: "INVALID_ARGUMENT")
866868 }
867869 case "clearBadge":
868870 setBadgeCount(0, callbackId: callbackId)
@@ -877,6 +879,8 @@ struct CraftWebView: UIViewRepresentable {
877879 case "setFlashlight":
878880 if let enabled = body["enabled"] as? Bool {
879881 setFlashlight(enabled: enabled, callbackId: callbackId)
882 } else {
883 rejectCallback(callbackId, error: "setFlashlight needs true or false", code: "INVALID_ARGUMENT")
880884 }
881885 // Vibrate pattern
882886 case "vibrate":
@@ -891,6 +895,8 @@ struct CraftWebView: UIViewRepresentable {
891895 if let urlString = body["url"] as? String, let url = URL(string: urlString) {
892896 UIApplication.shared.open(url)
893897 resolveCallback(callbackId, result: true)
898 } else {
899 rejectCallback(callbackId, error: "openURL needs a URL it can parse", code: "INVALID_ARGUMENT")
894900 }
895901 // App state
896902 case "getAppState":
@@ -1298,6 +1304,8 @@ struct CraftWebView: UIViewRepresentable {
12981304 case "setShortcuts":
12991305 if let shortcuts = body["shortcuts"] as? [[String: Any]] {
13001306 setAppShortcuts(shortcuts: shortcuts, callbackId: callbackId)
1307 } else {
1308 rejectCallback(callbackId, error: "setShortcuts needs a list of shortcuts", code: "INVALID_ARGUMENT")
13011309 }
13021310 case "clearShortcuts":
13031311 clearAppShortcuts(callbackId: callbackId)
@@ -1308,16 +1316,22 @@ struct CraftWebView: UIViewRepresentable {
13081316 let value = body["value"] as? String {
13091317 let group = body["group"] as? String
13101318 setSharedKeychainItem(key: key, value: value, group: group, callbackId: callbackId)
1319 } else {
1320 rejectCallback(callbackId, error: "setSharedItem needs both a key and a value", code: "INVALID_ARGUMENT")
13111321 }
13121322 case "getSharedItem":
13131323 if let key = body["key"] as? String {
13141324 let group = body["group"] as? String
13151325 getSharedKeychainItem(key: key, group: group, callbackId: callbackId)
1326 } else {
1327 rejectCallback(callbackId, error: "getSharedItem needs a key", code: "INVALID_ARGUMENT")
13161328 }
13171329 case "removeSharedItem":
13181330 if let key = body["key"] as? String {
13191331 let group = body["group"] as? String
13201332 removeSharedKeychainItem(key: key, group: group, callbackId: callbackId)
1333 } else {
1334 rejectCallback(callbackId, error: "removeSharedItem needs a key", code: "INVALID_ARGUMENT")
13211335 }
13221336
13231337 // MARK: - Local Auth Persistence
@@ -1325,6 +1339,8 @@ struct CraftWebView: UIViewRepresentable {
13251339 if let enabled = body["enabled"] as? Bool {
13261340 let duration = body["duration"] as? Double ?? 300 // 5 min default
13271341 setBiometricPersistence(enabled: enabled, duration: duration, callbackId: callbackId)
1342 } else {
1343 rejectCallback(callbackId, error: "setBiometricPersistence needs true or false", code: "INVALID_ARGUMENT")
13281344 }
13291345 case "checkBiometricPersistence":
13301346 checkBiometricPersistence(callbackId: callbackId)
@@ -1408,6 +1424,8 @@ struct CraftWebView: UIViewRepresentable {
14081424 case "updateWidget":
14091425 if let data = body["data"] as? [String: Any] {
14101426 updateWidget(data: data, callbackId: callbackId)
1427 } else {
1428 rejectCallback(callbackId, error: "updateWidget needs a data object", code: "INVALID_ARGUMENT")
14111429 }
14121430 case "reloadWidgets":
14131431 reloadAllWidgets(callbackId: callbackId)
@@ -1417,20 +1435,28 @@ struct CraftWebView: UIViewRepresentable {
14171435 if let phrase = body["phrase"] as? String,
14181436 let action = body["shortcutAction"] as? String {
14191437 registerSiriShortcut(phrase: phrase, action: action, callbackId: callbackId)
1438 } else {
1439 rejectCallback(callbackId, error: "registerSiriShortcut needs both a phrase and a shortcutAction", code: "INVALID_ARGUMENT")
14201440 }
14211441 case "removeSiriShortcut":
14221442 if let action = body["shortcutAction"] as? String {
14231443 removeSiriShortcut(action: action, callbackId: callbackId)
1444 } else {
1445 rejectCallback(callbackId, error: "removeSiriShortcut needs a shortcutAction", code: "INVALID_ARGUMENT")
14241446 }
14251447
14261448 // MARK: - Watch Connectivity
14271449 case "sendToWatch":
14281450 if let message = body["message"] as? [String: Any] {
14291451 sendMessageToWatch(message: message, callbackId: callbackId)
1452 } else {
1453 rejectCallback(callbackId, error: "sendToWatch needs a message object", code: "INVALID_ARGUMENT")
14301454 }
14311455 case "updateWatchContext":
14321456 if let context = body["context"] as? [String: Any] {
14331457 updateWatchContext(context: context, callbackId: callbackId)
1458 } else {
1459 rejectCallback(callbackId, error: "updateWatchContext needs a context object", code: "INVALID_ARGUMENT")
14341460 }
14351461 case "isWatchReachable":
14361462 isWatchReachable(callbackId: callbackId)
packages/zig/test/ios_conformance_test.zigmodified+110-0
@@ -1800,6 +1800,74 @@ fn caseUsesCallback(region: []const u8, action: []const u8, use: CaseUse) bool {
18001800 return false;
18011801}
18021802
1803/// The lines of the dispatcher's `case` for `action`, label excluded.
1804///
1805/// Same boundary rule as `caseUsesCallback`: from the label to the next line
1806/// at or above its indentation that starts another case, a `default:` or the
1807/// closing brace.
1808fn caseBody(region: []const u8, action: []const u8) ?[]const u8 {
1809 var case_indent: ?usize = null;
1810 var body_start: usize = 0;
1811 var offset: usize = 0;
1812 var lines = std.mem.splitScalar(u8, region, '\n');
1813 while (lines.next()) |line| {
1814 const next_offset = offset + line.len + 1;
1815 defer offset = next_offset;
1816 const body = std.mem.trimStart(u8, line, " \t");
1817 if (body.len == 0) continue;
1818 const indent = line.len - body.len;
1819
1820 if (case_indent) |at| {
1821 const closes = indent <= at and (std.mem.startsWith(u8, body, "case ") or
1822 std.mem.startsWith(u8, body, "default") or body[0] == '}');
1823 if (closes) return region[body_start..offset];
1824 continue;
1825 }
1826 if (std.mem.startsWith(u8, body, "case ") and labels(body, action)) {
1827 case_indent = indent;
1828 body_start = next_offset;
1829 }
1830 }
1831 if (case_indent != null) return region[body_start..];
1832 return null;
1833}
1834
1835/// Whether every `if let` written directly in this case has an `else`.
1836///
1837/// A case that casts its argument and answers only inside the `if` leaves the
1838/// page waiting for ever on a malformed one: `craft.setBadge('3')` posted a
1839/// callbackId and nothing settled it, on any path (#221). The `else` does not
1840/// have to reject — `caseUsesCallback` is what checks the case answers at all
1841/// — but a branch that simply is not there cannot.
1842///
1843/// Only the `if let`s at the case's own indentation are read. One nested
1844/// inside a closure or a helper belongs to that closure's contract, not to
1845/// this dispatch.
1846fn caseAnswersEveryPath(region: []const u8, action: []const u8) bool {
1847 const body = caseBody(region, action) orelse return true;
1848
1849 var if_indent: ?usize = null;
1850 var lines = std.mem.splitScalar(u8, body, '\n');
1851 while (lines.next()) |line| {
1852 const trimmed = std.mem.trimStart(u8, line, " \t");
1853 if (trimmed.len == 0 or std.mem.startsWith(u8, trimmed, "//")) continue;
1854 const indent = line.len - trimmed.len;
1855
1856 if (if_indent) |at| {
1857 if (indent != at) continue;
1858 // The line that closes it. `} else` in any form is the branch
1859 // this is looking for; a bare `}` is the gap.
1860 if (!std.mem.startsWith(u8, trimmed, "}")) continue;
1861 if (std.mem.indexOf(u8, trimmed, "else") == null) return false;
1862 if_indent = null;
1863 continue;
1864 }
1865 if (std.mem.startsWith(u8, trimmed, "if let ")) if_indent = indent;
1866 }
1867 // An `if let` still open at the end of the case never closed with an else.
1868 return if_indent == null;
1869}
1870
18031871fn specAnswers(region: []const u8, action: []const u8) bool {
18041872 return caseUsesCallback(region, action, .answers);
18051873}
@@ -1903,6 +1971,48 @@ test "every call the spec answers carries a callbackId to answer it on" {
19031971 try testing.expect(literal_posts >= 40);
19041972}
19051973
1974test "no case the page waits on casts its argument without answering the other way" {
1975 // #221: three cases — openURL, setBadge, setFlashlight — cast and then
1976 // answered only inside the `if`, so `craft.setBadge('3')` posted a
1977 // callbackId that nothing ever settled. The Siri pair had the same shape.
1978 // Zig refuses all five, so this is the path an app with no Zig runtime
1979 // takes, and the one a Zig build falls back to when the payload will not
1980 // serialize.
1981 const region = dispatcherRegion();
1982 var awaited = try collectAwaitedActions(testing.allocator);
1983 defer awaited.deinit();
1984 try testing.expect(awaited.count() >= 80);
1985
1986 // Cases still missing an `else`. It may only shrink: an entry here is a
1987 // page promise that never settles for a malformed argument.
1988 const known_gaps = [_][]const u8{};
1989
1990 // Every gap, not the first: one case at a time would hide the rest of a
1991 // class the whole point of this check is to see all of.
1992 var gaps: usize = 0;
1993 var it = awaited.keyIterator();
1994 while (it.next()) |action| {
1995 if (caseAnswersEveryPath(region, action.*)) continue;
1996 for (known_gaps) |allowed| {
1997 if (std.mem.eql(u8, allowed, action.*)) break;
1998 } else {
1999 gaps += 1;
2000 std.debug.print(
2001 "the page waits on `{s}`, and its case has an `if let` with no `else`.\n",
2002 .{action.*},
2003 );
2004 }
2005 }
2006 if (gaps > 0) {
2007 std.debug.print(
2008 " An argument that does not cast settles nothing at all, and the page waits\n" ++
2009 " out its own timeout instead of hearing INVALID_ARGUMENT.\n",
2010 .{},
2011 );
2012 return error.CaseLeavesAMalformedArgumentUnanswered;
2013 }
2014}
2015
19062016test "every call the page waits on has a case that can resolve it" {
19072017 const region = dispatcherRegion();
19082018 var awaited = try collectAwaitedActions(testing.allocator);