Skip to content

feat(middleware): add shared Redis rate limiting - #453

Merged
gg582 merged 5 commits into
devfrom
fix/403-redis-rate-limit
Oct 27, 2026
Merged

gg582 merged 5 commits into
devfrom
fix/403-redis-rate-limit

Conversation

@DPS0340

@DPS0340 DPS0340 commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

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

  • Add cwist_mw_rate_limit_ip_redis(const cwist_rate_limit_redis_cfg_t *).
  • Use one atomic Lua EVAL to increment and set a first-hit 60-second expiry. Active windows are not extended; missing expiry is repaired.
  • Share the same prefix and limit across independently created process-local pools.
  • Return 429 with Retry-After for excess requests. Default backend policy is fail-open; configurable fail-closed returns 503.
  • Add a typed pooled Redis command with one monotonic deadline for slot wait, reconnect, write and reply I/O. Timed-out connections are discarded; ambiguous commands are not retried.
  • Cache the resolved peer for reconnects, reject inherited pools, and contain SIGPIPE on the timed path.
  • Add actual two-process Redis tests, fake-peer deadline tests, CI, API baseline and English/Korean docs.

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 getpagesize call. 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 at 0de355143.

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 head 0de355143 also merges dev and preserves both additive API sets (790 functions). Final CI at 0de355143: 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_ex with 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.

Comment thread src/net/redis/cwist_redis.c Fixed
DPS0340 and others added 2 commits October 26, 2026 21:49
…lly overflowing call to snprintf'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@gg582
gg582 marked this pull request as ready for review October 26, 2026 13:37
DPS0340 and others added 2 commits October 26, 2026 22:38
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.
@gg582

gg582 commented Oct 27, 2026

Copy link
Copy Markdown
Member

yo, LGTM.

@gg582
gg582 merged commit 29df1ac into dev Oct 27, 2026
39 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants