Skip to content

fix(weather): enforce read deadline + size cap + percent-encode geocode (closes #203) - #205

Merged
ai-hpc merged 1 commit into
GeniePod:mainfrom
galuis116:fix/weather-timeouts-and-url-encoding
May 27, 2026
Merged

fix(weather): enforce read deadline + size cap + percent-encode geocode (closes #203)#205
ai-hpc merged 1 commit into
GeniePod:mainfrom
galuis116:fix/weather-timeouts-and-url-encoding

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Summary

Three sibling concerns in crates/genie-core/src/tools/weather.rs's
raw-TCP HTTP client, closed by one PR over one file:

  1. No read timeout. http_get (pre-fix lines 182-228) had a 10 s
    TcpStream::connect timeout but nothing on write_all / read_line
    afterwards. A slow / hung Open-Meteo wedged the chat task forever.
    Same crash mode as bug [bug] ha/client: only TcpStream::connect has a timeout — any hung HA reply blocks chat/voice/dashboard tool calls indefinitely #173 / PR fix(ha): enforce request-deadline + response-size cap on HA HTTP client (closes #173) #174 for ha/client.
  2. No body-size cap. The body-accumulation loop had no bound. A
    misbehaving (or man-in-the-middle, since the connection is plain
    HTTP per the file comment) response could grow RSS without limit.
  3. Broken chunked-encoding decoder at pre-fix lines 216-225 joined
    non-hex lines with push_str (no separator → silent JSON corruption
    across chunks) and misfired its heuristic on legitimate responses
    whose first body char was a hex digit.
  4. URL encoding only handled spaces at pre-fix line 68. Any reserved
    RFC 3986 char in the location (&, ?, #, =, %, +) leaked
    into the geocode query string. "Q&A Cafe Tokyo" produced
    name=Q&A+Cafe+Tokyo&count=1… — Open-Meteo parsed it as name=Q
    and silently returned no match.

Closes #203.

Changes

  • crates/genie-core/src/tools/weather.rs:

    • New constants WEATHER_CONNECT_TIMEOUT = 10s,
      WEATHER_REQUEST_TIMEOUT = 15s, WEATHER_MAX_RESPONSE_BYTES = 1 MiB.
      Doc-commented with the failure mode each one prevents.
    • http_get(host, path) becomes a thin wrapper around new
      pub(crate) async fn http_get_with_limits(host, port, path, connect_timeout, request_timeout, max_bytes) so the test module can
      drive an ephemeral mock listener at millisecond-scale.
    • Connect uses tokio::time::timeout(connect_timeout, …) — clear
      "Open-Meteo connect to {addr} timed out" on Elapsed.
    • Post-connect (write + read) is wrapped in
      tokio::time::timeout(request_timeout, …) — clear
      "Open-Meteo GET {path} timed out after {N}s" on Elapsed.
    • New helper read_http_get_body(reader, max_bytes): bails on
      Transfer-Encoding: chunked with an explicit
      "Open-Meteo response uses unsupported chunked encoding"
      (replacing the old silently-corrupting decoder); bounds the body
      String with a check-and-bail on every appended line.
    • New helper url_encode_query_param(s: &str) -> String percent-
      encodes every non-RFC-3986-unreserved byte; multi-byte UTF-8 is
      encoded byte-by-byte (e.g. "München"M%C3%BCnchen). Hand-
      rolled, no new dependency.
    • geocode() swaps location.replace(' ', "+") for
      url_encode_query_param(location).
  • crates/genie-core/src/tools/weather.rs::tests — 5 new regressions:

    • url_encode_query_param_percent_encodes_reserved_chars — unit
      coverage on the helper: ASCII pass-through, every reserved char,
      multi-byte UTF-8, empty input.
    • hung_server_after_connect_times_out_cleanly — mock TcpListener
      accepts and sleep_forevers; asserts Err("…timed out…") inside
      the test budget. Pre-fix this hangs the whole test process.
    • oversized_response_is_size_capped — listener streams 16 KiB
      chunks past a 64 KiB cap; asserts Err("…exceeded…") before OOM.
    • chunked_encoding_is_explicitly_rejected — listener returns
      Transfer-Encoding: chunked; asserts the explicit
      Err("…chunked encoding…") instead of silent body corruption.
    • geocode_request_line_contains_percent_encoded_location — listener
      captures the raw HTTP request line and echoes it back as JSON;
      asserts the line contains name=Q%26A and does NOT contain
      name=Q&A. Locks the URL-encoding fix at the wire level.

No config schema change, no public API change. ASCII / space-only
locations against a healthy Open-Meteo see byte-identical behaviour.

Real Behavior Proof

  • Built and ran the affected code locally.
  • NOT verified on Jetson hardware. The change is in pure tokio I/O
    scheduling and string formatting — no audio, voice, ALSA, CUDA, HA,
    LLM-backend, or systemd surface — exercised end-to-end by 5 new
    #[tokio::test] regressions that spin up real local TcpListeners
    and drive each failure mode.

What I ran

Environment: x86_64 Linux dev host (Ubuntu 22.04, Rust 1.95.0). No
Jetson available.

cargo fmt --all -- --check                                                # clean
cargo clippy --workspace --all-targets -- -D warnings                    # clean
cargo clippy --workspace --all-targets --no-default-features -- -D warnings  # clean
cargo test -p genie-core --lib tools::weather                             # 6 / 0
cargo test                                                                # 675 / 0 / 3
cargo test --workspace --no-default-features                             # 570 / 0

What I observed

  1. Hung Open-Meteo is bounded. With a TcpListener that accepts
    then sleep_forevers, http_get_with_limits(…, 500ms, 500ms, …)
    returns Err("Open-Meteo GET … timed out after 0s") in well under
    a second. Pre-fix the same call hangs for 600 s.
  2. Oversized response is rejected before OOM. Listener streams
    16 KiB chunks against a 64 KiB cap; the helper bails with
    "Open-Meteo response exceeded 65536 bytes (got at least …)".
  3. Chunked encoding gets an explicit error. Listener returns
    Transfer-Encoding: chunked with a real chunk; helper bails with
    "Open-Meteo response uses unsupported chunked encoding". Pre-fix
    this silently corrupted multi-chunk JSON.
  4. URL encoding is wire-correct. Listener captures the raw HTTP
    request line via read and echoes it back as JSON. For
    geocode("Q&A Cafe Tokyo") the captured line is
    GET /v1/search?name=Q%26A%20Cafe%20Tokyo&count=1&language=en&format=json HTTP/1.1.
    Pre-fix this would have been name=Q&A+Cafe+Tokyo&…, which
    Open-Meteo parses as name=Q.
  5. No regression on the happy path. wmo_codes (existing test)
    still passes. Full cargo test: 675 / 0 / 3 (up from main baseline
    ~670 by exactly the 5 new tests). --no-default-features: 570 / 0.

Test plan

A reviewer can re-verify on any Rust 1.85+ host (no Jetson, no Open-
Meteo, no audio needed):

  • cargo test -p genie-core --lib tools::weather — 6 tests (1 existing
    • 5 new), all green in <1 s.
  • Optional manual proof against a real Open-Meteo:
    1. Start genie-core with default configuration.
    2. curl -X POST http://127.0.0.1:3000/api/chat \ -H 'Content-Type: application/json' \ -d '{"message":"weather in Q&A Cafe Tokyo"}'.
      Pre-fix: returns "location not found". Post-fix: returns a
      geocoded weather report for the closest matching location.
    3. Optionally tcpdump -i any 'port 80' to see
      GET /v1/search?name=Q%26A%20Cafe%20Tokyo&… on the wire.

Notes for reviewers

  • No merge-conflict surface with any open PR. The file
    crates/genie-core/src/tools/weather.rs has not been touched since
    the workspace scaffold commit 0a7363b. No open PR touches it.
  • Why three concerns in one PR. They live in the same function
    family (http_get + the three callers) and the test scaffolding
    (mock TcpListener + raw HTTP framing) is shared. Splitting them
    would multiply review surface for the same total diff.
  • Why reject chunked instead of fix the decoder. Open-Meteo's free
    tier returns Content-Length, never chunked. The pre-fix decoder
    silently corrupted multi-chunk JSON — refusing chunked explicitly is
    safer than fixing a decoder for a path that should be unreachable.
    A proper chunked decoder can land later if Open-Meteo ever changes.
  • Defaults. 15 s request timeout is comfortably above Open-Meteo's
    typical 200 ms response. 1 MiB is ~50x the largest realistic forecast
    response. Both are workspace-private constants; making them operator-
    configurable can be a follow-up if needed.
  • Related work. PR fix(ha): enforce request-deadline + response-size cap on HA HTTP client (closes #173) #174 (mine, merged) fixed the same unbounded-
    read pattern in ha/client. PR fix(llm): bound OpenAI-compat client timeouts to unwedge chat turn gate #182 (open) fixes the LLM client.
    This PR is the third leg — once it lands the "raw-TCP HTTP client
    with insufficient read timeouts" class is closed across the
    workspace.

@ai-hpc
ai-hpc merged commit 15c2c95 into GeniePod:main May 27, 2026
6 checks passed
@ai-hpc

ai-hpc commented May 27, 2026

Copy link
Copy Markdown
Contributor

reviewd, bounded weather HTTP reads and encoded geocode queries and merged at 15c2c95
Thanks @galuis116

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] tools/weather: unbounded HTTP read + silent chunked corruption + bare-spaces URL encoding lose chat or return wrong location

2 participants