upload: follow a redirect, take an early answer, and give up on a frozen upload (#232)

[openipc-ai]

From a network that cuts connections to openipc.org after their first
kilobytes, ipctool upload with a backup sat at "Sending 0%" for
minutes, and through another --host it reported "openipc.org answered
502" for an answer openipc.org never gave.

• After the headers, wait up to 2 s for an answer before sending the
  body. A server that redirects or refuses says so at once, and the
  answer to the headers gets back where a body would not.
• Follow a 307/308 to a plain http:// address, up to three hops, with
  the same body. openipc.org can then send a cut-off network's report
  where it gets through, and nothing in ipctool names that place.
• A send that cannot place a byte for 30 s is a stall: wait 10 s for an
  answer, then say the upload stopped moving and that --host can send it
  another way, instead of "Could not reach" after several minutes.
  Waited through poll(), since SO_SNDTIMEO restarts whenever a few bytes
  squeeze in.
• An answer with no explanation no longer names openipc.org.

Fixes #232.

Tested natively: report_test (new cases for the redirect parser and the message), and http_post driven against local servers. A 16 MB body to a server that answers 307 on the headers: no body is sent there, it is followed, and the second server gets all 16,777,216 bytes, 2.0 s in total. Against a server that never reads: ERR_STALLED after 42 s (2 + 30 + 10), where it used to sit for minutes. The cross builds are left to CI.

The website side (openipc.org's port 80 answering a cut-off network's report with a 307) is OpenIPC/website, to install once this is in the published ipctool.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Handle upload redirects, early responses, and stalled sends

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Read early responses before sending backup data, avoiding unnecessary uploads on rejection or
 redirect.
• Replay the multipart report across up to three HTTP 307/308 redirects.
• Detect stalled sends, offer --host guidance, and avoid misattributing unexplained server errors.

Diagram

graph TD
  A["Upload command"] --> B["HTTP POST"] --> C{"Early answer?"}
  C -->|redirect| D["Redirect parser"] -->|valid, under limit| B
  C -->|none| E["Send body"] --> F{"Send fails?"} -->|yes| G["Late answer"] --> H["Report outcome"]
  F -->|no| H
  C -->|other response| H
  C -->|hop limit| H
  D -->|invalid| H

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use HTTP/1.1 Expect: 100-continue

• ➕ Could avoid the fixed two-second wait when a server promptly approves the body.
• ➕ Uses a standard mechanism for deciding whether to send a large request body.
• ➖ Requires interim-response handling and compatible behavior from upload servers.
• ➖ Adds protocol complexity to the existing HTTP/1.0 client.

Recommendation: The bounded early-response wait is a pragmatic fit for the current HTTP/1.0 client and the stated cut-off-network failure. Keep the bounded redirect and stall handling; consider Expect: 100-continue only if the two-second delay on ordinary uploads becomes significant.

Files changed (6) +202 / -16

Enhancement (2) +14 / -2

http.hExpose redirect locations and stalled-send errors +7/-2

Expose redirect locations and stalled-send errors

• Extends the POST interface with a Location output buffer and documents early-response behavior. Adds ERR_STALLED for a body that stops moving without a server answer.

src/http.h

report.hDeclare the report redirect parser +7/-0

Declare the report redirect parser

• Publishes the redirect parser and documents which destinations the upload client can follow.

src/report.h

Bug fix (3) +150 / -14

http.cRead early responses and bound stalled body sends +79/-6

Read early responses and bound stalled body sends

• Waits up to two seconds for an answer after sending headers, and extracts a Location header from valid responses. Uses poll before nonblocking sends to detect a 30-second lack of progress, then allows ten seconds for a server explanation.

src/http.c

report.cParse supported redirect destinations without blaming a fixed host +35/-2

Parse supported redirect destinations without blaming a fixed host

• Adds parsing for absolute plain-HTTP redirect URLs with an optional port, rejecting unsupported schemes and malformed destinations. Removes the hard-coded openipc.org attribution from unexplained non-JSON failures.

src/report.c

report_cmd.cFollow bounded upload redirects and explain stalls +36/-6

Follow bounded upload redirects and explain stalls

• Retries the same multipart report across up to three supported 307/308 redirects. Reports stalled uploads with --host guidance and uses the current destination in error messages.

src/report_cmd.c

Tests (1) +38 / -0

report_test.cCover redirect parsing and host-neutral errors +38/-0

Cover redirect parsing and host-neutral errors

• Adds cases for supported redirect URLs, rejected schemes and malformed destinations. Checks that an unexplained non-JSON failure does not attribute the response to openipc.org.

src/report_test.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Redirects and stalls lack automated tests ✓ Resolved

Description

The new http_post() early-answer and stall paths have no automated test that calls them.
report_test covers redirect parsing and message text, but its test target does not compile
http.c, so it cannot catch failures in response timing or a stalled 16 MB upload.

Code

src/http.c[R233-239]

+    struct pollfd early = {.fd = s, .events = POLLIN};
+    int ready;
+    do
+        ready = poll(&early, 1, EARLY_ANSWER_MS);
+    while (ready < 0 && errno == EINTR);
+    if (ready > 0)
+        goto answer;

Evidence

Rule 11 requires effective hardware-free coverage of affected behavior. The changed upload paths are
in http.c, while the existing report_test target compiles only the parser and body-building
sources; repository references to http_post() contain no test caller.

CLAUDE.md: Keep Hardware-Free Tests Effective: CLAUDE.md: Keep Hardware-Free Tests Effective: CLAUDE.md: Keep Hardware-Free Tests Effective: CLAUDE.md: Keep Hardware-Free Tests Effective: CLAUDE.md: Keep Hardware-Free Tests Effective: CLAUDE.md: Keep Hardware-Free Tests Effective: CLAUDE.md: Keep Hardware-Free Tests Effective: CLAUDE.md: Keep Hardware-Free Tests Effective
src/http.c[233-239]
src/http.c[248-257]
src/report_test.c[124-156]
CMakeLists.txt[497-502]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new early-answer, redirect, and stall behavior is exercised manually but not by a hardware-free automated test.
## Fix Focus Areas
- src/http.c[233-239]
- src/http.c[248-257]
- CMakeLists.txt[497-502]
## Recommended Fix
Add a hardware-free test target that calls the upload path against local servers. Verify an early 307 is followed without sending its body, the destination receives the complete body, and a non-reading server produces the stall error.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Private backups can reach other servers ✓ Resolved

Description

report_redirect() accepts any HTTP hostname, and report_cmd() sends the same multipart body to
that hostname without checking whether the owner approved it. When a redirect changes hosts, the
followed request can carry a private flash backup containing passwords and keys to a server other
than the one named before consent.

Code

src/report_cmd.c[R285-290]

+        if (!report_redirect(location, next, sizeof(next), &nport, path,
+                             sizeof(path)))
+            break;
+        fprintf(stderr, "%s sends the report to %s.\n", hostname, location);
+        snprintf(hostname, sizeof(hostname), "%s", next);
+        port = nport;

Evidence

The command describes the initial destination and says a private backup is limited to OpenIPC
maintainers, but the new loop accepts an unrestricted redirect hostname and replays the existing
backup spans to it.

src/report_cmd.c[148-155]
src/report_cmd.c[169-186]
src/report.c[121-150]
src/report_cmd.c[277-291]
src/http.c[242-260]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A cross-host redirect can send a private flash backup to a destination the owner did not approve.
## Fix Focus Areas
- src/report_cmd.c[278-291]
- src/report.c[121-150]
## Recommended Fix
Before replaying a backup to a different host, require explicit approval of that destination or restrict it to trusted destinations; fail closed when approval is unavailable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Interim replies block report uploads ✓ Resolved

Description

http_post() treats any readability after sending headers as a final answer and jumps past its only
body-send loop. If a server sends 100 Continue before waiting for the body, the client instead
waits for the server to finish responding and never supplies the bytes it needs.

Code

src/http.c[R233-239]

+    struct pollfd early = {.fd = s, .events = POLLIN};
+    int ready;
+    do
+        ready = poll(&early, 1, EARLY_ANSWER_MS);
+    while (ready < 0 && errno == EINTR);
+    if (ready > 0)
+        goto answer;

Evidence

The new poll jumps straight to response reception on any readable event. That bypasses the body
loop, while the receive loop waits for EOF or a timeout and has no handling for an interim status.

src/http.c[233-255]
src/http.c[276-295]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The pre-body poll skips sending the upload when the first readable response is interim.
## Fix Focus Areas
- src/http.c[233-239]
- src/http.c[276-295]
## Recommended Fix
Read and classify an early response before deciding to skip the body. Consume interim responses and continue sending; stop only for a complete final response.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Redirects with bare queries fail ✓ Resolved

Description

report_redirect() looks only for / when separating the hostname from the rest of a URL, so it
includes ?query in the hostname when no slash precedes it. A valid location such as
http://mirror.example?token=1 is then passed to DNS as mirror.example?token=1 rather than
followed with /?token=1.

Code

src/report.c[R126-129]

+    const char *h = location + sizeof(scheme) - 1;
+    const char *p = strchr(h, '/');
+    if (!p)
+        p = h + strlen(h);

Evidence

The parser searches only for a slash and copies everything before it into host; the redirect loop
uses that value as the next connection hostname, which the DNS resolver attempts to resolve.

src/report.c[126-150]
src/report_cmd.c[278-291]
src/http.c[90-102]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A redirect URL with a query but no slash is parsed with the query inside the hostname.
## Fix Focus Areas
- src/report.c[126-150]
- src/report_test.c[124-156]
## Recommended Fix
End the authority at a slash, question mark, or fragment marker. Construct a leading-slash request path for a bare query and add a regression test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗