| @@ -416,3 +416,269 @@ test "the manifest renders and parses" { |
| 416 | 416 | try testing.expect(channels.count() > 0); |
| 417 | 417 | try testing.expect(channels.get("craft:fs:change") != null); |
| 418 | 418 | } |
| 419 | + |
| 420 | +test "every bridge type the page can send is routed by the dispatcher" { |
| 421 | + // The direction nothing checked. `test "the registry covers exactly the |
| 422 | + // namespaces the dispatcher routes"` pins the registry against the |
| 423 | + // dispatcher, and the channel test pins the events — but what the page can |
| 424 | + // *call* was never compared against what native will answer. |
| 425 | + // |
| 426 | + // A new `craft.foo.bar()` added to the facade with no arm in |
| 427 | + // `handleBridgeMessageJSON` posts a message that is parsed, matched |
| 428 | + // against every arm, and dropped. `_send` resolves the moment the message |
| 429 | + // leaves, so the caller gets a fulfilled promise and nothing happens — |
| 430 | + // silent, and exactly the shape of #27, #47 and #69. |
| 431 | + const chain = blk: { |
| 432 | + const start = std.mem.indexOf(u8, dispatcher_source, "pub fn handleBridgeMessageJSON") orelse |
| 433 | + return error.DispatcherNotFound; |
| 434 | + const end = std.mem.indexOfPos(u8, dispatcher_source, start, "\npub fn handleBridgeMessage(") orelse |
| 435 | + dispatcher_source.len; |
| 436 | + break :blk dispatcher_source[start..end]; |
| 437 | + }; |
| 438 | + |
| 439 | + var checked: usize = 0; |
| 440 | + var unrouted: usize = 0; |
| 441 | + |
| 442 | + // `_send('type', 'action'` / `_req(` / `_post(` — the three ways the |
| 443 | + // facade posts a message. |
| 444 | + for ([_][]const u8{ "_send('", "_req('", "_post('" }) |opener| { |
| 445 | + var search: usize = 0; |
| 446 | + while (std.mem.indexOfPos(u8, bridge_js, search, opener)) |hit| { |
| 447 | + search = hit + opener.len; |
| 448 | + const rest = bridge_js[search..]; |
| 449 | + const close = std.mem.indexOfScalar(u8, rest, '\'') orelse continue; |
| 450 | + const msg_type = rest[0..close]; |
| 451 | + if (msg_type.len == 0 or msg_type.len > 64) continue; |
| 452 | + |
| 453 | + checked += 1; |
| 454 | + |
| 455 | + var needle_buf: [128]u8 = undefined; |
| 456 | + const needle = try std.fmt.bufPrint(&needle_buf, "msg_type, \"{s}\"", .{msg_type}); |
| 457 | + if (std.mem.indexOf(u8, chain, needle) == null) { |
| 458 | + unrouted += 1; |
| 459 | + std.debug.print( |
| 460 | + "craft-bridge.js posts type '{s}' but handleBridgeMessageJSON does not route it\n", |
| 461 | + .{msg_type}, |
| 462 | + ); |
| 463 | + } |
| 464 | + } |
| 465 | + } |
| 466 | + |
| 467 | + // Floor: the facade posts hundreds of messages. If a refactor changes how |
| 468 | + // they are written this scan finds none and would otherwise pass having |
| 469 | + // compared nothing at all. |
| 470 | + try testing.expect(checked >= 200); |
| 471 | + try testing.expectEqual(@as(usize, 0), unrouted); |
| 472 | +} |
| 473 | + |
| 474 | +/// Bridges whose payload field names are checked against what the page sends. |
| 475 | +/// |
| 476 | +/// A ratchet like `max_undeclared`: this list may only grow. Adding a bridge |
| 477 | +/// here is what "the fields on both sides agree" costs, and it is cheap — |
| 478 | +/// every entry below was added after the audit that found nine bridges reading |
| 479 | +/// keys the page has never sent. |
| 480 | +const field_checked = [_]struct { |
| 481 | + /// The bridge type as the page names it in `_send`/`_req`. |
| 482 | + namespace: []const u8, |
| 483 | + source: []const u8, |
| 484 | +}{ |
| 485 | + .{ .namespace = "fs", .source = @embedFile("src/bridge_fs.zig") }, |
| 486 | + .{ .namespace = "window", .source = @embedFile("src/bridge_window.zig") }, |
| 487 | + .{ .namespace = "updater", .source = @embedFile("src/bridge_updater.zig") }, |
| 488 | + .{ .namespace = "bluetooth", .source = @embedFile("src/bridge_bluetooth.zig") }, |
| 489 | + .{ .namespace = "serial", .source = @embedFile("src/bridge_serial.zig") }, |
| 490 | + .{ .namespace = "localServer", .source = @embedFile("src/bridge_local_server.zig") }, |
| 491 | +}; |
| 492 | + |
| 493 | +/// The body of `fn <action>(...)`, from its signature to the next ` fn `. |
| 494 | +/// |
| 495 | +/// Narrowing the search to the handler is what makes this catch a name used in |
| 496 | +/// the *wrong* handler — `writeFile` reading a key only `readFile` sends. A |
| 497 | +/// file-wide search cannot: the first version of this test passed against the |
| 498 | +/// exact bug it was written for, because the name still appeared elsewhere in |
| 499 | +/// the same file. |
| 500 | +fn handlerBody(source: []const u8, action: []const u8) ?[]const u8 { |
| 501 | + var needle_buf: [96]u8 = undefined; |
| 502 | + const needle = std.fmt.bufPrint(&needle_buf, "\n fn {s}(", .{action}) catch return null; |
| 503 | + const start = std.mem.indexOf(u8, source, needle) orelse return null; |
| 504 | + const after = start + needle.len; |
| 505 | + |
| 506 | + // Ends at the next declaration *or* the next doc comment, whichever comes |
| 507 | + // first. Stopping only at `fn ` swallows the following function's `///` |
| 508 | + // block — and these bridges document their payloads in it, so |
| 509 | + // `appendFile`'s `/// JSON: {"content": "data"}` leaked into `writeFile`'s |
| 510 | + // span and made the check pass against the bug it was written for. |
| 511 | + const next_fn = std.mem.indexOfPos(u8, source, after, "\n fn ") orelse source.len; |
| 512 | + const next_doc = std.mem.indexOfPos(u8, source, after, "\n ///") orelse source.len; |
| 513 | + return source[start..@min(next_fn, next_doc)]; |
| 514 | +} |
| 515 | + |
| 516 | +/// Whether a bridge's source mentions `name` as a payload field. |
| 517 | +/// |
| 518 | +/// Two spellings, because bridges read payloads two ways. Some scan for a |
| 519 | +/// literal needle — `json_utils.getString(data, "path")` — where the name |
| 520 | +/// appears quoted. Others parse into a struct, where it is an unquoted field |
| 521 | +/// declaration: `path: []const u8 = ""`. A check that knew only the first |
| 522 | +/// reports every struct-based bridge as broken, which is exactly what the |
| 523 | +/// first version of this test did. |
| 524 | +fn bridgeKnowsField(source: []const u8, name: []const u8) bool { |
| 525 | + var quoted_buf: [96]u8 = undefined; |
| 526 | + const quoted = std.fmt.bufPrint("ed_buf, "\"{s}\"", .{name}) catch return true; |
| 527 | + if (std.mem.indexOf(u8, source, quoted) != null) return true; |
| 528 | + |
| 529 | + // A struct field: the name at the start of an indented line, followed by a |
| 530 | + // colon. Anchored on the newline so `path:` inside `filePath:` or a |
| 531 | + // comment does not count. |
| 532 | + var field_buf: [96]u8 = undefined; |
| 533 | + const field = std.fmt.bufPrint(&field_buf, "\n {s}: ", .{name}) catch return true; |
| 534 | + if (std.mem.indexOf(u8, source, field) != null) return true; |
| 535 | + |
| 536 | + // A struct field at any indentation, identified by its default: every |
| 537 | + // payload struct in these bridges declares one (`baud: u32 = 9600`). |
| 538 | + // |
| 539 | + // The default is what tells a field from a *parameter*. `fn writeFile(self: |
| 540 | + // *Self, data: []const u8)` contains `data: `, and accepting that made the |
| 541 | + // check pass against the exact bug it was written for — the handler was |
| 542 | + // reading the wrong key while its own parameter supplied the name. |
| 543 | + var nested_buf: [96]u8 = undefined; |
| 544 | + const nested = std.fmt.bufPrint(&nested_buf, "{s}: ", .{name}) catch return true; |
| 545 | + var search: usize = 0; |
| 546 | + while (std.mem.indexOfPos(u8, source, search, nested)) |hit| { |
| 547 | + search = hit + nested.len; |
| 548 | + if (hit != 0) { |
| 549 | + const before = source[hit - 1]; |
| 550 | + if (before != ' ' and before != '\n' and before != '\t') continue; |
| 551 | + } |
| 552 | + const line_end = std.mem.indexOfScalarPos(u8, source, hit, '\n') orelse source.len; |
| 553 | + if (std.mem.indexOfScalarPos(u8, source[0..line_end], hit, '=') != null) return true; |
| 554 | + } |
| 555 | + return false; |
| 556 | +} |
| 557 | + |
| 558 | +/// Fields a handler legitimately does not name. |
| 559 | +/// |
| 560 | +/// Narrowing the search to the handler is what makes the check catch a name |
| 561 | +/// read in the wrong place — but it also flags two honest patterns: a handler |
| 562 | +/// that reads the payload without keying off the field name, and one that |
| 563 | +/// delegates the parsing to a helper. Each exemption costs a line and a |
| 564 | +/// reason, which is the point: the list is short and every entry is arguable, |
| 565 | +/// rather than the check being weakened for everyone. |
| 566 | +const field_exempt = [_]struct { |
| 567 | + namespace: []const u8, |
| 568 | + action: []const u8, |
| 569 | + field: []const u8, |
| 570 | + why: []const u8, |
| 571 | +}{ |
| 572 | + .{ |
| 573 | + .namespace = "window", |
| 574 | + .action = "setAlwaysOnTop", |
| 575 | + .field = "value", |
| 576 | + .why = "scans the payload for the literal \"false\", so it reads the boolean without naming the key", |
| 577 | + }, |
| 578 | + .{ .namespace = "window", .action = "setResizable", .field = "value", .why = "same key-agnostic boolean scan" }, |
| 579 | + .{ .namespace = "window", .action = "setMovable", .field = "value", .why = "same key-agnostic boolean scan" }, |
| 580 | + .{ .namespace = "window", .action = "setHasShadow", .field = "value", .why = "same key-agnostic boolean scan" }, |
| 581 | + .{ |
| 582 | + .namespace = "bluetooth", |
| 583 | + .action = "connectDevice", |
| 584 | + .field = "id", |
| 585 | + .why = "delegates to `deviceAddress`, which accepts both \"id\" and \"address\"", |
| 586 | + }, |
| 587 | + .{ .namespace = "bluetooth", .action = "disconnectDevice", .field = "id", .why = "same helper" }, |
| 588 | +}; |
| 589 | + |
| 590 | +fn isExempt(namespace: []const u8, action: []const u8, field: []const u8) bool { |
| 591 | + for (field_exempt) |e| { |
| 592 | + if (std.mem.eql(u8, e.namespace, namespace) and |
| 593 | + std.mem.eql(u8, e.action, action) and |
| 594 | + std.mem.eql(u8, e.field, field)) return true; |
| 595 | + } |
| 596 | + return false; |
| 597 | +} |
| 598 | + |
| 599 | +test "every field the page sends is a name the handler actually looks for" { |
| 600 | + // The bug class this exists for, from issue #69 and the audit that |
| 601 | + // followed it: the page posts `{"data": "..."}`, the handler reads |
| 602 | + // `"content"`, and nothing objects. Every bridge parses with |
| 603 | + // `ignore_unknown_fields = true` or scans for a literal needle, so a name |
| 604 | + // that does not match is not an error — the value is simply gone and the |
| 605 | + // caller's promise resolves. |
| 606 | + // |
| 607 | + // It cost `craft.fs.writeFile` writing empty files, `setFullscreen(false)` |
| 608 | + // entering fullscreen, `setVibrancy` removing vibrancy, and Bluetooth |
| 609 | + // connects that could never name a device. |
| 610 | + // |
| 611 | + // The check is deliberately loose: it asks only whether the field name |
| 612 | + // appears *somewhere* in the handling bridge, quoted. That is enough to |
| 613 | + // catch a name the native side has never heard of, which is the whole |
| 614 | + // failure, without trying to model which handler owns which action. |
| 615 | + var checked: usize = 0; |
| 616 | + var missing: usize = 0; |
| 617 | + |
| 618 | + for ([_][]const u8{ "_send('", "_req('" }) |opener| { |
| 619 | + var search: usize = 0; |
| 620 | + while (std.mem.indexOfPos(u8, bridge_js, search, opener)) |hit| { |
| 621 | + search = hit + opener.len; |
| 622 | + const rest = bridge_js[search..]; |
| 623 | + |
| 624 | + const type_end = std.mem.indexOfScalar(u8, rest, '\'') orelse continue; |
| 625 | + const msg_type = rest[0..type_end]; |
| 626 | + |
| 627 | + // Only the bridges that have opted in. |
| 628 | + var source: ?[]const u8 = null; |
| 629 | + for (field_checked) |entry| { |
| 630 | + if (std.mem.eql(u8, entry.namespace, msg_type)) source = entry.source; |
| 631 | + } |
| 632 | + if (source == null) continue; |
| 633 | + |
| 634 | + // The action, so the search can be narrowed to its handler. |
| 635 | + const after_type = rest[type_end + 1 ..]; |
| 636 | + const a_open = std.mem.indexOf(u8, after_type, "'") orelse continue; |
| 637 | + const a_rest = after_type[a_open + 1 ..]; |
| 638 | + const a_end = std.mem.indexOfScalar(u8, a_rest, '\'') orelse continue; |
| 639 | + const action_name = a_rest[0..a_end]; |
| 640 | + |
| 641 | + // `fn <action>(` when the handler is named after the action, which |
| 642 | + // is the convention throughout. Falls back to the whole file when |
| 643 | + // it is not — looser, but never a false alarm. |
| 644 | + const scope = handlerBody(source.?, action_name) orelse source.?; |
| 645 | + |
| 646 | + // The `_stringify({ ... })` literal for this call, if it has one. |
| 647 | + const call_end = std.mem.indexOfScalarPos(u8, bridge_js, search, '\n') orelse bridge_js.len; |
| 648 | + const call = bridge_js[search..call_end]; |
| 649 | + const brace = std.mem.indexOf(u8, call, "_stringify({") orelse continue; |
| 650 | + const obj_start = brace + "_stringify({".len; |
| 651 | + const obj_end = std.mem.indexOfScalarPos(u8, call, obj_start, '}') orelse continue; |
| 652 | + const obj = call[obj_start..obj_end]; |
| 653 | + |
| 654 | + // Field names: an identifier followed by a colon. |
| 655 | + var i: usize = 0; |
| 656 | + while (i < obj.len) { |
| 657 | + while (i < obj.len and !std.ascii.isAlphabetic(obj[i])) i += 1; |
| 658 | + const name_start = i; |
| 659 | + while (i < obj.len and (std.ascii.isAlphanumeric(obj[i]) or obj[i] == '_')) i += 1; |
| 660 | + if (i >= obj.len or name_start == i) break; |
| 661 | + const name = obj[name_start..i]; |
| 662 | + var j = i; |
| 663 | + while (j < obj.len and obj[j] == ' ') j += 1; |
| 664 | + if (j >= obj.len or obj[j] != ':') continue; |
| 665 | + |
| 666 | + checked += 1; |
| 667 | + if (isExempt(msg_type, action_name, name)) continue; |
| 668 | + if (!bridgeKnowsField(scope, name)) { |
| 669 | + missing += 1; |
| 670 | + std.debug.print( |
| 671 | + "craft-bridge.js sends '{s}' to the {s} bridge, which never mentions that name —\n" ++ |
| 672 | + " the value is parsed and discarded, and the caller's promise still resolves.\n", |
| 673 | + .{ name, msg_type }, |
| 674 | + ); |
| 675 | + } |
| 676 | + } |
| 677 | + } |
| 678 | + } |
| 679 | + |
| 680 | + // Floor: these six bridges carry dozens of fields between them. A scan |
| 681 | + // that matched nothing would pass having compared nothing. |
| 682 | + try testing.expect(checked >= 30); |
| 683 | + try testing.expectEqual(@as(usize, 0), missing); |
| 684 | +} |