ts_control_serde: netmap deserialization fails on a peer's NextDNS split-DNS URL field

[a1ecbr0wn]

Summary

After a Device successfully registers and is authorized (AuthState::Authorized), the netmap update stream immediately fails to deserialize and the ControlRunner actor crashes, tearing down the whole Device with an opaque Error::Internal(Actor).

Environment

• tailscale = "0.5.0"
• Real, ordinary tailnet (not a fresh/empty one) with several existing devices, at least one of which has NextDNS split-DNS configured for that specific device (producing a per-device NextDNS block-page URL in its DNS config, of the form https://dns.nextdns.io/<id>?device_id=...&device_name=...&device_model=...&device_ip=...).

Observed error

ERROR ts_control::client::map_stream: deserializing netmap error=invalid type: string "https://dns.nextdns.io/<redacted>?device_id=<redacted>&device_name=<redacted>&device_model=&device_ip=<redacted>", expected a borrowed string at line 1 column 33219
ERROR actor.lifecycle{actor.name="ts_runtime::control_runner::ControlRunner" ...}:actor.handle_message{... kameo::message::StreamMessage<Arc<ts_control::client::map_stream::StateUpdate>, (), ()>}: ts_runtime::control_runner: state update stream terminated
Error: Internal(Actor)

"expected a borrowed string" (rather than a plain type mismatch) suggests this is a zero-copy (&str) deserialization target that can't borrow this particular string value directly from the input buffer — possibly because of how it's escaped/encoded in the JSON payload, or because the value straddles a buffer boundary. It reproduces consistently against this tailnet.

Impact

Any tailnet containing a device with this kind of NextDNS-linked DNS config field appears to make the whole Device unusable — Device::new() (or the first netmap update shortly after) crashes outright rather than skipping/tolerating the one field it can't parse. This is a harder blocker than #383 (registration error classification), since it happens even after successful authorization, on otherwise-unremarkable real-world tailnet state.

Suggested fix direction

Netmap/state deserialization has had similar fragility before (#123, closed in 0.3.0 for a different field) — it may be worth deserializing peer/DNS metadata fields more defensively in general (e.g. accepting an owned String instead of requiring a zero-copy &str borrow, or tolerating/skipping unparseable optional fields) rather than fixing this one field in isolation.

Repro

Happy to provide more detail/a minimal repro binary if useful — this was hit via a small throwaway Device::new() + tcp_listen() test program, not a customized setup.

[dylan-tailscale]

Hey, thanks for the report! I can replicate this using the raw JSON from the control plane, which is encoding ampersands as \u0026. While using an alloc::string::String instead does indeed work, we have some reasons to keep the zero copy approach:

1. Netmaps for large tailnets consume tens-to-hundreds of kilobytes in memory, purely as JSON. Copying str-like fields just to decode isn't a big deal for a single process, but if you're running hundreds of processes on the same machine, it starts to add up.
2. We want to eventually be able to run tailscale-rs everywhere, including devices with small amounts of RAM. Minimizing the amount of memory needed to deserialize netmaps is a big part of that, as receiving periodic netmap updates alongside an existing, already-deserialized netmap while processing packets at line(-ish) rate starts straining memory budgets on smaller devices. There's a ton of work to do to get there, but the ts_control_serde crate is already no_std and using zero-copy approaches, and we don't want to lose that progress.

Long story short, we'll discuss internally and figure out a way to solve for every field this without moving to alloc::string::String everywhere, then get the fix up! In the meantime, the owned String approach works for anyone that has a fork and isn't concerned about the tradeoffs above.