HarperFast / HarperFast/symphony

Copy-buffer settings do not reach the HTTP/1 header-rewriting path

Open
#39 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.