test(UnitTest): handle early local HTTP requests

[alireza787b]

Description

LocalHttpTestServer could respond to the first readyRead notification even when it contained only part of an HTTP request header. That made camera-free transport tests timing-dependent.

The responder now accumulates a bounded request header across notifications and sends exactly one response after \r\n\r\n arrives. Oversized headers latch a terminal state before disconnecting. An immediate read after installing the socket callbacks remains as defensive handling for already-buffered data, but fragmented headers are the regression fixed here.

The regression test splits the header across two writes, verifies that the first fragment receives no response, and connects through server.url() so IPv4 and IPv6 fixture binds follow the same contract.

This changes test infrastructure only; application networking is unchanged.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Refactoring
• CI/Build changes
• Other: test fixture reliability

Testing

• Tested locally
• Added/updated unit tests
• Tested with simulator (not applicable)
• Tested with hardware (not applicable)

Local checks on exact head 67caa46a0cdcb6f3cf4a9c232956dd9a074bdc45:

• a standalone Qt 6.4.2 network probe verified no response to the incomplete fragment and a complete response after the terminator;
• git diff --check.

Exact-head QGC CI is complete: Linux x64/arm64 release builds, unit and integration tests, ASan/UBSan, coverage thresholds, Windows, macOS, iOS, Android, Docker, custom-plugin, and CodeQL checks passed. Linux evidence: run 30596338957.

Platforms Tested

• Linux
• Windows (QGC exact-head CI build)
• macOS (QGC exact-head CI build)
• Android (QGC exact-head CI build)
• iOS (QGC exact-head CI build)

Flight Stacks Tested

Not applicable; this is a unit-test fixture.

Screenshots

Not applicable; there is no UI change.

Checklist

• I have read the Contribution Guidelines
• I have read the Code of Conduct
• My code follows the project coding standards
• I have added a regression test for the fixture behavior
• New and existing unit and integration tests pass at the exact head in QGC CI

Related Issues

#14730 carries this as a test-infrastructure prerequisite because its camera-free MJPEG delivery test uses installRawResponder.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 33.28%. Comparing base (f29efd3) to head (d970595).
⚠️ Report is 265 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #14729      +/-   ##
==========================================
+ Coverage   25.47%   33.28%   +7.80%     
==========================================
  Files         769      919     +150     
  Lines       65912    82944   +17032     
  Branches    30495    38312    +7817     
==========================================
+ Hits        16788    27604   +10816     
+ Misses      37285    36616     -669     
- Partials    11839    18724    +6885     

Flag	Coverage Δ
unittests	33.28% <ø> (+7.80%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 625 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed09a05...d970595. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Build Results

Platform Status

Platform	Status	Details
Linux	Passed	View
Windows	Passed	View
MacOS	Passed	View
Android	Passed	View

All builds passed.

Pre-commit

Check	Status	Details
pre-commit	Failed (non-blocking)	View

Pre-commit hooks: 2 passed, 75 failed, 7 skipped.

Test Results

linux-coverage-integration: 41 passed, 0 skipped
linux-coverage-unit: 126 passed, 0 skipped
linux-sanitizers-integration: 23 passed, 0 skipped
linux-sanitizers-unit: 126 passed, 0 skipped
Total: 316 passed, 0 skipped

Code Coverage

Coverage	Baseline	Change
68.6%	68.6%	+0.0%

Artifact Sizes

Artifact	Size	Δ from master
QGroundControl	252.76 MB	-4.45 MB (decrease)
QGroundControl-aarch64	179.48 MB	No change
QGroundControl-installer-AMD64	137.89 MB	-0.45 MB (decrease)
QGroundControl-installer-AMD64-ARM64	80.41 MB	-0.21 MB (decrease)
QGroundControl-installer-ARM64	109.17 MB	+0.00 MB (increase)
QGroundControl-linux	87.14 MB	-78.83 MB (decrease)
QGroundControl-mac	87.14 MB	No change
QGroundControl-x86_64	191.71 MB	-0.00 MB (decrease)
Total size decreased by 83.95 MB

---

_{Updated: 2026-08-10 05:38:03 UTC • Commit: d970595 • Triggered by: Windows}

[Copilot]

Pull request overview

This PR improves the reliability of the TestFixtures::LocalHttpTestServer test infrastructure by ensuring already-buffered and fragmented HTTP request headers are fully accumulated before a response is sent, and adds a regression unit test covering the early/fragmented request scenario.

Changes:

• Adds LocalHttpTestServerTest to validate that the server does not respond to an incomplete HTTP request header fragment.
• Updates LocalHttpTestServer::installRawResponder to consume already-buffered bytes and wait for \r\n\r\n before responding.
• Registers the new unit test in the UnitTestFramework Tests CMake target.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
test/UnitTestFramework/Tests/LocalHttpTestServerTest.h	Adds a new UnitTest class for LocalHttpTestServer regression coverage.
test/UnitTestFramework/Tests/LocalHttpTestServerTest.cc	Implements a regression test that splits an HTTP header across writes and asserts no early response.
test/UnitTestFramework/Tests/CMakeLists.txt	Adds the new test sources and registers LocalHttpTestServerTest.
test/UnitTestFramework/Fixtures/LocalHttpTestServer.cc	Updates the raw responder to accumulate request data and respond only after the header terminator is received.

[DonLakeFlyer]

Review feedback from a Claude Fable review:

Worth changing

1. Drop the unrelated reformatting hunks — the httpReasonPhrase switch re-indent, the installHttpResponder string-literal re-wrap, and the } // namespace → } // namespace spacing changes touch code the fix doesn't. They double the diff for zero behavior change and obscure the real fix.
2. Latch the terminal state on the oversized-header path — set state->responseSent = true before disconnectFromHost() when MAX_REQUEST_HEADER_SIZE is exceeded, so later readyRead notifications don't keep appending to state->request while the disconnect is in flight. This also addresses the substance of the existing size-bound review comment without the complexity of a budgeted read, which seems unnecessary for a loopback test fixture.
3. Restore alphabetical ordering in test/UnitTestFramework/Tests/CMakeLists.txt — LocalHttpTestServerTest is inserted after MultiSignalSpyTest in both target_sources and the add_qgc_test list.
4. Include order in LocalHttpTestServerTest.cc — sibling tests order own header → project headers → Qt headers; the new file puts Qt headers before Fixtures/LocalHttpTestServer.h.

Optional

5. PR description overstates the first bug — the "request bytes buffered before the one-shot readyRead connection was installed" gap is essentially theoretical: the connect happens synchronously inside the newConnection handler before the event loop spins again, and Qt's read notifier is level-triggered, so already-buffered data still produces a readyRead. The partial-header race is the real bug; the immediate sendResponseWhenRequestComplete() call is fine as defensive hardening, but the description should reflect that.
6. Merge the two anonymous namespaces in LocalHttpTestServer.cc — the new RawResponderState/MAX_REQUEST_HEADER_SIZE block adds a second anonymous namespace while httpReasonPhrase keeps its own mid-file.

For the record: the core fix is correct and well-tested — the fragmented-header regression test genuinely fails against the old single-shot responder, and the shared-state/lifetime handling (QSharedPointer state, socket-context connection, deleteLater cleanup) is sound.

[DonLakeFlyer]

Claude Fable review — assessment of the two Copilot inline comments:

1. IPv4/IPv6 bind mismatch in LocalHttpTestServerTest.cc — agree, should be fixed. The fixture explicitly supports falling back to an IPv6-only bind (listen() tries LocalHostIPv6 second) and url() already handles that case, so a test hardcoding QHostAddress::LocalHost contradicts the fixture's own contract. It will pass in today's CI (IPv4 loopback essentially always binds), but it's a latent spurious failure on IPv6-only configurations, and the fix is one line: derive the host from server.url() instead of hardcoding the address.

2. Size bound enforced after append(readAll()) — technically correct observation, but the suggested budgeted read is over-engineered for this fixture. The worst case is a single transient over-allocation bounded by the kernel socket buffer (a few hundred KB), freed as soon as the socket closes; the bound's real job — keeping a terminator-less test from accumulating forever — is already achieved, just one readyRead late. Reading "at most the remaining header budget +1 byte" and truncating to the terminator adds branching that itself needs testing, which isn't warranted in a loopback test fixture. The minimal fix worth taking is to latch the terminal state on the overflow path (state->responseSent = true before disconnectFromHost()), which stops further accumulation while the disconnect is in flight. I'd decline the rest of the suggestion.

[alireza787b]

Thanks. The review points are addressed in 67caa46a0.

• Restored the unrelated switch, string-literal, and namespace-comment formatting.
• Latched the terminal state before disconnecting an oversized request.
• Restored alphabetical CMake ordering and project-before-Qt include ordering.
• Connected the regression client through server.url(), so it follows IPv4 or IPv6 fixture binding.
• Merged the anonymous namespaces.
• Renamed the test and revised the PR description to center the real fragmented-header regression; the immediate read is described only as defensive handling.

For the size bound, I followed your recommendation: terminal-state latching without adding the budgeted-read branches to this loopback fixture. A standalone Qt 6.4.2 fragmented-header probe passed at the pushed head; QGC CI is running.

[DonLakeFlyer]

All changes look good thanks. Kicking off another final CoPilot review...

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[DonLakeFlyer]

@alireza787b Ok, this is good to go now. Can you squash to single commit and then once last CI passes I'll merge.

[alireza787b]

Squashed the branch to the single commit requested: d970595.

The new commit has the exact same tree as reviewed head 67caa46; only the commit history changed. Fresh CI is now running.