Repository navigation
feat(middleware): add shared Redis rate limiting - #453
Merged
Merged
Conversation
…lly overflowing call to snprintf' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
gg582
marked this pull request as ready for review
October 26, 2026 13:37
Preserve the upstream Autofix commit and supersede its void encoder with the independently reviewed checked implementation. Reject formatting failures and capacity mismatches before acquiring a connection; cover bounds and formatter failures without suppressing CodeQL.
Conflict: docs/api/v4.0-api-baseline.txt, where dev's lwIP adapter API (#452) and this branch's Redis rate-limit API land in the same sections. Regenerated with `make api-baseline` from the merged headers: against dev it adds only this PR's entries, against the branch only dev's.
Member
|
yo, LGTM. |
gg582
added a commit
that referenced
this pull request
Oct 27, 2026
…in (#454) * feat(port): serve a CWIST app over lwIP's raw TCP API (#447 Phase 3) (#452) * feat(port): serve a CWIST app over lwIP's raw TCP API (#447 Phase 3) - include/cwist/sys/port/lwip.h + src/sys/port/lwip.c: a listener on a raw-API tcp_pcb (NO_SYS-safe). Requests are framed by Content-Length with the parser's CL/TE rules (501 for Transfer-Encoding, 400 for CL+TE or a bad/conflicting CL, 431/413 above the configured caps), answered with 100 Continue on Expect, parsed and run through cwist_app_dispatch, and serialized. Keep-alive follows request and response as on the socket path; pipelined requests are answered in order. Responses go out zero-copy via tcp_write and are freed once acknowledged; while one is in flight, excess input is refused back to lwIP so the window throttles the peer. Connection cap, idle timeout via tcp_poll, HEAD without body, SOF_REUSEADDR when SO_REUSE is on. - The adapter is compiled by the firmware next to its own lwIP and lwipopts.h; it uses only public CWIST API and is not part of libcwist. - lib/lwip: lwIP STABLE-2_2_1_RELEASE, vendored for the tests only. - tests/test_lwip_adapter.c (make test): real lwIP TCP over the loopback netif in one process, covering keep-alive, pipelining, split bodies, 100-continue, every framing error, Connection: close, HTTP/1.0, HEAD, a 20 KB response through a 2-segment send window, the connection cap, the idle timeout and teardown. Clean under ASan/UBSan/LSan. - tests/baremetal/hello_lwip.c joins make baremetal-smoke: the same stack on Cortex-M7 under qemu-arm. * fix(port): keep cppcheck off the lwIP option #error cppcheck analyzes src/ without the firmware's lwIP tree, so LWIP_TCP was undefined and the requirement check fired. lwip/opt.h always defines LWIP_TCP and LWIP_CALLBACK_API, so check only once it has been seen; a real build with LWIP_TCP=0 still hits the #error. (cherry picked from commit ad7ecad) * feat(middleware): add shared Redis rate limiting (#453) * feat(middleware): add shared Redis rate limiting * test(redis): fix GCC buffer and pipe declarations * fix(redis): Potential fix for pull request finding 'CodeQL / Potentially overflowing call to snprintf' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * fix(redis): validate frame encoding before pooled I/O Preserve the upstream Autofix commit and supersede its void encoder with the independently reviewed checked implementation. Reject formatting failures and capacity mismatches before acquiring a connection; cover bounds and formatter failures without suppressing CodeQL. --------- Co-authored-by: 이윤진(Lee Yunjin) <168180007+gg582@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Lee Yunjin <gzblues61@daum.net> (cherry picked from commit 29df1ac) * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --------- Co-authored-by: Jiho Lee <optional.int@kakao.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
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
Add an optional Redis-backed per-IP rate limiter for #403. The existing local limiter is unchanged.
Cause
The current limiter stores token buckets in process-local memory. Separate processes cannot share that state. The existing Redis pool also has unbounded waits and blocking I/O, so a request-path Redis command needs a bounded alternative.
Changes
cwist_mw_rate_limit_ip_redis(const cwist_rate_limit_redis_cfg_t *).Verification
Isolated Redis: 40 attempts from two processes share exactly seven admissions and 33 rejections.
TTL preservation, TTL repair, expiry reset, namespace isolation and IPv6 peer isolation passed.
Redis command errors, missing peers and paused-backend failure policies passed.
Fourteen fake-peer deadline, stale-response, reconnect, fork, SIGPIPE, malformed-reply and concurrency tests passed.
Extra resource-failure regression: a 120 ms pool-wait budget took 257.8 ms before the fallback fix and about 120 ms after it.
The integration and relevant sibling tests passed under ASan/UBSan on macOS. Leak detection was disabled there. ABI layout checks passed.
API baseline, API coverage ratchet, test wiring, changed-file formatting, Python lint and workflow parse passed.
Public-API tutorial gate passed: 30 tutorials built and 10 document snippets compiled.
Fat archive built and passed the same isolated-Redis integration when linked without separate vendor archives.
macOS amalgamation compilation is blocked by an undeclared SQLite
getpagesizecall. The exact base commit fails the same way under the same command; this is not a new failure from this diff. Linux CI passed both default and tiny amalgamation distribution jobs at0de355143.The full independent review and the follow-up review passed. The pre-publication findings are resolved: per-process first-error diagnostics, explicit fresh-connection/no-AUTH/no-SELECT contract, prefix-per-policy and IPv6 /128 limits. The diagnostic regression failed before the fix and passed afterwards, including ASan/UBSan.
CI follow-up
The GCC test warnings are fixed; Linux TSan and the shared Redis job passed at
cf80a6a1a. CodeQL alert #36 prompted explicit formatter-result, capacity and final-length checks before any pool I/O. The checked encoder, error cleanup and fault/bounds tests passed independent security review and local GCC/ASan/UBSan checks. The maintainer Autofix commit is preserved in history. Current head0de355143also merges dev and preserves both additive API sets (790 functions). Final CI at0de355143: 37 success, 1 skipped, 1 neutral; no failures or pending checks. Linux Redis integration, all 14 deadline tests and the new encoder bounds/fault regression passed. C/C++ CodeQL analysis passed; alert #36 is fixed on the PR merge ref, which has no open alerts. The CodeQL comparison check remains neutral because the base has an Actions-language configuration absent from this PR's analysis; this configuration warning was not suppressed.Limits
This is bounded blocking middleware, not an async reactor API. Initial pool creation/DNS and application logging are outside the Redis command budget. Scheduling and large trusted caller buffers are not real-time guarantees.
Use a dedicated DB 0 pool with the default user and no AUTH. Reconnects do not restore AUTH, SELECT, or other connection state. A server that requires AUTH applies the chosen failure policy to every request, with one first-error warning per config and process. Different policies need different key prefixes; IPv6 keys use full /128 addresses.
Pools must be created separately in each serving process after fork and outlive callbacks. The example uses
cwist_app_listen_exwith one worker. There is no new automatic worker-start hook, and inherited pools are rejected.Keys use the actual connected socket peer, not forwarded headers. A QUIC request without a TCP peer follows the configured missing-peer policy; QUIC path extraction is not added. Redis RESP2 needs a private network or TLS tunnel. Eviction, failover or external deletion can reset counters.
No merge or release is part of this change.
Closes #403.