ReviewOS

also looking at this

craft-native/craft

feat(android): deleteCalendarEvent — the first action to answer through the channel

#155
Merged glennmichael123 wants to merge feat/android-delete-calendar-event into main
7 files +288 -3
9 days ago

Ratchet 87 → 86, and the reply channel (#152) gets its first user.

What the boolean means

The native returns whether Zig took the action, not whether the delete worked. The outcome reaches the page through the channel, so true is what stops the Kotlin also running and settling the promise a second time — the same distinction ios_dispatch draws between claiming an action and answering it.

The one thing deliberately not ported

"window._craftDeleteEventReject && window._craftDeleteEventReject('${e.message}')"

The message goes into a single-quoted JavaScript string unescaped, and eventId.toLong() puts the caller's own text in that message:

craft.calendar.delete("1'x")
  → NumberFormatException: For input string: "1'x"
  → …Reject('For input string: "1'x"')
                                  ^ string ends here → SyntaxError
  → evaluateJavascript runs nothing; neither resolve nor reject fires

The injected JS builds this promise by hand with no timeout, so the page waits forever with nothing in the console. Twenty other paths do the same — filed as #154.

Here the payload is JSON through appendJsonEscaped, so the same id rejects normally. A divergence in the page's favour, and deliberate.

toLong() is reproduced including what it refuses

Long.parseLong takes an optional sign and decimal digits and nothing else. Zig's parseInt also accepts 1_000. So the digits are checked before parsing — an id Zig accepted and the shim didn't is an id Zig would delete an event for that the shim would never have looked up.

Tested against "", "abc", "1'x", " 42", "0x2a", "1e3", "42L", "١٤٢", "1_000", and Long.MAX_VALUE + 1.

A test that passed for the wrong reason

The escaping test built its payload inline rather than calling rejectPayload — so removing the escaping from the code left it green. It was asserting about three lines rewritten in the test, not about the code.

mutationbeforeafter
digit pre-check droppedcaughtcaught
escaping dropped from the rejectionnot caughtcaught

Extracted into rejectPayload, which the test now calls. Worth flagging because it's the third time this session a test needed to be checked by mutating rather than trusted by reading.

zig build test:android → 112 tests, 11/11 steps · both .so build · generator 10/10.

7 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.