feat: webview support for real iOS devices#246
Open
gmegidish wants to merge 6 commits into
Open
Conversation
Adds WebView list/goto/reload/back/forward/content/evaluate/waitForLoadState for physical iOS devices. A real device can't dlopen an unsigned dylib like the simulator path, so the agent is injected into the foreground app via LLDB over the go-ios CoreDevice debug proxy, then driven over a forwarded HTTP/JSON-RPC port using the shared agent protocol (agentRequest/WebViewInfo). All device-specific code lives in devices/ios_device_webview.go so it never conflicts with the shared simulator/Android webview code. Vendors two small, dependency-light packages from go-ios that aren't in the released v1.0.211 this module uses: - devices/ios/debuggertools: ObjC-runtime-over-GDB calls, used to detect the foreground app without WDA. - devices/ios/debugserver: GDBServer with the bufio.Scanner buffer raised to 64MB so the multi-hundred-KB jGetLoadedDynamicLibrariesInfos image-list packet can be read; the 64KB default made LLDB load zero images (no symbols -> expr injection failed).
The injected agent persists inside the app after LLDB detaches, but the in-memory port cache is lost between CLI invocations, so every command re-ran the full (~13s) LLDB injection. Before injecting, probe the device agent port range (27042-27051): forward a local port to each and check whether the HTTP/JSON-RPC agent already answers. If one does, reuse it and skip findForegroundApp + LLDB entirely. A refused device port fails fast, so the cold-path tax is negligible. Caveat: this reuses whatever agent is alive on the device in that range; if the foreground app changed but the previous app is still running, it may talk to the previous app's agent.
…agent LLDB compiles the injected agent expression in ARC. Holding the result of evaluateJavaScript (jsResult = r) added a retain that, for any heap (non-tagged) value, got over-released when WebKit drained its delivery autorelease pool — crashing the target app right after the value was returned (EXC_BREAKPOINT in _CFRelease during objc_autoreleasePoolPop). Small/tagged results (short strings, numbers) survived, which is why `list` and trivial evals worked but `content` (large outerHTML) reliably killed the app. Rework the evaluate path to never hold WebKit's result object: the injected JS now returns JSON.stringify([ok, value]); the completion handler copies those UTF-8 bytes into C memory (strdup, no ObjC retain) and signals; the response is rebuilt off-thread from our own bytes, using arrays + a mutable dictionary to avoid the single-entry immutable dictionaries involved in the over-release. Verified on a real iOS 26.5 device: list, eval (strings/arrays/200K string), repeated content (full HTML), and goto all succeed with the app staying alive.
…/agent.m The injected LLDB expression was a ~17 KB / 238-line Go raw-string literal — roughly half of ios_device_webview.go. Extract it to agents/ios-real/agent.m and embed it from the agents package (same convention as the simulator dylib and Android agent), referenced as agents.IOSRealDeviceWebViewAgent. The .m lives under agents/ (not devices/) because a Go package may not contain C/.m source files without cgo, and //go:embed cannot reach across "..". It is an LLDB expression, not a standalone translation unit, so it is embedded, not compiled — the .m extension is just for editor highlighting. A leading newline is prepended after "expr -l objc --" so LLDB enters multi-line expression mode and treats the whole source as one expression (the original raw-string literal began with a newline; the embedded file does not).
The device agent hard-coded bounds to zero. Compute the webview's frame in
window coordinates (matching the simulator agent's
convertRect:bounds toView:nil). UIKit/CoreGraphics headers aren't on the LLDB
expression's include path, so call the struct-returning -bounds and
-convertRect:toView: through cast'd objc_msgSend with a CGRect-compatible
struct (CGFloat is double on arm64; arm64 has no objc_msgSend_stret).
Verified on a real iOS 26.5 device: bounds now report e.g.
{x:0, y:102, width:414, height:794}.
WalkthroughThis PR adds comprehensive WebView control for real iOS devices. It implements a GDB RSP wire layer, Mach-O export resolution, and an ObjC runtime bridge to call functions in a target process. An embedded Objective‑C HTTP/JSON‑RPC agent is injected via an LLDB proxy, binds a loopback port on the device, and exposes methods to list, navigate, evaluate JS in, and wait for load state of WKWebView instances. Host-side orchestration handles foreground app selection, LLDB proxying, port forwarding, and public IOSDevice WebView APIs. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/ios-real/agent.m`:
- Around line 173-206: The handler currently ignores params[@"args"] when
evaluating expressions; either consume those args or return an explicit error.
Update the block that builds `wrapped` (where `expr` is used) to read `id args =
params[@"args"]` and if args is present JSON-encode it and inject it into the JS
invocation (e.g. call the expression as a function with the parsed args) so the
JS receives the parameters, or if you prefer to disallow args, set `resp` to an
error JSON (similar to the missing id/expr branch) when args != nil; touch the
symbols `params`, `expr`, `wrapped`, and the evaluateJavaScript completion flow
so the injected args are sent safely (or explicitly rejected) before calling
`evaluateJavaScript`.
- Around line 241-244: The current sends for hdrData and resp may short-write
and ignore return values; update the send logic around hdrs/hdrData and resp to
loop until all bytes are written: for each buffer (hdrData.bytes/hdrData.length
and resp.bytes/resp.length) call send(cfd, ptr, remaining, 0), check the ssize_t
return, on positive advance ptr by bytes_sent and decrement remaining, on 0 or
-1 handle EINTR by retrying and treat other errors as fatal (close cfd/return);
ensure you use unsigned char* pointer arithmetic and preserve the original
buffers (hdrs/hdrData and resp) so the full HTTP header + JSON body is
transmitted.
In `@devices/ios_device_webview.go`:
- Around line 399-415: findRunningDeviceAgent currently reuses any responsive
agent on device ports 27042–27051 without verifying ownership, causing cross-app
hits; update the flow so that after establishing the port forward
(iosutil.NewPortForwarder) and before returning, perform an identity handshake
with the agent (extend isAgentReady or add a new helper like
getAgentIdentity(localPort) to return bundle ID or PID) and compare that
identity to the current foreground app (use or add a helper such as
getForegroundAppBundleID on IOSDevice or equivalent); only return localPort if
the identities match, otherwise call pf.Stop() and continue scanning, ensuring
no leftover forwards are reused for the wrong app.
- Around line 168-177: The code treats a PID as foreground because state
defaults to 0 and the error from rt.Call is ignored; change the logic to capture
and check the error from the applicationState call separately (e.g., appInst,
err := rt.ClassCall("UIApplication", "sharedApplication"); if err == nil {
state, callErr := rt.Call(appInst, "applicationState"); if callErr != nil { /*
mark state unknown / skip foreground logic */ } } ), only use state when the
rt.Call returned no error, and ensure the foreground branch (the condition using
state) requires both no errors and state == 0 (referencing appInst,
rt.ClassCall, rt.Call, and the state variable).
In `@devices/ios/debuggertools/macho.go`:
- Around line 258-267: The code drops errors during symbol bootstrap: capture
and handle errors returned by the first gdb.Request("qShlibInfoAddr") call,
validate fmt.Sscanf into shlibAddr (check its returned count), and
capture/handle the error from the second gdb.Request(fmt.Sprintf("m%x,%x",
shlibAddr, imageInfoArrayOff+8)) as well as the hex.DecodeString(resp) error
before proceeding; return a clear fmt.Errorf from the surrounding function on
any of these failures so symbol bootstrapping fails fast and reports the
underlying transport/parse/decode error (referencing gdb.Request,
qShlibInfoAddr, shlibAddr, fmt.Sscanf, imageInfoArrayOff, and hex.DecodeString).
- Around line 126-128: The calculation of trieAddr uses unsigned subtraction
that can underflow (uint64(exportFileOff) - linkeditFileOff); guard this by
validating offsets before computing trieAddr: ensure exportFileOff (converted to
uint64) is >= linkeditFileOff (or perform signed check) and that the resulting
delta is within the linkedit size bounds; if the check fails, return an error or
skip processing. Locate the trieAddr computation (the slide, linkeditVMAddr,
exportFileOff and linkeditFileOff variables) and add the conditional check and
appropriate error handling to avoid producing a bogus address.
In `@devices/ios/debuggertools/objc_runtime.go`:
- Around line 253-263: The writeData method currently doesn't verify that the
aligned write fits inside the single data page; before issuing the GDB write,
compute required := (uint64(len(data)) + 7) &^ 7 and check if m.dataOff+required
<= m.dataPageSize, and if not either allocate a fresh data page (implement a
small helper like m.allocDataPage() that updates m.dataAddr and resets
m.dataOff) or return a clear error; then proceed to call Request and update
m.dataOff by required (not recomputing alignment afterward). Ensure you
reference gdbMem.writeData, m.dataAddr, m.dataOff, and m.dataPageSize when
making the change.
- Around line 156-173: The Call method in ObjCRuntime currently treats a zero
return from objc_msgSend as an error; remove the post-call nil-check so valid
zero/false/nil returns (e.g., UIApplication.applicationState == 0) are not
surfaced as failures. Specifically, in ObjCRuntime.Call, after calling
rt.mem.call(rt.msgSend, ...) do not return an error when result == 0 — only
propagate the error returned by rt.mem.call; delete or disable the block that
does "if result == 0 { return 0, fmt.Errorf(...)" so callers can interpret a
zero result themselves.
In `@devices/ios/debugserver/gdbserver.go`:
- Around line 80-85: The Recv method on GDBServer currently treats a closed
scanner as a successful empty payload; change Recv so that when g.scanner.Scan()
returns false you check g.scanner.Err() and if that error is nil return io.EOF
(instead of nil error), otherwise return the scanner error; update imports to
include io if not present. This affects the Recv function and uses g.scanner and
g.scanner.Err() as the identifying symbols.
- Around line 40-60: The split function passed to scanner.Split currently
returns the bytes between '$' and '#' without verifying the two-byte hex
checksum; update that anonymous func to compute the modulo-256 checksum of
data[start+1:end], parse the two hex digits immediately after '#'
(data[end+1:end+3]), and compare them; if the checksum does not match, return 0,
nil, ErrInvalidGDBServerPayload (or another appropriate error) instead of
accepting the packet so corrupted frames are rejected before further parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48ae61b4-5355-4e30-aba0-e78b90311d88
📒 Files selected for processing (8)
agents/agents.goagents/ios-real/agent.mdevices/ios/debuggertools/LICENSEdevices/ios/debuggertools/macho.godevices/ios/debuggertools/objc_runtime.godevices/ios/debugserver/LICENSEdevices/ios/debugserver/gdbserver.godevices/ios_device_webview.go
Comment on lines
+173
to
+206
| NSString *wvId = params[@"id"], *expr = params[@"expression"]; | ||
| id wv = (wvId && expr) ? __findWV(wvId) : nil; | ||
| if (!wvId || !expr) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"missing id or expression"}} options:0 error:nil]; | ||
| else if (!wv) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"webview not found"}} options:0 error:nil]; | ||
| else { | ||
| // A heap (non-tagged) value returned from evaluateJavaScript is | ||
| // over-released when WebKit's delivery pool drains, crashing the | ||
| // app — so we must never hold WebKit's result object. Instead the | ||
| // JS JSON-stringifies [ok, value]; inside the handler we copy the | ||
| // resulting bytes to C memory (no ObjC retain) and rebuild the | ||
| // response from our own bytes. Arrays + a mutable dict avoid the | ||
| // single-entry immutable dictionaries seen in the crash. | ||
| NSString *wrapped = [NSString stringWithFormat:@"(function(){try{return JSON.stringify([1,(function(){%@})()])}catch(e){return JSON.stringify([0,''+e])}})()", expr]; | ||
| __block char *jbuf = NULL; | ||
| id sem = dispatch_semaphore_create(0); | ||
| [[NSOperationQueue mainQueue] addOperationWithBlock:^{ | ||
| (void)[wv evaluateJavaScript:wrapped completionHandler:^(id r, NSError *e) { | ||
| if ([(NSObject *)r isKindOfClass:[NSString class]]) { const char *u = [(NSString *)r UTF8String]; if (u) jbuf = strdup(u); } | ||
| dispatch_semaphore_signal(sem); | ||
| }]; | ||
| }]; | ||
| dispatch_semaphore_wait(sem, dispatch_time(0, 30000000000LL)); | ||
| if (!jbuf) { | ||
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"no result from evaluate"}} options:0 error:nil]; | ||
| } else { | ||
| NSArray *parsed = [NSJSONSerialization JSONObjectWithData:[NSData dataWithBytes:jbuf length:strlen(jbuf)] options:0 error:nil]; | ||
| free(jbuf); | ||
| BOOL ok2 = [(NSObject *)parsed isKindOfClass:[NSArray class]] && [parsed count] == 2; | ||
| if (ok2 && [(NSNumber *)parsed[0] intValue] == 0) { | ||
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":parsed[1]}} options:0 error:nil]; | ||
| } else { | ||
| NSMutableDictionary *rd = [NSMutableDictionary dictionary]; | ||
| rd[@"result"] = ok2 ? parsed[1] : [NSNull null]; | ||
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"result":rd} options:0 error:nil]; |
There was a problem hiding this comment.
Honor args in device.webview.evaluate or reject them explicitly.
IOSDevice.WebViewEvaluate sends params["args"], but this handler never reads them. Any caller relying on parameterized evaluation gets incorrect behavior with no error.
Minimal safe fix
- NSString *wvId = params[@"id"], *expr = params[@"expression"];
+ NSString *wvId = params[@"id"], *expr = params[@"expression"];
+ NSArray *args = params[@"args"];
id wv = (wvId && expr) ? __findWV(wvId) : nil;
if (!wvId || !expr) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"missing id or expression"}} options:0 error:nil];
+ else if ([args isKindOfClass:[NSArray class]] && [args count] > 0) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"args not supported on real iOS yet"}} options:0 error:nil];
else if (!wv) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"webview not found"}} options:0 error:nil];
else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| NSString *wvId = params[@"id"], *expr = params[@"expression"]; | |
| id wv = (wvId && expr) ? __findWV(wvId) : nil; | |
| if (!wvId || !expr) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"missing id or expression"}} options:0 error:nil]; | |
| else if (!wv) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"webview not found"}} options:0 error:nil]; | |
| else { | |
| // A heap (non-tagged) value returned from evaluateJavaScript is | |
| // over-released when WebKit's delivery pool drains, crashing the | |
| // app — so we must never hold WebKit's result object. Instead the | |
| // JS JSON-stringifies [ok, value]; inside the handler we copy the | |
| // resulting bytes to C memory (no ObjC retain) and rebuild the | |
| // response from our own bytes. Arrays + a mutable dict avoid the | |
| // single-entry immutable dictionaries seen in the crash. | |
| NSString *wrapped = [NSString stringWithFormat:@"(function(){try{return JSON.stringify([1,(function(){%@})()])}catch(e){return JSON.stringify([0,''+e])}})()", expr]; | |
| __block char *jbuf = NULL; | |
| id sem = dispatch_semaphore_create(0); | |
| [[NSOperationQueue mainQueue] addOperationWithBlock:^{ | |
| (void)[wv evaluateJavaScript:wrapped completionHandler:^(id r, NSError *e) { | |
| if ([(NSObject *)r isKindOfClass:[NSString class]]) { const char *u = [(NSString *)r UTF8String]; if (u) jbuf = strdup(u); } | |
| dispatch_semaphore_signal(sem); | |
| }]; | |
| }]; | |
| dispatch_semaphore_wait(sem, dispatch_time(0, 30000000000LL)); | |
| if (!jbuf) { | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"no result from evaluate"}} options:0 error:nil]; | |
| } else { | |
| NSArray *parsed = [NSJSONSerialization JSONObjectWithData:[NSData dataWithBytes:jbuf length:strlen(jbuf)] options:0 error:nil]; | |
| free(jbuf); | |
| BOOL ok2 = [(NSObject *)parsed isKindOfClass:[NSArray class]] && [parsed count] == 2; | |
| if (ok2 && [(NSNumber *)parsed[0] intValue] == 0) { | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":parsed[1]}} options:0 error:nil]; | |
| } else { | |
| NSMutableDictionary *rd = [NSMutableDictionary dictionary]; | |
| rd[@"result"] = ok2 ? parsed[1] : [NSNull null]; | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"result":rd} options:0 error:nil]; | |
| NSString *wvId = params[@"id"], *expr = params[@"expression"]; | |
| NSArray *args = params[@"args"]; | |
| id wv = (wvId && expr) ? __findWV(wvId) : nil; | |
| if (!wvId || !expr) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"missing id or expression"}} options:0 error:nil]; | |
| else if ([args isKindOfClass:[NSArray class]] && [args count] > 0) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32602),@"message":@"args not supported on real iOS yet"}} options:0 error:nil]; | |
| else if (!wv) resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"webview not found"}} options:0 error:nil]; | |
| else { | |
| // A heap (non-tagged) value returned from evaluateJavaScript is | |
| // over-released when WebKit's delivery pool drains, crashing the | |
| // app — so we must never hold WebKit's result object. Instead the | |
| // JS JSON-stringifies [ok, value]; inside the handler we copy the | |
| // resulting bytes to C memory (no ObjC retain) and rebuild the | |
| // response from our own bytes. Arrays + a mutable dict avoid the | |
| // single-entry immutable dictionaries seen in the crash. | |
| NSString *wrapped = [NSString stringWithFormat:@"(function(){try{return JSON.stringify([1,(function(){%@})()])}catch(e){return JSON.stringify([0,''+e])}})()", expr]; | |
| __block char *jbuf = NULL; | |
| id sem = dispatch_semaphore_create(0); | |
| [[NSOperationQueue mainQueue] addOperationWithBlock:^{ | |
| (void)[wv evaluateJavaScript:wrapped completionHandler:^(id r, NSError *e) { | |
| if ([(NSObject *)r isKindOfClass:[NSString class]]) { const char *u = [(NSString *)r UTF8String]; if (u) jbuf = strdup(u); } | |
| dispatch_semaphore_signal(sem); | |
| }]; | |
| }]; | |
| dispatch_semaphore_wait(sem, dispatch_time(0, 30000000000LL)); | |
| if (!jbuf) { | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":@"no result from evaluate"}} options:0 error:nil]; | |
| } else { | |
| NSArray *parsed = [NSJSONSerialization JSONObjectWithData:[NSData dataWithBytes:jbuf length:strlen(jbuf)] options:0 error:nil]; | |
| free(jbuf); | |
| BOOL ok2 = [(NSObject *)parsed isKindOfClass:[NSArray class]] && [parsed count] == 2; | |
| if (ok2 && [(NSNumber *)parsed[0] intValue] == 0) { | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"error":@{@"code":@(-32000),@"message":parsed[1]}} options:0 error:nil]; | |
| } else { | |
| NSMutableDictionary *rd = [NSMutableDictionary dictionary]; | |
| rd[@"result"] = ok2 ? parsed[1] : [NSNull null]; | |
| resp = [NSJSONSerialization dataWithJSONObject:@{@"jsonrpc":@"2.0",@"id":rqId,@"result":rd} options:0 error:nil]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agents/ios-real/agent.m` around lines 173 - 206, The handler currently
ignores params[@"args"] when evaluating expressions; either consume those args
or return an explicit error. Update the block that builds `wrapped` (where
`expr` is used) to read `id args = params[@"args"]` and if args is present
JSON-encode it and inject it into the JS invocation (e.g. call the expression as
a function with the parsed args) so the JS receives the parameters, or if you
prefer to disallow args, set `resp` to an error JSON (similar to the missing
id/expr branch) when args != nil; touch the symbols `params`, `expr`, `wrapped`,
and the evaluateJavaScript completion flow so the injected args are sent safely
(or explicitly rejected) before calling `evaluateJavaScript`.
Comment on lines
+241
to
+244
| NSString *hdrs = [NSString stringWithFormat:@"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n", (unsigned long)resp.length]; | ||
| NSData *hdrData = [hdrs dataUsingEncoding:NSASCIIStringEncoding]; | ||
| send(cfd, hdrData.bytes, hdrData.length, 0); | ||
| send(cfd, resp.bytes, resp.length, 0); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does POSIX send(2) guarantee writing the full buffer on a blocking TCP socket, or can it return a short count that requires looping until all bytes are written?
💡 Result:
No. POSIX send(2) (and POSIX send in general) does not guarantee that a single blocking call writes the entire buffer; it can return a short, nonnegative count that requires looping to send the remaining bytes. POSIX says that upon successful completion send returns “the number of bytes sent” [1], and that when the message does not fit into the socket’s send buffer, send normally blocks (for blocking sockets) until space is available [1]. However, “returning the number of bytes sent” means the call may transfer only part of the requested length, depending on how much space is available/consumed at the time of the call; therefore callers must handle partial writes by checking the return value and sending the rest as needed. POSIX also explicitly states that “successful completion … does not guarantee delivery of the message” [2]. Practical rule: always loop on send/write until all bytes are sent (handling -1/EINTR, etc.), even when the socket is blocking.
Citations:
- 1: https://pubs.opengroup.org/onlinepubs/9699919799/functions/send.html
- 2: https://pubs.opengroup.org/onlinepubs/7908799/xns/send.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and print the relevant region with line numbers
ls -la agents/ios-real/agent.m || true
python3 - <<'PY'
import pathlib
p = pathlib.Path("agents/ios-real/agent.m")
print("exists:", p.exists())
PY
if [ -f agents/ios-real/agent.m ]; then
nl -ba agents/ios-real/agent.m | sed -n '200,280p'
fi
# Find all send() calls in this file to see if there is any existing write-all helper/loop
rg -n --fixed-string "send(cfd" agents/ios-real/agent.m || true
rg -n --fixed-string "send(" agents/ios-real/agent.m || trueRepository: mobile-next/mobilecli
Length of output: 214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="agents/ios-real/agent.m"
# Show the relevant region (roughly around the reported lines)
if [ -f "$FILE" ]; then
echo "---- $FILE (lines 220-270) ----"
sed -n '220,270p' "$FILE" | cat -n
fi
# Find send(cfd, ...) call sites
echo "---- send(cfd, ...) call sites ----"
rg -n --fixed-string "send(cfd" "$FILE" || true
# Look for how cfd/socket is created/configured (blocking vs non-blocking)
echo "---- cfd/socket setup references ----"
rg -n --context 2 -E "cfd|socket|connect|accept|fcntl|O_NONBLOCK|set.*nonblock|timeout" "$FILE" || true
# Show function/logic around the request/response write, if we can narrow by earlier markers
echo "---- response header/body write nearby ----"
rg -n --context 5 -E "HTTP/1\.1|Content-Length|Content-Type|Connection: close|hdrs|hdrData" "$FILE" || trueRepository: mobile-next/mobilecli
Length of output: 3486
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="agents/ios-real/agent.m"
echo "---- file size ----"
wc -l "$FILE"
echo "---- relevant send() calls ----"
rg -n --fixed-strings "send(cfd" "$FILE" || true
echo "---- all send() calls ----"
rg -n --fixed-strings "send(" "$FILE" || true
echo "---- header/body content-length occurrences ----"
rg -n "Content-Length" "$FILE" || true
rg -n "Content-Type" "$FILE" || true
echo "---- socket setup / blocking references ----"
rg -n "accept\(|socket\(|bind\(|listen\(|connect\(|fcntl\(|O_NONBLOCK|nonblock|setsockopt\(|SO_RCVTIMEO|SO_SNDTIMEO|timeout" "$FILE" || true
echo "---- show larger context around the response write ----"
sed -n '200,280p' "$FILE" | cat -nRepository: mobile-next/mobilecli
Length of output: 6257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="agents/ios-real/agent.m"
sed -n '1,130p' "$FILE" | cat -nRepository: mobile-next/mobilecli
Length of output: 8480
Loop on send() until the full HTTP header + JSON body bytes are written (agents/ios-real/agent.m:241-244)
Single send(cfd, ...) calls are allowed to perform short writes (returning a smaller positive byte count than requested), even for blocking TCP sockets; the current code ignores the return value, so large JSON/HTML-eval responses can be truncated and break the client payload.
Suggested fix
- send(cfd, hdrData.bytes, hdrData.length, 0);
- send(cfd, resp.bytes, resp.length, 0);
+ const char *hdrBytes = (const char *)hdrData.bytes;
+ NSUInteger hdrOff = 0;
+ while (hdrOff < hdrData.length) {
+ long wrote = send(cfd, hdrBytes + hdrOff, hdrData.length - hdrOff, 0);
+ if (wrote <= 0) break;
+ hdrOff += (NSUInteger)wrote;
+ }
+ const char *respBytes = (const char *)resp.bytes;
+ NSUInteger respOff = 0;
+ while (respOff < resp.length) {
+ long wrote = send(cfd, respBytes + respOff, resp.length - respOff, 0);
+ if (wrote <= 0) break;
+ respOff += (NSUInteger)wrote;
+ }
close(cfd);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| NSString *hdrs = [NSString stringWithFormat:@"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n", (unsigned long)resp.length]; | |
| NSData *hdrData = [hdrs dataUsingEncoding:NSASCIIStringEncoding]; | |
| send(cfd, hdrData.bytes, hdrData.length, 0); | |
| send(cfd, resp.bytes, resp.length, 0); | |
| NSString *hdrs = [NSString stringWithFormat:@"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n", (unsigned long)resp.length]; | |
| NSData *hdrData = [hdrs dataUsingEncoding:NSASCIIStringEncoding]; | |
| const char *hdrBytes = (const char *)hdrData.bytes; | |
| NSUInteger hdrOff = 0; | |
| while (hdrOff < hdrData.length) { | |
| long wrote = send(cfd, hdrBytes + hdrOff, hdrData.length - hdrOff, 0); | |
| if (wrote <= 0) break; | |
| hdrOff += (NSUInteger)wrote; | |
| } | |
| const char *respBytes = (const char *)resp.bytes; | |
| NSUInteger respOff = 0; | |
| while (respOff < resp.length) { | |
| long wrote = send(cfd, respBytes + respOff, resp.length - respOff, 0); | |
| if (wrote <= 0) break; | |
| respOff += (NSUInteger)wrote; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agents/ios-real/agent.m` around lines 241 - 244, The current sends for
hdrData and resp may short-write and ignore return values; update the send logic
around hdrs/hdrData and resp to loop until all bytes are written: for each
buffer (hdrData.bytes/hdrData.length and resp.bytes/resp.length) call send(cfd,
ptr, remaining, 0), check the ssize_t return, on positive advance ptr by
bytes_sent and decrement remaining, on 0 or -1 handle EINTR by retrying and
treat other errors as fatal (close cfd/return); ensure you use unsigned char*
pointer arithmetic and preserve the original buffers (hdrs/hdrData and resp) so
the full HTTP header + JSON body is transmitted.
Comment on lines
+168
to
+177
| appInst, err := rt.ClassCall("UIApplication", "sharedApplication") | ||
| var state uint64 | ||
| if err == nil { | ||
| state, _ = rt.Call(appInst, "applicationState") | ||
| } | ||
| rt.Cleanup() | ||
| gdb.Request(fmt.Sprintf("D;%x", app.pid)) //nolint:errcheck | ||
| conn.Close() | ||
| utils.Verbose("pid %d (%s) applicationState=%d", app.pid, app.bundleID, state) | ||
| if err == nil && state == 0 { |
There was a problem hiding this comment.
Don’t classify a PID as foreground when applicationState failed.
state defaults to 0, and the error from rt.Call is dropped. If that selector lookup/invocation fails, the first attachable app is treated as foreground.
Suggested fix
- appInst, err := rt.ClassCall("UIApplication", "sharedApplication")
- var state uint64
- if err == nil {
- state, _ = rt.Call(appInst, "applicationState")
- }
+ appInst, err := rt.ClassCall("UIApplication", "sharedApplication")
+ var (
+ state uint64
+ stateErr error
+ )
+ if err == nil {
+ state, stateErr = rt.Call(appInst, "applicationState")
+ }
rt.Cleanup()
gdb.Request(fmt.Sprintf("D;%x", app.pid)) //nolint:errcheck
conn.Close()
utils.Verbose("pid %d (%s) applicationState=%d", app.pid, app.bundleID, state)
- if err == nil && state == 0 {
+ if err == nil && stateErr == nil && state == 0 {
utils.Verbose("foreground app: %s (pid %d)", app.bundleID, app.pid)
return app, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| appInst, err := rt.ClassCall("UIApplication", "sharedApplication") | |
| var state uint64 | |
| if err == nil { | |
| state, _ = rt.Call(appInst, "applicationState") | |
| } | |
| rt.Cleanup() | |
| gdb.Request(fmt.Sprintf("D;%x", app.pid)) //nolint:errcheck | |
| conn.Close() | |
| utils.Verbose("pid %d (%s) applicationState=%d", app.pid, app.bundleID, state) | |
| if err == nil && state == 0 { | |
| appInst, err := rt.ClassCall("UIApplication", "sharedApplication") | |
| var ( | |
| state uint64 | |
| stateErr error | |
| ) | |
| if err == nil { | |
| state, stateErr = rt.Call(appInst, "applicationState") | |
| } | |
| rt.Cleanup() | |
| gdb.Request(fmt.Sprintf("D;%x", app.pid)) //nolint:errcheck | |
| conn.Close() | |
| utils.Verbose("pid %d (%s) applicationState=%d", app.pid, app.bundleID, state) | |
| if err == nil && stateErr == nil && state == 0 { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios_device_webview.go` around lines 168 - 177, The code treats a PID
as foreground because state defaults to 0 and the error from rt.Call is ignored;
change the logic to capture and check the error from the applicationState call
separately (e.g., appInst, err := rt.ClassCall("UIApplication",
"sharedApplication"); if err == nil { state, callErr := rt.Call(appInst,
"applicationState"); if callErr != nil { /* mark state unknown / skip foreground
logic */ } } ), only use state when the rt.Call returned no error, and ensure
the foreground branch (the condition using state) requires both no errors and
state == 0 (referencing appInst, rt.ClassCall, rt.Call, and the state variable).
Comment on lines
+399
to
+415
| func (d *IOSDevice) findRunningDeviceAgent() (int, bool) { | ||
| for devPort := 27042; devPort <= 27051; devPort++ { | ||
| localPort, err := findFreeLocalPort(27052, 27151) | ||
| if err != nil { | ||
| return 0, false | ||
| } | ||
| pf := iosutil.NewPortForwarder(d.Udid) | ||
| if err := pf.Forward(localPort, devPort); err != nil { | ||
| continue | ||
| } | ||
| if isAgentReady(localPort) { | ||
| utils.Verbose("reusing running agent on device port %d (local %d)", devPort, localPort) | ||
| return localPort, true | ||
| } | ||
| pf.Stop() //nolint:errcheck | ||
| } | ||
| return 0, false |
There was a problem hiding this comment.
Verify agent identity before reusing a leftover port.
This reuses the first agent that answers on 27042-27051 without checking which app owns it. If a previously injected app is still alive, later list/eval/navigation calls can hit the wrong app’s webviews.
A safer fast path is to have the agent expose its bundle ID or PID in the readiness handshake and compare that to the current foreground app before caching/reusing the port.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios_device_webview.go` around lines 399 - 415, findRunningDeviceAgent
currently reuses any responsive agent on device ports 27042–27051 without
verifying ownership, causing cross-app hits; update the flow so that after
establishing the port forward (iosutil.NewPortForwarder) and before returning,
perform an identity handshake with the agent (extend isAgentReady or add a new
helper like getAgentIdentity(localPort) to return bundle ID or PID) and compare
that identity to the current foreground app (use or add a helper such as
getForegroundAppBundleID on IOSDevice or equivalent); only return localPort if
the identities match, otherwise call pf.Stop() and continue scanning, ensuring
no leftover forwards are reused for the wrong app.
Comment on lines
+126
to
+128
| slide := baseAddr - textVMAddr | ||
| trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff) | ||
|
|
There was a problem hiding this comment.
Guard unsigned arithmetic before computing trieAddr.
On Line 127, uint64(exportFileOff) - linkeditFileOff can underflow if offsets are inconsistent/malformed, yielding a huge bogus address.
Suggested fix
- slide := baseAddr - textVMAddr
- trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff)
+ if baseAddr < textVMAddr || uint64(exportFileOff) < linkeditFileOff {
+ return 0
+ }
+ slide := baseAddr - textVMAddr
+ trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| slide := baseAddr - textVMAddr | |
| trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff) | |
| if baseAddr < textVMAddr || uint64(exportFileOff) < linkeditFileOff { | |
| return 0 | |
| } | |
| slide := baseAddr - textVMAddr | |
| trieAddr := slide + linkeditVMAddr + (uint64(exportFileOff) - linkeditFileOff) | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debuggertools/macho.go` around lines 126 - 128, The calculation
of trieAddr uses unsigned subtraction that can underflow (uint64(exportFileOff)
- linkeditFileOff); guard this by validating offsets before computing trieAddr:
ensure exportFileOff (converted to uint64) is >= linkeditFileOff (or perform
signed check) and that the resulting delta is within the linkedit size bounds;
if the check fails, return an error or skip processing. Locate the trieAddr
computation (the slide, linkeditVMAddr, exportFileOff and linkeditFileOff
variables) and add the conditional check and appropriate error handling to avoid
producing a bogus address.
Comment on lines
+258
to
+267
| resp, _ := gdb.Request("qShlibInfoAddr") | ||
| if resp == "" || strings.HasPrefix(resp, "E") { | ||
| return nil, fmt.Errorf("qShlibInfoAddr: %s", resp) | ||
| } | ||
| var shlibAddr uint64 | ||
| fmt.Sscanf(resp, "%x", &shlibAddr) | ||
|
|
||
| // Read version + count + infoArray pointer | ||
| resp, _ = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8)) | ||
| shlibData, _ := hex.DecodeString(resp) |
There was a problem hiding this comment.
Don’t drop transport/parse/decode errors in symbol bootstrap.
Lines 258-267 ignore gdb.Request errors, Sscanf result, and hex.DecodeString errors. Failures here get misreported later as symbol misses.
Suggested fix
- resp, _ := gdb.Request("qShlibInfoAddr")
+ resp, err := gdb.Request("qShlibInfoAddr")
+ if err != nil {
+ return nil, fmt.Errorf("qShlibInfoAddr request failed: %w", err)
+ }
if resp == "" || strings.HasPrefix(resp, "E") {
return nil, fmt.Errorf("qShlibInfoAddr: %s", resp)
}
var shlibAddr uint64
- fmt.Sscanf(resp, "%x", &shlibAddr)
+ if n, scanErr := fmt.Sscanf(resp, "%x", &shlibAddr); scanErr != nil || n != 1 {
+ return nil, fmt.Errorf("invalid qShlibInfoAddr response: %q", resp)
+ }
// Read version + count + infoArray pointer
- resp, _ = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8))
- shlibData, _ := hex.DecodeString(resp)
+ resp, err = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8))
+ if err != nil {
+ return nil, fmt.Errorf("read dyld_all_image_infos failed: %w", err)
+ }
+ shlibData, err := hex.DecodeString(resp)
+ if err != nil {
+ return nil, fmt.Errorf("invalid dyld_all_image_infos hex payload: %w", err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resp, _ := gdb.Request("qShlibInfoAddr") | |
| if resp == "" || strings.HasPrefix(resp, "E") { | |
| return nil, fmt.Errorf("qShlibInfoAddr: %s", resp) | |
| } | |
| var shlibAddr uint64 | |
| fmt.Sscanf(resp, "%x", &shlibAddr) | |
| // Read version + count + infoArray pointer | |
| resp, _ = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8)) | |
| shlibData, _ := hex.DecodeString(resp) | |
| resp, err := gdb.Request("qShlibInfoAddr") | |
| if err != nil { | |
| return nil, fmt.Errorf("qShlibInfoAddr request failed: %w", err) | |
| } | |
| if resp == "" || strings.HasPrefix(resp, "E") { | |
| return nil, fmt.Errorf("qShlibInfoAddr: %s", resp) | |
| } | |
| var shlibAddr uint64 | |
| if n, scanErr := fmt.Sscanf(resp, "%x", &shlibAddr); scanErr != nil || n != 1 { | |
| return nil, fmt.Errorf("invalid qShlibInfoAddr response: %q", resp) | |
| } | |
| // Read version + count + infoArray pointer | |
| resp, err = gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8)) | |
| if err != nil { | |
| return nil, fmt.Errorf("read dyld_all_image_infos failed: %w", err) | |
| } | |
| shlibData, err := hex.DecodeString(resp) | |
| if err != nil { | |
| return nil, fmt.Errorf("invalid dyld_all_image_infos hex payload: %w", err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debuggertools/macho.go` around lines 258 - 267, The code drops
errors during symbol bootstrap: capture and handle errors returned by the first
gdb.Request("qShlibInfoAddr") call, validate fmt.Sscanf into shlibAddr (check
its returned count), and capture/handle the error from the second
gdb.Request(fmt.Sprintf("m%x,%x", shlibAddr, imageInfoArrayOff+8)) as well as
the hex.DecodeString(resp) error before proceeding; return a clear fmt.Errorf
from the surrounding function on any of these failures so symbol bootstrapping
fails fast and reports the underlying transport/parse/decode error (referencing
gdb.Request, qShlibInfoAddr, shlibAddr, fmt.Sscanf, imageInfoArrayOff, and
hex.DecodeString).
Comment on lines
+156
to
+173
| func (rt *ObjCRuntime) Call(receiver uint64, selector string, args ...uint64) (uint64, error) { | ||
| if receiver == 0 { | ||
| return 0, fmt.Errorf("nil receiver for [? %s]", selector) | ||
| } | ||
| sel, err := rt.sel(selector) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| callArgs := make([]uint64, 0, 2+len(args)) | ||
| callArgs = append(callArgs, receiver, sel) | ||
| callArgs = append(callArgs, args...) | ||
| result, err := rt.mem.call(rt.msgSend, callArgs...) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("objc_msgSend(%s): %w", selector, err) | ||
| } | ||
| if result == 0 { | ||
| return 0, fmt.Errorf("[0x%x %s] returned nil", receiver, selector) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Objective-C on iOS, can methods invoked via objc_msgSendlegitimately return0/NO/nilwithout that meaning a transport failure, and what numeric value doesUIApplication.applicationState use for the active state?
💡 Result:
In Objective-C, methods invoked via objc_msgSend can legitimately return 0, NO, or nil without it representing a transport failure [1]. This behavior is a fundamental design feature of the Objective-C runtime: sending a message to a nil receiver does not cause a crash or a transport error; instead, it is treated as a no-op that returns 0 (for scalar or pointer return types) [2][1]. Because the runtime simply returns the zeroed-out value when the receiver is nil, this result is indistinguishable from a method that intentionally returns 0, NO, or nil [3][1]. Regarding UIApplication.applicationState, the active state is represented by the enum constant UIApplicationStateActive (or UIApplication.State.active in Swift) [4][5]. As an enumeration, this value uses an underlying integer, which is 0 [5]. Summary of return behavior for messages sent to nil: - Objects: Returns nil (0) [1]. - Scalar values (ints, chars, etc.): Returns 0 [3][1]. - Structs: Returns 0.0 for every field if the struct is returned in registers; otherwise, the result is undefined [1].
Citations:
- 1: https://stackoverflow.com/questions/2696891/calling-a-method-on-an-uninitialized-object-null-pointer
- 2: https://www.mikeash.com/pyblog/friday-qa-2012-12-14-objective-c-pitfalls.html
- 3: https://ridiculousfish.com/blog/posts/nil.html
- 4: https://developer.apple.com/documentation/uikit/uiapplication/state
- 5: https://stackoverflow.com/questions/7937400/need-clarification-about-uiapplicationstate
Don’t treat a zero objc_msgSend return as a generic failure in ObjCRuntime.Call.
In devices/ios/debuggertools/objc_runtime.go (around the if result == 0 check), Objective-C message sends can legitimately return 0/NO/nil without indicating a transport/runtime failure. Specifically, UIApplication.applicationState’s active state (UIApplicationStateActive) is the enum value 0, so this layer currently turns a valid “active” result into an error.
Suggested fix
func (rt *ObjCRuntime) Call(receiver uint64, selector string, args ...uint64) (uint64, error) {
if receiver == 0 {
return 0, fmt.Errorf("nil receiver for [? %s]", selector)
}
sel, err := rt.sel(selector)
if err != nil {
return 0, err
}
callArgs := make([]uint64, 0, 2+len(args))
callArgs = append(callArgs, receiver, sel)
callArgs = append(callArgs, args...)
result, err := rt.mem.call(rt.msgSend, callArgs...)
if err != nil {
return 0, fmt.Errorf("objc_msgSend(%s): %w", selector, err)
}
- if result == 0 {
- return 0, fmt.Errorf("[0x%x %s] returned nil", receiver, selector)
- }
return result, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (rt *ObjCRuntime) Call(receiver uint64, selector string, args ...uint64) (uint64, error) { | |
| if receiver == 0 { | |
| return 0, fmt.Errorf("nil receiver for [? %s]", selector) | |
| } | |
| sel, err := rt.sel(selector) | |
| if err != nil { | |
| return 0, err | |
| } | |
| callArgs := make([]uint64, 0, 2+len(args)) | |
| callArgs = append(callArgs, receiver, sel) | |
| callArgs = append(callArgs, args...) | |
| result, err := rt.mem.call(rt.msgSend, callArgs...) | |
| if err != nil { | |
| return 0, fmt.Errorf("objc_msgSend(%s): %w", selector, err) | |
| } | |
| if result == 0 { | |
| return 0, fmt.Errorf("[0x%x %s] returned nil", receiver, selector) | |
| } | |
| func (rt *ObjCRuntime) Call(receiver uint64, selector string, args ...uint64) (uint64, error) { | |
| if receiver == 0 { | |
| return 0, fmt.Errorf("nil receiver for [? %s]", selector) | |
| } | |
| sel, err := rt.sel(selector) | |
| if err != nil { | |
| return 0, err | |
| } | |
| callArgs := make([]uint64, 0, 2+len(args)) | |
| callArgs = append(callArgs, receiver, sel) | |
| callArgs = append(callArgs, args...) | |
| result, err := rt.mem.call(rt.msgSend, callArgs...) | |
| if err != nil { | |
| return 0, fmt.Errorf("objc_msgSend(%s): %w", selector, err) | |
| } | |
| return result, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debuggertools/objc_runtime.go` around lines 156 - 173, The Call
method in ObjCRuntime currently treats a zero return from objc_msgSend as an
error; remove the post-call nil-check so valid zero/false/nil returns (e.g.,
UIApplication.applicationState == 0) are not surfaced as failures. Specifically,
in ObjCRuntime.Call, after calling rt.mem.call(rt.msgSend, ...) do not return an
error when result == 0 — only propagate the error returned by rt.mem.call;
delete or disable the block that does "if result == 0 { return 0,
fmt.Errorf(...)" so callers can interpret a zero result themselves.
Comment on lines
+253
to
+263
| func (m *gdbMem) writeData(data []byte) (uint64, error) { | ||
| addr := m.dataAddr + m.dataOff | ||
| r, err := m.gdb.Request(fmt.Sprintf("M%x,%x:%s", addr, len(data), hexEncode(data))) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| if r != "OK" { | ||
| return 0, fmt.Errorf("write: %s", r) | ||
| } | ||
| m.dataOff = (m.dataOff + uint64(len(data)) + 7) &^ 7 | ||
| return addr, nil |
There was a problem hiding this comment.
Bound-check the remote data page before writing.
writeData never verifies that the aligned write still fits in the single dataPageSize allocation. After enough selector/class/string allocations, this starts writing past the reserved page and fails unpredictably.
Suggested fix
func (m *gdbMem) writeData(data []byte) (uint64, error) {
+ nextOff := (m.dataOff + uint64(len(data)) + 7) &^ 7
+ if nextOff > dataPageSize {
+ return 0, fmt.Errorf("remote data page overflow: need %d bytes, %d remaining", len(data), dataPageSize-m.dataOff)
+ }
addr := m.dataAddr + m.dataOff
r, err := m.gdb.Request(fmt.Sprintf("M%x,%x:%s", addr, len(data), hexEncode(data)))
if err != nil {
return 0, err
}
if r != "OK" {
return 0, fmt.Errorf("write: %s", r)
}
- m.dataOff = (m.dataOff + uint64(len(data)) + 7) &^ 7
+ m.dataOff = nextOff
return addr, nil
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debuggertools/objc_runtime.go` around lines 253 - 263, The
writeData method currently doesn't verify that the aligned write fits inside the
single data page; before issuing the GDB write, compute required :=
(uint64(len(data)) + 7) &^ 7 and check if m.dataOff+required <= m.dataPageSize,
and if not either allocate a fresh data page (implement a small helper like
m.allocDataPage() that updates m.dataAddr and resets m.dataOff) or return a
clear error; then proceed to call Request and update m.dataOff by required (not
recomputing alignment afterward). Ensure you reference gdbMem.writeData,
m.dataAddr, m.dataOff, and m.dataPageSize when making the change.
Comment on lines
+40
to
+60
| scanner.Split(func(data []byte, atEOF bool) (int, []byte, error) { | ||
| const lenPacketSuffix = 3 // len("#00") | ||
|
|
||
| if atEOF && len(data) == 0 { | ||
| return 0, nil, nil | ||
| } | ||
|
|
||
| start := bytes.IndexRune(data, '$') | ||
| end := bytes.IndexRune(data, '#') | ||
| // Need more data | ||
| if start < 0 || end < 0 || len(data) < end+lenPacketSuffix { | ||
| return 0, nil, nil | ||
| } | ||
| // Invalid data | ||
| if end < start { | ||
| return 0, nil, ErrInvalidGDBServerPayload | ||
| } | ||
|
|
||
| // Strip the $ prefix before returning | ||
| return end + lenPacketSuffix, data[start+1 : end], nil | ||
| }) |
There was a problem hiding this comment.
Validate RSP checksum before accepting a packet.
On Line 59, the split function returns payload bytes without checking the #xx checksum. A corrupted frame is treated as valid and can cascade into wrong memory/symbol parsing.
Suggested fix
scanner.Split(func(data []byte, atEOF bool) (int, []byte, error) {
const lenPacketSuffix = 3 // len("`#00`")
@@
- // Strip the $ prefix before returning
- return end + lenPacketSuffix, data[start+1 : end], nil
+ payload := data[start+1 : end]
+ recvChecksumHex := data[end+1 : end+3]
+ recvChecksum, err := hex.DecodeString(string(recvChecksumHex))
+ if err != nil || len(recvChecksum) != 1 {
+ return 0, nil, ErrInvalidGDBServerPayload
+ }
+ sum := 0
+ for _, b := range payload {
+ sum += int(b)
+ }
+ if recvChecksum[0] != byte(sum%256) {
+ return 0, nil, ErrInvalidGDBServerPayload
+ }
+
+ // Strip the $ prefix before returning
+ return end + lenPacketSuffix, payload, nil
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debugserver/gdbserver.go` around lines 40 - 60, The split
function passed to scanner.Split currently returns the bytes between '$' and '#'
without verifying the two-byte hex checksum; update that anonymous func to
compute the modulo-256 checksum of data[start+1:end], parse the two hex digits
immediately after '#' (data[end+1:end+3]), and compare them; if the checksum
does not match, return 0, nil, ErrInvalidGDBServerPayload (or another
appropriate error) instead of accepting the packet so corrupted frames are
rejected before further parsing.
Comment on lines
+80
to
+85
| func (g *GDBServer) Recv() (string, error) { | ||
| if g.scanner.Scan() == false { | ||
| return "", g.scanner.Err() | ||
| } | ||
| return g.scanner.Text(), nil | ||
| } |
There was a problem hiding this comment.
Return io.EOF when the stream closes.
On Lines 81-83, Scan()==false with no scanner error returns ("", nil). That hides connection closure as a successful empty payload.
Suggested fix
func (g *GDBServer) Recv() (string, error) {
- if g.scanner.Scan() == false {
- return "", g.scanner.Err()
- }
+ if g.scanner.Scan() == false {
+ if err := g.scanner.Err(); err != nil {
+ return "", err
+ }
+ return "", io.EOF
+ }
return g.scanner.Text(), nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (g *GDBServer) Recv() (string, error) { | |
| if g.scanner.Scan() == false { | |
| return "", g.scanner.Err() | |
| } | |
| return g.scanner.Text(), nil | |
| } | |
| func (g *GDBServer) Recv() (string, error) { | |
| if g.scanner.Scan() == false { | |
| if err := g.scanner.Err(); err != nil { | |
| return "", err | |
| } | |
| return "", io.EOF | |
| } | |
| return g.scanner.Text(), nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devices/ios/debugserver/gdbserver.go` around lines 80 - 85, The Recv method
on GDBServer currently treats a closed scanner as a successful empty payload;
change Recv so that when g.scanner.Scan() returns false you check
g.scanner.Err() and if that error is nil return io.EOF (instead of nil error),
otherwise return the scanner error; update imports to include io if not present.
This affects the Recv function and uses g.scanner and g.scanner.Err() as the
identifying symbols.
The injected agent scanned 27042-27051 for a free port and the reuse probe scanned the same range. Bind a single fixed port (12008) instead: it removes the scan on both the agent and host sides and keeps reuse discovery trivial (the simulator path avoids this differently, via an exported mobilecli_get_port symbol, which an injected expression can't offer). Also quieten --verbose: log lldb-proxy device responses as a byte count rather than dumping the (often binary) packet contents. Verified on a real iOS 26.5 device: fresh inject binds 12008, the reuse fast-path finds it, and list/content work.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/ios-real/agent.m`:
- Around line 50-51: The code currently binds __sfd to a fixed port
(htons(12008)) and on bind failure it leaves the socket open; update the failure
path in the bind block to call close(__sfd) and set __sfd to -1 (or another
sentinel) so the descriptor is not leaked, and only set __port = 12008 when bind
returns success; modify the block that calls bind(__sfd, (struct __sockaddr
*)&__sa, sizeof(__sa)) to perform the close(__sfd) cleanup on the non-zero
return path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: befdfd7b-ddf9-4218-bf83-06ade2d68468
📒 Files selected for processing (2)
agents/ios-real/agent.mdevices/ios_device_webview.go
🚧 Files skipped from review as they are similar to previous changes (1)
- devices/ios_device_webview.go
Comment on lines
+50
to
+51
| __sa.sin_port = htons((unsigned short)12008); | ||
| if (bind(__sfd, (struct __sockaddr *)&__sa, sizeof(__sa)) == 0) { __port = 12008; } |
There was a problem hiding this comment.
Close __sfd when the fixed-port bind fails.
With a hard-coded port, bind() can now fail whenever the agent is already running on 12008 or another service owns that port. On that path the newly created socket is leaked in the target app process because __sfd is never closed.
Suggested fix
__sa.sin_port = htons((unsigned short)12008);
-if (bind(__sfd, (struct __sockaddr *)&__sa, sizeof(__sa)) == 0) { __port = 12008; }
+if (bind(__sfd, (struct __sockaddr *)&__sa, sizeof(__sa)) == 0) {
+ __port = 12008;
+} else {
+ close(__sfd);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| __sa.sin_port = htons((unsigned short)12008); | |
| if (bind(__sfd, (struct __sockaddr *)&__sa, sizeof(__sa)) == 0) { __port = 12008; } | |
| __sa.sin_port = htons((unsigned short)12008); | |
| if (bind(__sfd, (struct __sockaddr *)&__sa, sizeof(__sa)) == 0) { | |
| __port = 12008; | |
| } else { | |
| close(__sfd); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agents/ios-real/agent.m` around lines 50 - 51, The code currently binds __sfd
to a fixed port (htons(12008)) and on bind failure it leaves the socket open;
update the failure path in the bind block to call close(__sfd) and set __sfd to
-1 (or another sentinel) so the descriptor is not leaked, and only set __port =
12008 when bind returns success; modify the block that calls bind(__sfd, (struct
__sockaddr *)&__sa, sizeof(__sa)) to perform the close(__sfd) cleanup on the
non-zero return path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds WebView support for physical iOS devices (
list,goto,reload,back,forward,content,eval,wait), complementing the existing simulator + Android support from #244.A real device can't
dlopenan unsigned dylib like the simulator path, so the agent is injected into the foreground app via LLDB over the go-ios CoreDevice debug proxy, then driven over a forwarded HTTP/JSON-RPC port using the shared agent protocol (agentRequest/WebViewInfo).All device-specific code lives in
devices/ios_device_webview.go(andagents/ios-real/agent.m) so it never conflicts with the shared simulator/Android webview code.Notable pieces
v1.0.211this module uses:devices/ios/debuggertools— ObjC-runtime-over-GDB calls, used to detect the foreground app without WDA.devices/ios/debugserver—GDBServerwith thebufio.Scannerbuffer raised to 64 MB. The default 64 KB cap truncated thejGetLoadedDynamicLibrariesInfosimage-list packet, which made LLDB load zero images (no symbols → expression injection failed).evaluateJavaScript's result over-released any heap value when WebKit drained its delivery pool (crashed the app oncontent). The agent now JSON-stringifies[ok, value]and copies the bytes out in C without ever retaining WebKit's object.agents/ios-real/agent.m(embedded via//go:embed) rather than a Go raw-string literal.Testing
Verified end-to-end on a real iPhone (iOS 26.5):
list,eval(strings, arrays, a 200 K string), repeatedcontent(full HTML),goto, and bounds — all succeed with the app staying alive and repeat calls hitting the reuse path.