A model download that stalls (server accepts the connection then stops sending) hangs forever. The Cancel button in the UI does nothing, because the cancel flag is only checked between reads and the read never returns. For an 80 GiB artifact over a flaky link this is a realistic and unrecoverable state — the user has to quit the app.
verify() at src/model/transfer.rs:186-196 has the same shape, but reads from a local file, so it is not affected.
Second, smaller problem on the same line: .https_only(url.starts_with("https://")) only enforces HTTPS when the URL is already HTTPS, which is a no-op. URLs are built from constants (src/model.rs:258), so nothing is exploitable today, but the guard reads as protection it does not provide. It should be .https_only(true).
How ds4 handles this
../ds4 gives every socket operation a named timeout constant — it is the house convention, not an afterthought:
ds4_web.c:169web_tcp_connect(host, port, timeout_ms, ...) and ds4_web.c:234web_read_some(fd, buf, len, timeout_ms) — every read goes through a poll() with an explicit deadline; ds4_web.c:486-493 turns an expired deadline into a "websocket read timeout" error rather than a hang.
ds4_server.c:45 — DS4_SERVER_IO_TIMEOUT_SEC 10, applied to every accepted socket at ds4_server.c:12498-12503 via SO_RCVTIMEO/SO_SNDTIMEO.
Note for honesty: ds4's model downloader is download_model.sh, which shells out to curl -fL --progress-meter -C - (download_model.sh:335) and also sets no explicit timeout. But that runs in a terminal where a stalled transfer is visible in the progress meter and killable with Ctrl-C. DS4Server is a GUI where the same stall is invisible and the Cancel button is inert — so the missing timeout is materially worse here than in the reference.
Suggested fix
Configure the agent with an overall and a per-read deadline, following ds4's named-constant convention:
Check the ureq 3.x config surface for the body-read stall timeout name before writing it — the crate has renamed these across versions. A stalled read should surface as Err("Model download failed: ..."), which the existing resume logic already handles: the partial file is kept and download_url_to_partial will re-Range from where it stopped on the next attempt (src/model/transfer.rs:233-253).
Acceptance criteria
A download against a server that accepts and then stalls fails with a readable error within ~30 s instead of hanging.
After the failure the .part file is intact and a retry resumes from the existing offset (verified by the existing download_artifact resume tests in src/model/transfer.rs:420+).
Cancel during a healthy download still works.
.https_only(true) unconditionally.
## Symptom
A model download that stalls (server accepts the connection then stops sending) hangs forever. The Cancel button in the UI does nothing, because the cancel flag is only checked *between* reads and the read never returns. For an 80 GiB artifact over a flaky link this is a realistic and unrecoverable state — the user has to quit the app.
## Evidence
`src/model/transfer.rs:234-244`:
```rust
let agent: ureq::Agent = ureq::Agent::config_builder()
.https_only(url.starts_with("https://"))
.build()
.into();
```
No `timeout_global`, `timeout_connect`, `timeout_recv_response`, or `timeout_recv_body`. `ureq` applies no deadline by default.
The cancel check is outside the blocking call:
```rust
loop {
if cancel.load(Ordering::Relaxed) { return Ok(DownloadOutcome::Stopped); } // :266
let count = body.read(&mut buffer)...; // :269 blocks forever
```
`verify()` at `src/model/transfer.rs:186-196` has the same shape, but reads from a local file, so it is not affected.
Second, smaller problem on the same line: `.https_only(url.starts_with("https://"))` only enforces HTTPS when the URL is *already* HTTPS, which is a no-op. URLs are built from constants (`src/model.rs:258`), so nothing is exploitable today, but the guard reads as protection it does not provide. It should be `.https_only(true)`.
## How ds4 handles this
`../ds4` gives every socket operation a named timeout constant — it is the house convention, not an afterthought:
- `ds4_web.c:30-31` — `DS4_WEB_CONNECT_TIMEOUT_MS 3000`, `DS4_WEB_CDP_TIMEOUT_MS 20000`
- `ds4_web.c:169` `web_tcp_connect(host, port, timeout_ms, ...)` and `ds4_web.c:234` `web_read_some(fd, buf, len, timeout_ms)` — every read goes through a `poll()` with an explicit deadline; `ds4_web.c:486-493` turns an expired deadline into a `"websocket read timeout"` error rather than a hang.
- `ds4_server.c:45` — `DS4_SERVER_IO_TIMEOUT_SEC 10`, applied to every accepted socket at `ds4_server.c:12498-12503` via `SO_RCVTIMEO`/`SO_SNDTIMEO`.
Note for honesty: ds4's *model downloader* is `download_model.sh`, which shells out to `curl -fL --progress-meter -C -` (`download_model.sh:335`) and also sets no explicit timeout. But that runs in a terminal where a stalled transfer is visible in the progress meter and killable with Ctrl-C. DS4Server is a GUI where the same stall is invisible and the Cancel button is inert — so the missing timeout is materially worse here than in the reference.
## Suggested fix
Configure the agent with an overall and a per-read deadline, following ds4's named-constant convention:
```rust
const DOWNLOAD_STALL_TIMEOUT: Duration = Duration::from_secs(30);
let agent: ureq::Agent = ureq::Agent::config_builder()
.https_only(true)
.timeout_connect(Some(Duration::from_secs(10)))
.timeout_recv_response(Some(DOWNLOAD_STALL_TIMEOUT))
.build()
.into();
```
Check the `ureq` 3.x config surface for the body-read stall timeout name before writing it — the crate has renamed these across versions. A stalled read should surface as `Err("Model download failed: ...")`, which the existing resume logic already handles: the partial file is kept and `download_url_to_partial` will re-`Range` from where it stopped on the next attempt (`src/model/transfer.rs:233-253`).
## Acceptance criteria
- A download against a server that accepts and then stalls fails with a readable error within ~30 s instead of hanging.
- After the failure the `.part` file is intact and a retry resumes from the existing offset (verified by the existing `download_artifact` resume tests in `src/model/transfer.rs:420+`).
- Cancel during a healthy download still works.
- `.https_only(true)` unconditionally.
hugo
added the bug label 2026-07-25 09:39:28 +00:00
Fixed in 429cbc7. Model downloads now enforce a 10 s connect timeout plus 30 s response/body stall timeouts, and production downloads are HTTPS-only. Added a loopback regression test proving a stalled transfer times out without damaging the resumable partial file. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.
Fixed in 429cbc7. Model downloads now enforce a 10 s connect timeout plus 30 s response/body stall timeouts, and production downloads are HTTPS-only. Added a loopback regression test proving a stalled transfer times out without damaging the resumable partial file. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Symptom
A model download that stalls (server accepts the connection then stops sending) hangs forever. The Cancel button in the UI does nothing, because the cancel flag is only checked between reads and the read never returns. For an 80 GiB artifact over a flaky link this is a realistic and unrecoverable state — the user has to quit the app.
Evidence
src/model/transfer.rs:234-244:No
timeout_global,timeout_connect,timeout_recv_response, ortimeout_recv_body.ureqapplies no deadline by default.The cancel check is outside the blocking call:
verify()atsrc/model/transfer.rs:186-196has the same shape, but reads from a local file, so it is not affected.Second, smaller problem on the same line:
.https_only(url.starts_with("https://"))only enforces HTTPS when the URL is already HTTPS, which is a no-op. URLs are built from constants (src/model.rs:258), so nothing is exploitable today, but the guard reads as protection it does not provide. It should be.https_only(true).How ds4 handles this
../ds4gives every socket operation a named timeout constant — it is the house convention, not an afterthought:ds4_web.c:30-31—DS4_WEB_CONNECT_TIMEOUT_MS 3000,DS4_WEB_CDP_TIMEOUT_MS 20000ds4_web.c:169web_tcp_connect(host, port, timeout_ms, ...)andds4_web.c:234web_read_some(fd, buf, len, timeout_ms)— every read goes through apoll()with an explicit deadline;ds4_web.c:486-493turns an expired deadline into a"websocket read timeout"error rather than a hang.ds4_server.c:45—DS4_SERVER_IO_TIMEOUT_SEC 10, applied to every accepted socket atds4_server.c:12498-12503viaSO_RCVTIMEO/SO_SNDTIMEO.Note for honesty: ds4's model downloader is
download_model.sh, which shells out tocurl -fL --progress-meter -C -(download_model.sh:335) and also sets no explicit timeout. But that runs in a terminal where a stalled transfer is visible in the progress meter and killable with Ctrl-C. DS4Server is a GUI where the same stall is invisible and the Cancel button is inert — so the missing timeout is materially worse here than in the reference.Suggested fix
Configure the agent with an overall and a per-read deadline, following ds4's named-constant convention:
Check the
ureq3.x config surface for the body-read stall timeout name before writing it — the crate has renamed these across versions. A stalled read should surface asErr("Model download failed: ..."), which the existing resume logic already handles: the partial file is kept anddownload_url_to_partialwill re-Rangefrom where it stopped on the next attempt (src/model/transfer.rs:233-253).Acceptance criteria
.partfile is intact and a retry resumes from the existing offset (verified by the existingdownload_artifactresume tests insrc/model/transfer.rs:420+)..https_only(true)unconditionally.Fixed in
429cbc7. Model downloads now enforce a 10 s connect timeout plus 30 s response/body stall timeouts, and production downloads are HTTPS-only. Added a loopback regression test proving a stalled transfer times out without damaging the resumable partial file. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.