Skip to content

Harden HTTP request body and upload handling - #1848

Open
Distortions81 wants to merge 4 commits into
bitaxeorg:masterfrom
Distortions81:fix/api-request-hardening
Open

Harden HTTP request body and upload handling#1848
Distortions81 wants to merge 4 commits into
bitaxeorg:masterfrom
Distortions81:fix/api-request-hardening

Conversation

@Distortions81

@Distortions81 Distortions81 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Harden JSON reads, query parsing, and firmware/frontend uploads against oversized, fragmented, or stalled input.

  • Read bounded bodies completely with size_t lengths and a total deadline.
  • Strictly parse whole JSON documents and bound statistics queries.
  • Reject empty or partition-oversized uploads before writing.
  • Enforce total/no-progress upload deadlines and cleanly abort failures.
  • Keep upload lengths and progress arithmetic size-safe.

Why

Several handlers read only once or narrowed peer-controlled lengths before checking them. Upload loops also lacked total/stall deadlines. Malformed requests could therefore become partial JSON, unsafe size conversions, or indefinitely occupied handlers.

This keeps the trusted-network model and adds no authentication.

Review follow-up

Incorporates @johnny9's requested production-backed boundary coverage. Small pure size/read/deadline helpers now drive the real HTTP paths and are tested at exact accepted/rejected boundaries.

Stack and related work

Builds cumulatively on #1846 and #1847. Related to #1845, while additionally covering statistics bounds and OTA/AxeOS upload limits and deadlines.

Validation

  • ESP-IDF 6.0.2 full firmware and ESP32-S3 test images built
  • Exact CI QEMU environment: 83 passed, 0 failed
  • AxeOS production build passed with Node.js 22
  • git diff --check

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Test Results

  2 files  ±0    2 suites  ±0   1s ⏱️ ±0s
120 tests ±0  120 ✅ ±0  0 💤 ±0  0 ❌ ±0 
122 runs  ±0  122 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 0db611e. ± Comparison against base commit c32b52d.

♻️ This comment has been updated with latest results.

@johnny9 johnny9 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 94c54a4. I did not find a correctness defect in the changed receive/upload implementation. The remaining issue is that the advertised api_rx tests do not exercise the new HTTP body and upload deadline decisions. The production-backed coverage patch passes 81/81 ESP32-S3 QEMU tests and a fresh full ESP-IDF 5.5.3 firmware build.

Comment thread main/http_server/http_server.c Outdated
@Distortions81
Distortions81 marked this pull request as ready for review August 7, 2026 22:30
@johnny9

johnny9 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Physical Bitaxe 602 smoke test passed on head 02d4154a7e882dffede6e2ac7d074d6ef0288222.

The application-only OTA reported the expected firmware version, the device returned to healthy mining with three stable API samples, zero fault indication, and the configured pool intact. An independent authorized Stratum V1 probe received a fresh mining.notify. The 602 has no separate bridge firmware, so no bridge image was applied.

Full test result and artifacts: https://mining-qa-status.vercel.app/results/55df79e5-1015-423e-a2c1-d9400e605403

Test harness: johnny9/miner-testcode@b3694f6b01617baa8b71ed84c28300baf4cdab61.

@Distortions81
Distortions81 force-pushed the fix/api-request-hardening branch 2 times, most recently from b4b747f to 0db611e Compare August 8, 2026 02:04
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.

2 participants