Fix response metadata and upstream authority handling#53
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 45 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Walkthrough本次 PR：将请求 ID 生成改为 UUID v7；引入可配置的 Server 响应头并在响应中注入 Date/Server；重构 HTTP 代理客户端为基于 socket 地址的端点客户端缓存；增加上游 authority 追踪；新增/调整相关配置验证与测试；添加/更新工作区依赖（uuid、httpdate、webpki 等）。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Proxy as Proxy/Server
participant Config as ConfigCompiler
participant Upstream as Upstream
participant ResponseFinalizer as FinalizeResponse
Note over Client,Upstream: 请求流与响应最终化（高层次）
Client->>Proxy: HTTP Request
activate Proxy
Config->>Proxy: 提供 listener.server_header（编译/验证阶段）
Proxy->>Upstream: 转发请求（build_proxy_uri 使用 peer.upstream_authority）
Upstream-->>Proxy: Upstream Response
Proxy->>ResponseFinalizer: 传入 listener.server_header 与响应
ResponseFinalizer->>ResponseFinalizer: 生成 UUIDv7 请求 ID
ResponseFinalizer->>ResponseFinalizer: 计算当前 HTTP Date (httpdate::fmt_http_date)
ResponseFinalizer->>ResponseFinalizer: 插入 Date header
ResponseFinalizer->>ResponseFinalizer: 插入 Server header (来自配置)
ResponseFinalizer->>ResponseFinalizer: 插入 x-request-id header
ResponseFinalizer->>Client: 返回 HTTP Response
deactivate Proxy
预计代码审查工作量🎯 4 (Complex) | ⏱️ ~60 minutes 可能相关的PR
诗歌
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates downstream response metadata handling (request IDs, date/server headers), improves HTTP/3 connection-close logging, and fixes HTTPS/H2 upstream dialing so the logical upstream authority is preserved while still connecting to resolved endpoints.
Changes:
- Switch downstream
x-request-idgeneration to UUIDv7 and update tests accordingly. - Add configurable
server_header(defaultrginx) and injectdate,server, andx-request-idinto downstream responses. - Preserve logical upstream authority in proxy URIs while dialing resolved socket endpoints; add HTTP/3 clean-close handling.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rginx-runtime/src/health.rs | Update tests to populate new Server.server_header. |
| crates/rginx-runtime/src/bootstrap/shutdown.rs | Update tests to populate new Server.server_header. |
| crates/rginx-runtime/src/bootstrap/listeners.rs | Update tests to populate new Server.server_header. |
| crates/rginx-http/src/transition.rs | Update tests to populate new Server.server_header. |
| crates/rginx-http/src/state/tls_runtime/bindings.rs | Update tests to populate new Server.server_header. |
| crates/rginx-http/src/state/tests.rs | Update tests to populate new Server.server_header. |
| crates/rginx-http/src/state/mod.rs | Remove monotonic request-id counter from shared state. |
| crates/rginx-http/src/state/lifecycle.rs | Generate request IDs using UUIDv7. |
| crates/rginx-http/src/server/http3.rs | Treat clean HTTP/3 closes as debug-level and stop warning/error path. |
| crates/rginx-http/src/proxy/tests/request_headers.rs | Add coverage ensuring proxy URI uses logical upstream authority. |
| crates/rginx-http/src/proxy/tests/mod.rs | Update test helper server construction for server_header. |
| crates/rginx-http/src/proxy/mod.rs | Remove unused hyper-rustls ResolveServerName import. |
| crates/rginx-http/src/proxy/health/registry.rs | Update tests for server_header and continue using dial authority for ordering/display. |
| crates/rginx-http/src/proxy/common.rs | Build proxy URI with upstream_authority instead of dial_authority. |
| crates/rginx-http/src/proxy/clients/tests.rs | Update tests to populate new Server.server_header. |
| crates/rginx-http/src/proxy/clients/mod.rs | Dial fixed resolved endpoints while keeping logical authority; cache clients per endpoint socket. |
| crates/rginx-http/src/handler/tests.rs | Update tests for new finalize signature and assert date/server injection. |
| crates/rginx-http/src/handler/dispatch.rs | Inject date/server headers and thread through server_header to final response pipeline. |
| crates/rginx-http/src/client_ip.rs | Update tests to populate new Server.server_header. |
| crates/rginx-http/Cargo.toml | Add uuid dependency for UUIDv7 request IDs. |
| crates/rginx-core/src/lib.rs | Re-export DEFAULT_SERVER_HEADER / default_server_header. |
| crates/rginx-core/src/config/tests.rs | Update tests to set server_header. |
| crates/rginx-core/src/config.rs | Add Server.server_header and default constant/helper. |
| crates/rginx-config/src/validate/tests.rs | Add validation test for empty server_header; update configs for new field. |
| crates/rginx-config/src/validate/server.rs | Validate server_header for legacy server and explicit listeners. |
| crates/rginx-config/src/model.rs | Add server_header to raw config model (server + listener). |
| crates/rginx-config/src/compile/tests.rs | Assert default/custom compiled server_header behavior; update configs for new field. |
| crates/rginx-config/src/compile/server.rs | Compile server_header into runtime HeaderValue with default. |
| crates/rginx-config/src/compile/mod.rs | Pass legacy server’s server_header as default for explicit listeners. |
| crates/rginx-app/tests/upstream_http2.rs | Add integration test ensuring H2 upstream authority is preserved while dialing resolved IP. |
| crates/rginx-app/tests/phase1.rs | Update request-id assertions to UUIDv7 shape. |
| Cargo.toml | Add workspace uuid dependency with v7 feature. |
| Cargo.lock | Lockfile updates for new uuid dependency tree. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b3346206f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/rginx-app/tests/phase1.rs (1)
268-280: LGTM!UUIDv7 字段位置(version nibble 在字符位置 14)和长度断言都正确,而且通过
is_ascii_hexdigit兜底了非连字符的字符校验。可选的小改进:还可以校验 RFC 4122 variant nibble(位置 19 应在
8/9/a/b),让格式断言更严格:♻️ 可选加强
assert_eq!(value.as_bytes()[14], b'7', "generated request id should be UUIDv7"); assert_eq!(value.as_bytes()[18], b'-'); + assert!( + matches!(value.as_bytes()[19], b'8' | b'9' | b'a' | b'b'), + "generated request id should use the RFC 4122 variant, got {value:?}" + ); assert_eq!(value.as_bytes()[23], b'-');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/rginx-app/tests/phase1.rs` around lines 268 - 280, The test helper assert_generated_request_id currently checks UUIDv7 length and version nibble (char pos 14) and hex digits; add an assertion to validate the RFC 4122 variant nibble at character position 19 is one of '8','9','a','b' to tighten the format check. Locate assert_generated_request_id and after the existing checks for position 18 add a single assertion that value.as_bytes()[19] is one of those byte values (or check value.chars().nth(19) against '8','9','a','b') and include a clear failure message like "UUID variant nibble must be 8/9/a/b".crates/rginx-http/src/handler/dispatch.rs (1)
298-301: 建议改用httpdate生成 HTTP-date,避免在响应热路径上的 chrono 格式化开销。
current_http_date()在每个下行响应都调用chrono::Utc::now().format(...).to_string(),产生多次分配并依赖 chrono 的 strftime 引擎。httpdate是 hyper 生态的标准库,专门按 RFC 7231 IMF-fixdate 格式化,无 locale/strftime 开销。虽然 httpdate 已作为传递依赖在依赖图中可用,建议显式声明以避免隐式依赖问题。♻️ 建议改写
fn current_http_date() -> HeaderValue { - let value = chrono::Utc::now().format("%a, %d %b %Y %H:%M:%S GMT").to_string(); + let value = httpdate::fmt_http_date(std::time::SystemTime::now()); HeaderValue::from_str(&value).expect("formatted HTTP date should be a valid header") }在
crates/rginx-http/Cargo.toml的[dependencies]中添加:httpdate = "1.0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/rginx-http/src/handler/dispatch.rs` around lines 298 - 301, current_http_date() currently uses chrono::Utc::now().format(...).to_string(), causing allocation and strftime overhead per response; replace that with httpdate's formatter and add httpdate = "1.0" to crates/rginx-http Cargo.toml dependencies. Change the function to call httpdate::fmt_http_date(std::time::SystemTime::now()) (or the equivalent httpdate API) and pass the resulting string to HeaderValue::from_str(...) instead of using chrono::Utc::now(), keeping the same expect message.crates/rginx-http/src/proxy/clients/mod.rs (2)
367-409: 逐 endpoint 构建 client 让pool_max_idle_per_host的语义发生隐式变化,建议在文档中说明。每个已解析 endpoint 都会得到独立的
HyperProxyClient,因此pool_max_idle_per_host实际上变成 “每个 endpoint × 每个 host” 的空闲上限。当一个上游解析到 N 个 IP 时,空闲连接预算近似变为 N ×pool_max_idle_per_host,与改造前(authority 共用一个 client、N 个 IP 共享同一池)的资源占用模型不同。建议在配置文档或迁移说明里点出这一点,避免现网用户在升级后出现意料之外的连接数上涨。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/rginx-http/src/proxy/clients/mod.rs` around lines 367 - 409, The current implementation of build_hyper_client_for_endpoint constructs a separate HyperProxyClient per resolved endpoint (see function build_hyper_client_for_endpoint and HttpProxyClient.profile), which makes profile.pool_max_idle_per_host apply per endpoint rather than per-authority; document this behavior change (and its capacity impact: effective idle limit ≈ N × pool_max_idle_per_host when an upstream resolves to N IPs) in the configuration/upgrade notes and the doc comment for pool_max_idle_per_host and/or build_hyper_client_for_endpoint, and mention the alternative (sharing a single client per authority) and recommended tuning guidance for pool_max_idle_per_host and pool_max_idle_per_host related settings (pool_idle_timeout, pool_max_idle_per_host) to avoid unexpected connection growth.
270-287:endpoint_clients缓存缺少回收策略,长期运行下可能持续增长。
endpoint_clients仅以HashMap形式缓存,任何被client_for_peer命中的SocketAddr都会保留对应的HyperProxyClient及其连接池。在以下场景会出现问题:
- 上游 DNS 短 TTL 或周期性变更(滚动发布、Kubernetes Service Endpoints 滚动等)持续返回新 IP,旧 IP 对应的 client 永远不会被清理;
- 多个上游(或同一上游不同 peer)随时间累积大量历史 endpoint。
后果是内存与空闲连接(以及关联的 FD)随运行时间线性增长。建议结合
UpstreamResolver的快照,在 endpoint 不再被任何 peer 解析结果引用时主动 evict;或退而求其次给该 cache 设置 LRU/TTL 策略,与pool_idle_timeout对齐。♻️ 思路示例(伪代码,非完整 diff)
// 在每次 resolver 完成解析后,基于当前活跃 endpoint 集合做一次差集清理: fn prune_endpoint_clients(&self, live: &HashSet<SocketAddr>) { let mut guard = self.endpoint_clients .lock() .unwrap_or_else(|p| p.into_inner()); guard.retain(|addr, _| live.contains(addr)); }或者用
lru::LruCache<SocketAddr, HyperProxyClient>替换HashMap,以pool_max_idle_per_host或一个独立上限作为容量。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/rginx-http/src/proxy/clients/mod.rs` around lines 270 - 287, The endpoint_clients HashMap in HttpProxyClient.client_for_peer lacks any eviction, causing unbounded growth; add a pruning/eviction strategy: either replace endpoint_clients with an LRU/TTL cache (e.g. lru::LruCache keyed by SocketAddr with capacity tied to pool_max_idle_per_host) or implement a prune_endpoint_clients(&self, live: &HashSet<SocketAddr>) method that acquires the endpoint_clients lock and retain()s only live addresses after each UpstreamResolver snapshot update; ensure new clients are still created via build_hyper_client_for_endpoint and values remain HyperProxyClient, and align TTL/eviction timing with pool_idle_timeout to avoid keeping idle connections.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Cargo.toml`:
- Line 59: The Cargo.toml dependency for uuid currently enables only the "v7"
feature but uuid::Uuid::now_v7() requires both "v7" and "std"; update the uuid
entry to include features = ["v7", "std"] (or confirm "std" is pulled in
transitively) so Uuid::now_v7() will compile correctly, and if you determine
exposing millisecond timestamps via UUIDv7 (e.g., x-request-id) is unacceptable
for your threat model, switch usages to Uuid::new_v4() instead.
---
Nitpick comments:
In `@crates/rginx-app/tests/phase1.rs`:
- Around line 268-280: The test helper assert_generated_request_id currently
checks UUIDv7 length and version nibble (char pos 14) and hex digits; add an
assertion to validate the RFC 4122 variant nibble at character position 19 is
one of '8','9','a','b' to tighten the format check. Locate
assert_generated_request_id and after the existing checks for position 18 add a
single assertion that value.as_bytes()[19] is one of those byte values (or check
value.chars().nth(19) against '8','9','a','b') and include a clear failure
message like "UUID variant nibble must be 8/9/a/b".
In `@crates/rginx-http/src/handler/dispatch.rs`:
- Around line 298-301: current_http_date() currently uses
chrono::Utc::now().format(...).to_string(), causing allocation and strftime
overhead per response; replace that with httpdate's formatter and add httpdate =
"1.0" to crates/rginx-http Cargo.toml dependencies. Change the function to call
httpdate::fmt_http_date(std::time::SystemTime::now()) (or the equivalent
httpdate API) and pass the resulting string to HeaderValue::from_str(...)
instead of using chrono::Utc::now(), keeping the same expect message.
In `@crates/rginx-http/src/proxy/clients/mod.rs`:
- Around line 367-409: The current implementation of
build_hyper_client_for_endpoint constructs a separate HyperProxyClient per
resolved endpoint (see function build_hyper_client_for_endpoint and
HttpProxyClient.profile), which makes profile.pool_max_idle_per_host apply per
endpoint rather than per-authority; document this behavior change (and its
capacity impact: effective idle limit ≈ N × pool_max_idle_per_host when an
upstream resolves to N IPs) in the configuration/upgrade notes and the doc
comment for pool_max_idle_per_host and/or build_hyper_client_for_endpoint, and
mention the alternative (sharing a single client per authority) and recommended
tuning guidance for pool_max_idle_per_host and pool_max_idle_per_host related
settings (pool_idle_timeout, pool_max_idle_per_host) to avoid unexpected
connection growth.
- Around line 270-287: The endpoint_clients HashMap in
HttpProxyClient.client_for_peer lacks any eviction, causing unbounded growth;
add a pruning/eviction strategy: either replace endpoint_clients with an LRU/TTL
cache (e.g. lru::LruCache keyed by SocketAddr with capacity tied to
pool_max_idle_per_host) or implement a prune_endpoint_clients(&self, live:
&HashSet<SocketAddr>) method that acquires the endpoint_clients lock and
retain()s only live addresses after each UpstreamResolver snapshot update;
ensure new clients are still created via build_hyper_client_for_endpoint and
values remain HyperProxyClient, and align TTL/eviction timing with
pool_idle_timeout to avoid keeping idle connections.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: b731a825-560d-4f01-9a68-a4bd04470b49
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
Cargo.tomlcrates/rginx-app/tests/phase1.rscrates/rginx-app/tests/upstream_http2.rscrates/rginx-config/src/compile/mod.rscrates/rginx-config/src/compile/server.rscrates/rginx-config/src/compile/tests.rscrates/rginx-config/src/model.rscrates/rginx-config/src/validate/server.rscrates/rginx-config/src/validate/tests.rscrates/rginx-core/src/config.rscrates/rginx-core/src/config/tests.rscrates/rginx-core/src/lib.rscrates/rginx-http/Cargo.tomlcrates/rginx-http/src/client_ip.rscrates/rginx-http/src/handler/dispatch.rscrates/rginx-http/src/handler/tests.rscrates/rginx-http/src/proxy/clients/mod.rscrates/rginx-http/src/proxy/clients/tests.rscrates/rginx-http/src/proxy/common.rscrates/rginx-http/src/proxy/health/registry.rscrates/rginx-http/src/proxy/mod.rscrates/rginx-http/src/proxy/tests/mod.rscrates/rginx-http/src/proxy/tests/request_headers.rscrates/rginx-http/src/server/http3.rscrates/rginx-http/src/state/lifecycle.rscrates/rginx-http/src/state/mod.rscrates/rginx-http/src/state/tests.rscrates/rginx-http/src/state/tls_runtime/bindings.rscrates/rginx-http/src/transition.rscrates/rginx-runtime/src/bootstrap/listeners.rscrates/rginx-runtime/src/bootstrap/shutdown.rscrates/rginx-runtime/src/health.rs
💤 Files with no reviewable changes (1)
- crates/rginx-http/src/state/mod.rs
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/rginx-app/tests/phase1.rs (1)
268-284: 测试断言与 UUIDv7 规范一致，LGTM。
assert_generated_request_id的校验顺序合理：先断言长度为 36，再用字节索引访问位置 8/13/18/23 的连字符以及版本/变体半字节，避免越界；版本位固定为'7'，变体位限制在8|9|a|b，并对去除连字符后的字符做十六进制校验，与uuid::Uuid::now_v7()的小写规范输出相符。一个可选的小改进（非阻塞）：变体半字节校验只接受小写
a|b，依赖于uuidcrate 默认的小写格式；如果将来打印格式发生变化（例如改为to_string_upper或Hyphenated显式大写），该断言会失败。可考虑在比较前对该字节做to_ascii_lowercase()，让测试对大小写更鲁棒：♻️ 可选的鲁棒性增强
- assert!( - matches!(value.as_bytes()[19], b'8' | b'9' | b'a' | b'b'), - "generated request id should use the RFC 4122 variant, got {value:?}" - ); + assert!( + matches!( + value.as_bytes()[19].to_ascii_lowercase(), + b'8' | b'9' | b'a' | b'b' + ), + "generated request id should use the RFC 4122 variant, got {value:?}" + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/rginx-app/tests/phase1.rs` around lines 268 - 284, The test's assert_generated_request_id currently checks the variant nibble byte against lowercase b'8'|b'9'|b'a'|b'b', which will fail if UUID hex letters are uppercase; update the check in assert_generated_request_id to normalize that byte to lowercase (e.g., call to_ascii_lowercase on the byte or otherwise map A/B to a/b) before matching, so the variant assertion accepts both cases while keeping the rest of the UUIDv7 checks unchanged.crates/rginx-http/src/handler/dispatch.rs (1)
298-301: 可选优化：按秒缓存格式化后的 HTTP-date。每个响应都会执行一次
SystemTime::now()+httpdate::fmt_http_date()+HeaderValue::from_str。HTTP-date 精度只有秒级，在高 QPS 场景下可以把结果缓存到ArcSwap<HeaderValue>或OnceLock，由后台任务（或AtomicU64时间戳惰性比较）每秒刷新一次，能省掉热路径上的格式化与from_str校验。这是一个在 Hyper 等高性能 HTTP 服务器中已验证的模式。当前实现功能正确，仅作为后续可选改进。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/rginx-http/src/handler/dispatch.rs` around lines 298 - 301, The current_http_date function formats a new HTTP-date on every call (calling SystemTime::now, httpdate::fmt_http_date and HeaderValue::from_str) which is wasteful at high QPS; change it to return a cached HeaderValue updated at most once per second: introduce a global cached container (e.g., ArcSwap<HeaderValue> or OnceLock<(AtomicU64, HeaderValue)>) and a background/periodic update mechanism or a lazy compare-and-swap that checks the current epoch seconds and only recomputes and replaces the HeaderValue when the seconds change; update all callers of current_http_date to read from the cache and ensure the replacement uses HeaderValue::from_str once per second and handles any parse errors on refresh (not on hot path) while keeping function signature current_http_date() -> HeaderValue semantics by cloning or returning an Arc/clone of the cached HeaderValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/rginx-http/src/handler/dispatch.rs`:
- Line 289: 当前代码在 dispatch.rs 里无条件用
response.headers_mut().insert(http::header::DATE, current_http_date()) 覆盖上游的
Date 头，会影响缓存协商；请修改为仅在响应未包含 Date 头时才填充（检查
response.headers().contains_key(http::header::DATE) 或等价方法），保留现有
current_http_date() 生成逻辑并只在缺失时调用/插入，以避免覆盖上游时间戳。
In `@crates/rginx-http/src/server/http3.rs`:
- Around line 796-810: The current string-based fallback in
is_clean_http3_accept_close is brittle; add a unit test that constructs a real
h3::error::ConnectionError coming from a Quinn peer closing with
ApplicationClose code 0 and assert is_clean_http3_accept_close(&error) returns
true so future display-format changes in h3/quinn break the test instead of
reintroducing noisy warnings; locate is_clean_http3_accept_close in http3.rs and
add a test (e.g., in the same module or tests mod) that performs a minimal
Quinn/h3 handshake or simulates the peer ApplicationClose(0) path to produce the
ConnectionError and pass it to is_clean_http3_accept_close.
---
Nitpick comments:
In `@crates/rginx-app/tests/phase1.rs`:
- Around line 268-284: The test's assert_generated_request_id currently checks
the variant nibble byte against lowercase b'8'|b'9'|b'a'|b'b', which will fail
if UUID hex letters are uppercase; update the check in
assert_generated_request_id to normalize that byte to lowercase (e.g., call
to_ascii_lowercase on the byte or otherwise map A/B to a/b) before matching, so
the variant assertion accepts both cases while keeping the rest of the UUIDv7
checks unchanged.
In `@crates/rginx-http/src/handler/dispatch.rs`:
- Around line 298-301: The current_http_date function formats a new HTTP-date on
every call (calling SystemTime::now, httpdate::fmt_http_date and
HeaderValue::from_str) which is wasteful at high QPS; change it to return a
cached HeaderValue updated at most once per second: introduce a global cached
container (e.g., ArcSwap<HeaderValue> or OnceLock<(AtomicU64, HeaderValue)>) and
a background/periodic update mechanism or a lazy compare-and-swap that checks
the current epoch seconds and only recomputes and replaces the HeaderValue when
the seconds change; update all callers of current_http_date to read from the
cache and ensure the replacement uses HeaderValue::from_str once per second and
handles any parse errors on refresh (not on hot path) while keeping function
signature current_http_date() -> HeaderValue semantics by cloning or returning
an Arc/clone of the cached HeaderValue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 497eb328-e715-4cda-aab1-9d2cdf142fca
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlRELEASE_NOTES_v0.1.3-rc.13.mdcrates/rginx-app/tests/phase1.rscrates/rginx-http/Cargo.tomlcrates/rginx-http/src/handler/dispatch.rscrates/rginx-http/src/proxy/clients/mod.rscrates/rginx-http/src/proxy/clients/tests.rscrates/rginx-http/src/server/http3.rsdocs/HTTP3_PHASE7_RELEASE.mddocs/README.md
💤 Files with no reviewable changes (1)
- docs/HTTP3_PHASE7_RELEASE.md
✅ Files skipped from review due to trivial changes (2)
- RELEASE_NOTES_v0.1.3-rc.13.md
- docs/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/rginx-http/Cargo.toml
- Cargo.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
x-request-idvalues with UUIDv7 instead of monotonic countersserver_headerand emitdate,server, andx-request-idon downstream responsesApplicationClose: 0x0as debug-level connection closureVerification
cargo fmt --allcargo check --workspace --all-targets --message-format shortcargo test -p rginx-config server_header -- --nocapturecargo test -p rginx-http finalize_downstream_response -- --nocapturecargo test -p rginx --test phase1 return_responses_generate_and_preserve_request_id_headers -- --nocapturecargo test -p rginx --test http3 serves_return_handler_over_http3 -- --nocapturecargo test -p rginx --test upgrade -- --nocapture./scripts/test-fast.sh./scripts/run-tls-gate.sh./scripts/run-http3-gate.shDeployment Smoke Test
162.141.130.176/healthzreturn 200 withdate,server: rginx, and UUIDv7x-request-idws://andwss://with 101 handshake and echo-frame round triphttp3 request accept failedWARN