HarperFast / HarperFast/symphony
Copy-buffer settings do not reach the HTTP/1 header-rewriting path
- Dominant language
- Rust
- Stars
- 1
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
## Problem
The `readBufferSize` / `clientReadBufferSize` / `upstreamReadBufferSize` settings introduced in #36 govern the plain proxying path only. A route that injects HTTP headers — `sourceAddressHeader: 'xForwardedFor'`, or `forwardFingerprint: 'ja3'|'ja4'` under any mode other than `proxyProtocolV2` — takes `http_proxy::proxy_http1_rewriting` instead, which sizes its own reads:
- `read_header_block`: `[0u8; 4096]` (`src/http_proxy.rs:36`)
- `copy_exact` body copies: `[0u8; 8192]` (`src/http_proxy.rs:351`)
- tunnel / read-to-close responses: unsized `tokio::io::copy`
- carry buffers: `Vec::with_capacity(2048)`
So `clientReadBufferSize: 1024` is honored on a PROXY-protocol route and silently ignored on an `xForwardedFor` route on the same proxy.
Impact today is bounded: the new default (8192) equals what the body copies already use, so only a *raised or lowered* value is a no-op, and the primary high-connection-count target (MQTT on 8883) uses PROXY protocol and takes the plain path. #36 narrowed the README and doc comments to say so rather than leaving the proxy-wide claim standing, which is why this is a follow-up rather than a blocker there.
## What to do
Thread the directional sizes through the rewrite pumps, keeping two concerns separate — they are currently conflated by using one constant for both:
- **Parser bounds** (how much header may be buffered before erroring) are a safety limit and should stay pinned to `MAX_HEADER_SIZE` / `MAX_CHUNK_LINE`, *not* follow a user-tunable copy size. A 512-byte `readBufferSize` must not shrink the allowed header block.
- **Copy sizing** (body copies, tunnels, read-to-close) is the memory-vs-syscall trade the config is actually for, and should follow the configured per-direction size.
Note the per-connection memory shape differs on this path: the rewriting pump holds a `carry` Vec plus its read buffers per direction, so the `(client + upstream) × connections` formula from #36 will need restating for these routes once they are tunable.
## Alternative
If threading them through proves to muddy the framing logic, the other honest resolution is to leave these routes fixed and keep the documentation scoped as it is now — in which case close this issue with that rationale recorded. Worth deciding rather than leaving the asymmetry undocumented in the code.
Interacts with #37 (pooling the copy buffers), which will rework the plain path's buffer lifecycle and may make a shared abstraction available to both paths.
Contributor guide
Research direction
Start in src/http_proxy.rs at read_header_block, copy_exact, the tunnel/read-to-close copies, and the carry-buffer allocations. Trace how the directional settings from #36 reach the rewrite pumps, keeping parser bounds separate from copy sizing; also review #37 before choosing an approach. Done means rewrite routes honor configured copy sizes, parser limits remain fixed, and the per-connection memory documentation or rationale is updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, networking
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100