nic-6443 opened a new pull request, #13508:
URL: https://github.com/apache/apisix/pull/13508

   ### Description
   
   The `nanoid` C rock (lua-resty-nanoid 0.1) used by the request-id plugin for 
`algorithm: nanoid` is broken in three independent ways (see 
[src/nanoid.h](https://github.com/aikin-vip/lua-resty-nanoid/blob/master/src/nanoid.h)):
   
   1. **Entropy collapse → duplicate ids.** `safe_custom()` reads 21 bytes from 
/dev/urandom but then collapses them into a single `srand()` seed by summing 
the *signed* bytes into an **uninitialized** accumulator, and derives the 
entire id from `rand()`. Two calls landing on the same seed produce identical 
ids. A 10k-call loop yields **~84% duplicates**. This is what 
`t/plugin/request-id.t` TEST 15 intermittently catches in CI as `ids not 
unique` (e.g. failing both the run and the flaky-rerun).
   2. **Malformed ids.** The output buffer is never NUL-terminated, so 
`lua_pushstring` reads past the end of the allocation — ids intermittently come 
out 22+ chars long with non-alphabet garbage bytes (observed: 
`2KN8t_467sf9WVd2DjR1f^`).
   3. **fd leak.** Every call `fopen()`s /dev/urandom and never closes it — one 
leaked fd per generated id.
   
   Fix: replace the rock with a small pure-Lua generator on top of 
`resty.random` (OpenSSL CSPRNG, bundled with OpenResty and already used by 
apisix core). The standard nanoid alphabet has exactly 64 characters, so 6 bits 
of CSPRNG output map to one character with no modulo bias. The output format 
(21 chars of `[A-Za-z0-9_-]`) is unchanged; the `nanoid` rock dependency is 
removed.
   
   The new test (sequential requests asserting id length, charset, and 
uniqueness) fails deterministically on the old implementation — malformed 
22-char ids show up within a couple hundred requests — and passes with the fix.
   
   ### Which issue(s) this PR fixes:
   
   N/A
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to