lib: monkey: upgrade to v1.8.9 - #12212
Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates the bundled Monkey HTTP server to a newer upstream version. It adds a pluggable TLS transport layer (OpenSSL, mbedTLS, or a disabled stub), refactors network transport dispatch through ChangesMonkey Server TLS Integration
Vendored libevent Library Upgrade
Estimated code review effort: 5 (Critical) | ~180+ minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant mk_server
participant mk_net
participant mk_tls_transport
participant TLSBackend as OpenSSL/mbedTLS
Client->>mk_server: TCP connect
mk_server->>mk_net: mk_net_transport_default() or mk_tls_transport()
mk_net->>mk_tls_transport: mk_tls_enabled()
mk_tls_transport->>TLSBackend: mk_tls_init(server)
TLSBackend-->>mk_tls_transport: TLS handshake complete
mk_tls_transport-->>mk_server: read/write via mk_plugin_network callbacks
mk_server-->>Client: HTTP response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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: 99462f508d
ℹ️ 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".
| else { | ||
| ret = SSL_write(ssl, buf, used); |
There was a problem hiding this comment.
When a TLS static-file response requests fewer bytes than remain in the file, such as an HTTP range response, remain eventually reaches zero but the next loop iteration enters this branch and writes another full buffer. The response therefore continues through the rest of the file despite its advertised Content-Length, which can corrupt the next response on a keep-alive connection; terminate the loop when remain reaches zero. The equivalent loop in tls_mbedtls.c has the same behavior.
Useful? React with 👍 / 👎.
| option(MK_PLUGIN_LOGGER "Log Writer" No) | ||
| option(MK_PLUGIN_MANDRIL "Security" Yes) | ||
| option(MK_PLUGIN_TLS "TLS/SSL support" No) | ||
| option(MK_TLS "TLS/SSL support" Yes) |
There was a problem hiding this comment.
When Fluent Bit is configured with -DFLB_TLS=Off while the HTTP server remains enabled, this independent default still leaves MK_TLS=ON; the Monkey subdirectory then discovers OpenSSL or builds its bundled mbedTLS backend. TLS-disabled Fluent Bit builds therefore retain TLS dependencies and code instead of honoring the top-level option, so this setting should inherit FLB_TLS when Monkey is embedded.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/monkey/mk_core/deps/libevent/evdns.c (1)
89-107: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the vendored libevent source clearly.
The tree declares
libevent 2.2.2-alpha, andlib/monkey/mk_core/deps/libevent/ChangeLogdescribes changes in that alpha release. Add a local vendor note that says whether this is the upstream tag or an unreleased snapshot, and clarify the upstream project/version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/evdns.c` around lines 89 - 107, Add a local vendor note in the libevent dependency directory documenting the upstream project and version (libevent 2.2.2-alpha), and explicitly state whether the vendored source matches an upstream tag or is an unreleased snapshot. Keep the note alongside the existing ChangeLog and avoid modifying evdns.c behavior.lib/monkey/mk_core/deps/libevent/test/bench_httpclient.c (1)
161-170: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFree
riin the newbufferevent_socket_newfailure path.
riis allocated at line 161. The new early return at lines 166-170 leaves it unreferenced. Cppcheck reports the same leak at line 169.Also note
perroris not accurate here, becausebufferevent_socket_newdoes not seterrno. Considerfprintf(stderr, ...).🧹 Proposed fix
b = bufferevent_socket_new(base, sock, BEV_OPT_CLOSE_ON_FREE); if (b == NULL) { - perror("bufferevent_socket_new"); + fprintf(stderr, "bufferevent_socket_new failed\n"); + free(ri); evutil_closesocket(sock); return -1; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/test/bench_httpclient.c` around lines 161 - 170, Update the bufferevent_socket_new failure branch in the benchmark setup to free the previously allocated ri before returning, and replace perror with an explicit fprintf(stderr, ...) message because this function does not set errno.Source: Linters/SAST tools
lib/monkey/mk_core/deps/libevent/test/regress_buffer.c (1)
156-159: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winThe trailing-empty-chain loop never advances the cursor.
chainis not reassigned inside the loop body. Ifbufends with an empty chain that follows a non-empty chain,evbuffer_get_wasteloops forever andtest_evbuffer_expandhangs. The current test data reaches this loop only withchain == NULL, so the defect is latent.This code is upstream libevent, and the repository guidelines require confirmation before editing bundled library code. Confirm whether you want a local patch or prefer to keep the vendor drop byte-identical.
🐛 Proposed fix
/* subsequent empty chains */ while (chain) { a += chain->buffer_len; + chain = chain->next; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/test/regress_buffer.c` around lines 156 - 159, Confirm whether to patch the bundled libevent code locally or preserve the vendor drop byte-identical; if patching is approved, update the trailing-empty-chain loop in evbuffer_get_waste/test_evbuffer_expand to advance chain on every iteration so it terminates after processing all chains.Source: Coding guidelines
🟠 Major comments (36)
lib/monkey/integration_tests/run_tests.py-11-13 (1)
11-13: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSelect the virtual-environment interpreter for Windows.
VENV_PYTHONalways uses.venv/bin/python3. Windows virtual environments use.venv\\Scripts\\python.exe. This prevents the runner from re-executing in its managed environment on Windows.Proposed fix
SUITE_ROOT = Path(__file__).resolve().parent -VENV_PYTHON = SUITE_ROOT / ".venv" / "bin" / "python3" +if os.name == "nt": + VENV_PYTHON = SUITE_ROOT / ".venv" / "Scripts" / "python.exe" +else: + VENV_PYTHON = SUITE_ROOT / ".venv" / "bin" / "python3" REEXEC_ENV = "MONKEY_INTEGRATION_REEXEC"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/integration_tests/run_tests.py` around lines 11 - 13, Update the VENV_PYTHON interpreter selection to use the Windows virtual-environment path `.venv/Scripts/python.exe` on Windows and retain `.venv/bin/python3` on other platforms, using the existing SUITE_ROOT and platform-detection facilities.lib/monkey/mk_core/mk_event_libevent.c-69-79 (1)
69-79: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win) Make Winsock initialization completion-safe.
These two sites publish
WSAStartupsuccess before the Winsock layer is actually initialized. Concurrent callers can use sockets while startup is still in progress.Sites to fix
lib/monkey/mk_core/mk_event_libevent.c#L69andlib/monkey/mk_core/mk_win32_socketpair.c#L43both set the initialized flag beforeWSAStartup()completes.
- Use a completion-safe one-time initialization primitive such as a three-state flag/lock, or
InitOnceExecuteOnce()where available.- Hold the same lifecycle rule in
mk_win32_socketpair()so direct socketpair callers do not bypass the Winsock completion barrier.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/mk_event_libevent.c` around lines 69 - 79, Make Winsock initialization completion-safe in lib/monkey/mk_core/mk_event_libevent.c lines 69-79 by replacing the pre-startup initialized flag publication with a one-time initialization mechanism that blocks concurrent callers until WSAStartup completes and resets the state on failure. Apply the same lifecycle rule in lib/monkey/mk_core/mk_win32_socketpair.c lines 43-51 so mk_win32_socketpair() cannot bypass the completion barrier; both sites must share the same initialization state and only permit socket use after successful startup.lib/monkey/test/lib_server.c-244-256 (1)
244-256: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse
TEST_ASSERTfor the NULL check.
TEST_CHECKrecords a failure and continues. Ifmk_net_transport_default()returns NULL, execution reaches line 250 and dereferences a NULL pointer. The test binary then crashes instead of reporting a failure. UseTEST_ASSERTso the test stops at the NULL check.🛠️ Proposed fix
transport = mk_net_transport_default(); - TEST_CHECK(transport != NULL); + TEST_ASSERT(transport != NULL); TEST_CHECK(transport->read != NULL);Based on the coding guideline "Validate both success and failure paths, including invalid payloads, boundary sizes, and null or missing fields."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/test/lib_server.c` around lines 244 - 256, Replace the initial transport NULL check in test_core_plain_transport_available with TEST_ASSERT so the test stops immediately when mk_net_transport_default() returns NULL; leave the subsequent transport member checks unchanged.Source: Coding guidelines
lib/monkey/integration_tests/src/monkey_manager.py-228-245 (1)
228-245: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValgrind errors are never detected with the current shutdown path.
stop()callsself.process.terminate(), which sendsSIGTERM. When Valgrind exits because of a signal, the process exit status reports the signal, not--error-exitcode=99._validate_valgrindtherefore never observes99, and leak reports are silently ignored.Parse the Valgrind log for the error summary instead of relying only on the exit code.
🛠️ Proposed approach
def _validate_valgrind(self, return_code: int) -> None: if not self.valgrind_enabled: return if return_code == 99: raise AssertionError(self.valgrind_log_file.read_text(encoding="utf-8")) + + if not self.valgrind_log_file.exists(): + return + + report = self.valgrind_log_file.read_text(encoding="utf-8", errors="replace") + match = re.search(r"ERROR SUMMARY: (\d+) errors", report) + if match and int(match.group(1)) > 0: + raise AssertionError(report)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/integration_tests/src/monkey_manager.py` around lines 228 - 245, Update _validate_valgrind to inspect the Valgrind log for its error summary and detect reported errors even when stop() terminates the process via SIGTERM; retain the existing exit-code check where applicable, and raise AssertionError with the log contents whenever Valgrind reports errors.lib/monkey/.github/workflows/build-pr.yaml-83-90 (1)
83-90: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPin and verify the CMake archive SHA-256 before extraction.
Invoke-WebRequestwrites the file, lines 83-90 only check the ZIPPKheader, thenExpand-Archiveextracts it. Use a pinned SHA-256 digest forcmake-${cmakeVersion}-.windows-${cmakeArch}.zipand compare it withGet-FileHash -Algorithm SHA256before extracting. Do not download the expected digest from the release source at runtime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/.github/workflows/build-pr.yaml` around lines 83 - 90, Update the workflow validation between Invoke-WebRequest and Expand-Archive to compare Get-FileHash -Algorithm SHA256 for $cmakeZip against a pinned, repository-defined SHA-256 digest for cmake-${cmakeVersion}-.windows-${cmakeArch}.zip. Fail the workflow on mismatch, keep the existing ZIP-header validation if appropriate, and do not retrieve the expected digest from the release source at runtime.lib/monkey/CMakeLists.txt-64-70 (1)
64-70: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGive
MK_TLSprecedence overMK_PLUGIN_TLS.
MK_TLSis exposed as an explicit option, but the compatibility block can overwrite it withMK_PLUGIN_TLSafteroption()has already defined it. Assign fromMK_PLUGIN_TLSonly whenMK_TLSis not already defined, or remove the later override.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/CMakeLists.txt` around lines 64 - 70, Update the compatibility block around MK_TLS and MK_PLUGIN_TLS so the explicit MK_TLS option always takes precedence. Only assign MK_TLS from MK_PLUGIN_TLS when MK_TLS has not already been defined, or remove the override while preserving legacy compatibility.lib/monkey/mk_core/deps/libevent/CMakeLists.txt-131-137 (1)
131-137: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign
EVENT__DISABLE_OPENSSLinlib/monkey/mk_core/deps/CMakeLists.txtwith the new string option.
lib/monkey/mk_core/deps/CMakeLists.txtforcesEVENT__DISABLE_OPENSSLas aBOOLvalue (ON), so CMake treats it asON/OFFat the caller: when Monkey does not explicitly passAUTO,libeventseesON, disables OpenSSL, and stops adding the bundled SSL backend. Set the caller and documentation to the accepted valuesAUTO,ON, orOFFinstead, so future callers do not generate invalid bools.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/CMakeLists.txt` around lines 131 - 137, Update the EVENT__DISABLE_OPENSSL definition in the parent CMake configuration and its documentation to use the string values AUTO, ON, or OFF instead of BOOL semantics or a forced ON value. Preserve the libevent option’s accepted-value behavior and ensure callers default to AUTO unless explicitly selecting ON or OFF.lib/monkey/mk_core/deps/libevent/make-event-config.sed-27-27 (1)
27-27: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
\3for the symbol letter in theifdef/ifndefrule.This bundled
lib/monkey/mk_core/deps/libevent/make-event-config.sedrule captures indent without renamingSTDC_HEADERS, but theifdef/ifndefsymbol uses the wrong group. For input#ifdef FOO, the generated guard becomes#ifdef EVENT__OO; for#ifndef FOO, it becomes#ifndef EVENT__nOO, while thedefine/undefentries use the renamed symbol name.🐛 Proposed fix for the capture-group index
-s/#\( *\)if\(n*\)def \([A-Z]\)/#\1if\2def EVENT__\2/ +s/#\( *\)if\(n*\)def \([A-Z]\)/#\1if\2def EVENT__\3/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/make-event-config.sed` at line 27, Update the ifdef/ifndef substitution rule in make-event-config.sed to use capture group \3 for the symbol letter when constructing the EVENT__-prefixed guard, preserving the existing directive type and indentation captures so FOO becomes EVENT__FOO consistently with define/undef entries.lib/monkey/mk_core/deps/libevent/evdns.c-5416-5421 (1)
5416-5421: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnchecked allocations create NULL-dereference paths in the new cache and TCP code.
mm_callocat Line 5417 is not checked beforecache->base,cache->name, andcache->aiare assigned. Static analysis reports the same finding. Two sibling sites share this pattern:
- Line 5419:
mm_strdup(nodename)result is stored incache->namewithout a check.evdns_cache_comparecallsstrcasecmp(a->name, b->name)on that value.- Line 1351:
reply.data.raw = mm_malloc(buf_size)is not checked beforememcpywrites at Line 1378 and Line 1414.- Line 5829:
data->nodename = mm_strdup(nodename)is not checked beforeevdns_cache_writeuses it.These are allocation-failure paths only, so normal operation is unaffected. This file is a vendored upstream import, so do not patch it locally. Report the defect to the libevent project and track the fix through the next vendor drop.
As per path instructions, "Ask for explicit user confirmation before editing bundled or separately maintained library code under
lib/; keep such patches isolated and document their upstream project/path."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/evdns.c` around lines 5416 - 5421, Do not modify this vendored libevent file. Report the unchecked allocation defects to the libevent project, covering the cache allocation and name duplication near the evdns cache insertion, reply buffer allocation in the TCP code, and nodename duplication before evdns_cache_write; track the upstream fix for inclusion in the next vendor drop, and request explicit user confirmation before any bundled-library edit.Sources: Path instructions, Linters/SAST tools
lib/monkey/mk_core/deps/libevent/bufferevent.c-885-903 (1)
885-903: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle replacement with the current descriptor.
If
fd == old_fd, Line 897 closes the active descriptor.be_async_ctrl()then accepts the same descriptor without reassigning it. The function can return success while the bufferevent retains a closed descriptor.Return success before closing when both descriptors are equal. Also preserve the current descriptor if
BEV_CTRL_SET_FDfails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/bufferevent.c` around lines 885 - 903, Update bufferevent_replacefd to return success without closing or reassigning when the requested fd matches old_fd, and retain the existing descriptor if BEV_CTRL_SET_FD fails after closing the old descriptor. Ensure failure handling restores or preserves the current descriptor state rather than leaving the bufferevent with a closed descriptor.lib/monkey/mk_core/deps/libevent/arc4random.c-378-379 (1)
378-379: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFail closed when entropy seeding fails.
arc4_stir()returns-1when all entropy sources fail.arc4_stir_if_needed()ignores that result, soarc4random(),arc4random_buf(), andarc4random_uniform()can still callarc4_getword()with an unreseeded RC4 state. Propagatearc4_stir()failure to a fail-closed path, and add failure-injection coverage for each configured entropy seed source.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/arc4random.c` around lines 378 - 379, Update arc4_stir_if_needed() to check the return value of arc4_stir() and propagate failure so arc4random(), arc4random_buf(), and arc4random_uniform() never use an unreseeded RC4 state. Add failure-injection tests covering each configured entropy seed source and verify all affected APIs fail closed when seeding fails.Source: Learnings
lib/monkey/mk_core/deps/libevent/bufferevent_async.c-112-123 (1)
112-123: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not accept
ERROR_INVALID_PARAMETERas a successful IOCP association.
CreateIoCompletionPort()returnsERROR_INVALID_PARAMETERwhen a handle is already associated with another IOCP. This helper only checks the result and callers continue as if this IOCP owns the descriptor, so callbacks can queue completions to another port.Make
event_iocp_port_associate_()reject association unless it can confirm the handle already belongs to the sameport->port; then bothbufferevent_async_new_()andBEV_CTRL_SET_FDfail the path. Add a two-IOCP regression case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/bufferevent_async.c` around lines 112 - 123, Update fatal_error and event_iocp_port_associate_ so ERROR_INVALID_PARAMETER is not treated as successful association; only accept an already-associated handle after confirming it belongs to the same port->port, otherwise reject it. Ensure bufferevent_async_new_() and BEV_CTRL_SET_FD propagate the failure, and add a regression test using two IOCP instances.lib/monkey/mk_core/deps/libevent/configure-13755-13755 (1)
13755-13755: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFix the malformed shell assignment before the Emscripten archive_cmds block.
lib/monkey/mk_core/deps/libevent/configureline 13755 contains='-fPIC', which is not a valid shell assignment. Use the pic compiler variable assignment, for examplelt_prog_compiler_pic='-fPIC', so Emscripten configs do not fail with=-fPIC: command not foundbeforearchive_cmdsis set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/configure` at line 13755, Replace the malformed shell assignment immediately before the Emscripten archive_cmds block with a valid assignment to the pic compiler variable, using lt_prog_compiler_pic and retaining the -fPIC value so configure no longer attempts to execute =-fPIC.lib/monkey/mk_core/deps/libevent/poll.c-106-107 (1)
106-107: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winApply the required C block and declaration conventions.
Use braces for every changed
ifbody. Declaresawith the othersigfd_dellocal variables.
lib/monkey/mk_core/deps/libevent/poll.c#L106-L107: wrap theevsig_init_()fallback in braces.lib/monkey/mk_core/deps/libevent/select.c#L122-L123: wrap theevsig_init_()fallback in braces.lib/monkey/mk_core/deps/libevent/signal.c#L250-L251: wrap the failure return in braces.lib/monkey/mk_core/deps/libevent/signalfd.c#L147-L155: wrap the allocation and registration failure branches in braces.lib/monkey/mk_core/deps/libevent/signalfd.c#L190-L199: movesato the start ofsigfd_del.lib/monkey/mk_core/deps/libevent/signalfd.c#L209-L211: wrap the disabled-backend return in braces.As per coding guidelines, “Always use braces for
if,else,while, anddoblocks” and declare variables at the start of functions. Based on learnings, use K&R braces for control statements.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/poll.c` around lines 106 - 107, Apply K&R-style braces to all specified control-flow branches: wrap the evsig_init_ fallback in poll.c and select.c, the failure return in signal.c, and the allocation, registration-failure, and disabled-backend branches in signalfd.c; in signalfd.c, declare sa with the other locals at the start of sigfd_del. Update every listed file and preserve the existing branch behavior.Sources: Coding guidelines, Learnings
lib/monkey/mk_core/deps/libevent/signalfd.c-97-127 (1)
97-127: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPreserve the saved handler during reinitialization.
When
oldis nonzero,sigfd_free_sigevent()frees onlysigev. Lines 119-127 then overwrite the existingsig->sh_old[signo]pointer. Each post-fork reinitialization leaks one savedstruct sigaction.Retain the existing saved action during reinitialization. Allocate and query
sigactiononly for the first registration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/signalfd.c` around lines 97 - 127, Update the signal registration flow around sigfd_free_sigevent() and sig->sh_old[signo] so reinitialization with old nonzero preserves the existing saved struct sigaction. Only allocate sig->sh_old[signo] and query sigaction for the first registration, while retaining the existing error handling for initial allocation or sigaction failure.lib/monkey/mk_core/deps/libevent/signal.c-412-430 (1)
412-430: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not log from
evsig_handler.
event_warnxandstrerrorare not async-signal-safe. A full notification pipe can therefore deadlock or crash while handling a signal. RetryEINTR, but silently dropEAGAINand other write failures, or record them through an async-signal-safe mechanism.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/signal.c` around lines 412 - 430, Remove the event_warnx and strerror logging from the write-failure path in evsig_handler. Continue retrying write when errno is EINTR, and silently ignore EAGAIN and all other failures while preserving the existing loop termination behavior; do not invoke non-async-signal-safe functions from the signal handler.lib/monkey/mk_core/deps/libevent/signalfd.c-53-65 (1)
53-65: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle failed nonblocking signalfd reads.
sigfd_cbcalls nonblockingread()viasigfdops = { "signalfd_signal", NULL, ... } < 0. If the descriptor’s readiness becomes stale,read()returns-1withEAGAIN; if interrupted before any bytes are read, it can return-1withEINTR. The currentEVUTIL_ASSERT(ret == sizeof(fdsi))turns these recoverable states into debug aborts. SkipEAGAIN/EINTRfrom the callback path and only handle other non-retryableread()failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/signalfd.c` around lines 53 - 65, Update sigfd_cb to handle read() failures before asserting the full signalfd_siginfo size: return quietly for EAGAIN and EINTR, while preserving assertion or handling for other non-retryable errors. Continue processing fdsi only after a successful read of sizeof(fdsi).lib/monkey/mk_core/deps/libevent/signalfd.c-130-137 (1)
130-137: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSet the process-wide signal mask before enabling
signalfd.
sigprocmask()only blocks the signal in the calling thread. If another worker thread has no matching block on this signal, a process-directed signal can bypasssignalfdand follow the normal disposition or custom handler that libevent saves. Block the target signals beforepthread_create()/worker-thread setup, or rejectEVENT_USE_SIGNALFDafter threaded operation starts. Add a multi-threaded signalfd delivery test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/signalfd.c` around lines 130 - 137, Update the signalfd setup around the signal-mask logic in signalfd.c so target signals are blocked process-wide before pthread_create or other worker-thread initialization, ensuring all threads inherit the mask; alternatively reject EVENT_USE_SIGNALFD once threaded operation has begun. Preserve existing cleanup behavior and add a multi-threaded test verifying signals are delivered through signalfd rather than normal dispositions or handlers.lib/monkey/mk_core/deps/libevent/wepoll.c-43-69 (1)
43-69: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign
EPOLLONESHOTinwepoll.hwithwepoll.c.
wepoll.cdefinesEPOLLONESHOTas bit 31 and checks that bit insock_feed_event, butwepoll.hdefines it as bit 30. The localepoll.ctranslation unit includeswepoll.h, so the library API contract is inconsistent. SetEPOLLONESHOTto bit 31 inwepoll.h, or centralize it under one header.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/wepoll.c` around lines 43 - 69, Update the EPOLLONESHOT definition in wepoll.h to use bit 31, matching the enum and macro definitions in wepoll.c and the bit checked by sock_feed_event. Keep the public epoll event constants consistent across both headers and translation units.lib/monkey/mk_core/deps/libevent/sample/http-server.c-498-520 (1)
498-520: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
maincontinues afterevent_base_new_with_configorevhttp_newfails, which causes a null-pointer dereference.At Line 499 the code sets
ret = 1but does not jump toerr. Execution reaches Line 507 and callsevhttp_new(NULL). At Line 508 the code again setsret = 1without jumping toerr. Execution reaches Line 514 and callsevhttp_set_cb(http, ...)withhttp == NULL. The sample crashes instead of exiting with an error.Related error-path gaps in the same function share this cause:
- Line 535 uses
return 1instead ofret = 1; goto err;, sohttp,base, andcfgleak.- Lines 577 and 579 jump to
errwithout settingret, so the sample returns success after a failure.This file is vendored upstream libevent sample code. Do not patch it inline as part of this upgrade. Report the defect upstream, or isolate any local fix in a separate commit that documents the upstream project and path.
🐛 Reference fix, if a local patch is approved
base = event_base_new_with_config(cfg); if (!base) { fprintf(stderr, "Couldn't create an event_base: exiting\n"); ret = 1; + goto err; } event_config_free(cfg); cfg = NULL; /* Create a new evhttp object to handle requests. */ http = evhttp_new(base); if (!http) { fprintf(stderr, "couldn't create evhttp. Exiting.\n"); ret = 1; + goto err; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/sample/http-server.c` around lines 498 - 520, Do not modify the vendored sample inline. Report the error-path defect upstream, or create a separate documented commit for the local fix identifying upstream libevent and this sample path; if approved, ensure main exits via the existing err cleanup path after event_base_new_with_config or evhttp_new fails, replaces the direct return failure path with ret = 1 followed by goto err, and sets ret before the later err jumps so failures cannot return success.Source: Coding guidelines
lib/monkey/mk_core/deps/libevent/test/regress_ws.c-110-113 (1)
110-113: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
event_base_loopexitreceives0xDEADBEEF, not an event base.
http_on_ws_cbat line 134 passes(void *)0xDEADBEEFas the callback argument. Inon_ws_msg_cb,argtherefore holds0xDEADBEEF. Line 112 passes that value toevent_base_loopexitas astruct event_base *.If an unexpected message arrives, the process dereferences an invalid pointer instead of exiting the loop. The file-static
exit_baseholds the correct base and is already used for that purpose at line 444.Use
exit_base, and settest_okto a failure value so the cause is visible.🐛 Proposed fix
} else { /* unexpected test message */ - event_base_loopexit(arg, NULL); + test_ok = -1; + event_base_loopexit(exit_base, NULL); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/test/regress_ws.c` around lines 110 - 113, Update the unexpected-message branch in on_ws_msg_cb to call event_base_loopexit with the file-static exit_base instead of the callback arg, and set test_ok to the existing failure value before exiting so the test records the unexpected message.lib/monkey/mk_core/deps/libevent/test/regress_ssl.c-324-327 (1)
324-327: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
if (REGRESS_OPENSSL_BATCH_WRITE)is always true.The condition tests the enum constant
REGRESS_OPENSSL_BATCH_WRITE(value 8192), not thetypebitmask.BUFFEREVENT_SSL_BATCH_WRITEis therefore set on every bufferevent in every testcase that callsopen_ssl_bufevs.Two effects follow. The
bufferevent_socketpair_batch_writetestcase no longer tests a distinct configuration. All other testcases silently run in batch-write mode, so the non-batch write path loses coverage.Test the bit in
type.This is vendored upstream code. Confirm the upstream release carries the same defect before you patch it locally, and prefer reporting it upstream.
🐛 Proposed fix
- if (REGRESS_OPENSSL_BATCH_WRITE) { + if (type & REGRESS_OPENSSL_BATCH_WRITE) { bufferevent_ssl_set_flags(*bev1_out, BUFFEREVENT_SSL_BATCH_WRITE); bufferevent_ssl_set_flags(*bev2_out, BUFFEREVENT_SSL_BATCH_WRITE); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/test/regress_ssl.c` around lines 324 - 327, Update the condition in open_ssl_bufevs to test whether the type bitmask includes REGRESS_OPENSSL_BATCH_WRITE, rather than testing the enum constant directly; keep BUFFEREVENT_SSL_BATCH_WRITE enabled only for batch-write configurations. Before applying the local patch, verify the vendored upstream release contains the same defect and prefer reporting it upstream.lib/monkey/mk_core/deps/libevent/test/regress_testutils.c-396-420 (1)
396-420: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
parse_csv_address_listignoresfamilyand can dereference a NULL token.Two defects exist in this function.
First, line 412 passes
AF_INETtoevutil_inet_ptoninstead offamily. The function computesnext_addras astruct in6_addr *whenfamilyisAF_INET6, but then parses the token as IPv4. IPv6 input fails to parse, and a successful IPv4 parse writes only 4 bytes into a 16-byte slot. TheAAAAbranch at lines 255-261 does not call this helper today, so the defect is latent, but it will trigger for any future IPv6 caller.Second, the loop is a
do/while. Ifscontains no token,strtokreturnsNULLat line 407 and the body still runs, soevutil_inet_ptonreceives aNULLstring pointer.Use
familyin the parse call and convert the loop to awhile.This is vendored upstream code. Confirm the upstream release before you diverge locally.
🐛 Proposed fix
token = strtok(buf, ","); - do { + while (token) { tt_assert((unsigned)i < addrs_size); next_addr = (family == AF_INET) ? (void *)((struct in_addr*)addrs + i) : (void *)((struct in6_addr*)addrs + i); - if (!evutil_inet_pton(AF_INET, token, next_addr)) { + if (!evutil_inet_pton(family, token, next_addr)) { TT_DIE(("Bad %s value %s in table", (family == AF_INET) ? "A" :"AAAA", token)); } ++i; token = strtok (NULL, ","); - } while (token); + } end: return i;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/test/regress_testutils.c` around lines 396 - 420, Update parse_csv_address_list to pass its family parameter to evutil_inet_pton, preserving correct IPv4 and IPv6 parsing for the address storage selected by family. Replace the do/while iteration with a while loop so the body executes only when strtok returns a non-NULL token, and confirm the corresponding upstream vendored release before applying the local change.lib/monkey/mk_core/deps/libevent/test/test.sh-144-149 (1)
144-149: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
do_testnever applies the backend configuration.Line 144 shifts the positional parameters. For the call
do_test EPOLL "(timerfd)",$#is 1 after the shift, so the test[ $# -gt 1 ]at line 145 is false.backend_confstays empty. The timerfd, changelist, timerfd+changelist, and signalfd branches at lines 153-162 never run, so the-t,-c,-T, and-Soptions silently run plain backend tests.Line 146 also reads
$2after the shift, which is the parameter after the configuration.🐛 Proposed fix
do_test() { backend="$1" && shift - if [ $# -gt 1 ]; then - backend_conf="$2" && shift + if [ $# -gt 0 ]; then + backend_conf="$1" && shift else backend_conf="" fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_core/deps/libevent/test/test.sh` around lines 144 - 149, Fix do_test’s positional-argument handling so the optional backend configuration is captured before shifting away the backend argument. Detect the configuration based on the remaining argument count and read the correct positional parameter, ensuring calls such as do_test EPOLL "(timerfd)" set backend_conf and activate the -t, -c, -T, and -S test branches.lib/monkey/tls/tls.conf-22-23 (1)
22-23: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winGenerate at least 2048-bit DH parameters.
Line 23 recommends 1024-bit DH parameters. This strength is obsolete and can fail stricter TLS security policies. Change the sample command to generate 2048-bit parameters.
Proposed fix
- # $ openssl dhparam -out dhparam.pem 1024 + # $ openssl dhparam -out dhparam.pem 2048🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/tls/tls.conf` around lines 22 - 23, Update the sample OpenSSL command in the TLS configuration comment to generate 2048-bit DH parameters instead of 1024-bit parameters.lib/monkey/tls/tls_openssl.c-677-699 (1)
677-699: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
mk_tls_send_filesends data pastfile_countin both TLS backends. The same loop was copied into both files.remainstarts atfile_countand decreases with each write, but the loop only exits when a write fails or the file ends. Onceremainreaches 0, theremain > 0branch is skipped and theelsebranch writes the full buffer, so the transport keeps sending until end of file. Range requests and chunk boundaries break as a result.
lib/monkey/tls/tls_openssl.c#L677-L699: exit thedo/whileloop whenremainreaches 0, and limit eachtls_preadtomin(SENDFILE_BUF_SIZE, remain).lib/monkey/tls/tls_mbedtls.c#L779-L802: apply the identical change to the mbedTLS loop, keepinghandle_returnfor the error path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/tls/tls_openssl.c` around lines 677 - 699, Update the mk_tls_send_file loop in lib/monkey/tls/tls_openssl.c:677-699 to stop when remain reaches zero and pass min(SENDFILE_BUF_SIZE, remain) to tls_pread; apply the identical bounds and termination change in lib/monkey/tls/tls_mbedtls.c:779-802 while preserving handle_return on the mbedTLS error path.lib/monkey/mk_bin/monkey.c-356-364 (1)
356-364: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle a failed configuration-directory resolution.
mk_default_config_dirreturns NULL when no candidate directory exists. Line 363 then assigns NULL toserver->path_conf_root.mk_server_setupcallsmk_config_start_configure(server)with that NULL root, and the guard added atlib/monkey/mk_server/monkey.cline 195 only triggers whenpath_conf_rootis not NULL. The failure is therefore silent, and the reported error does not name the real cause.🛡️ Proposed fix
else { resolved_config_dir = mk_default_config_dir(default_config_dir, sizeof(default_config_dir), argv[0]); + if (resolved_config_dir == NULL) { + mk_err("Could not locate a configuration directory, use -c/--configdir."); + return EXIT_FAILURE; + } server->path_conf_root = (char *) resolved_config_dir; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_bin/monkey.c` around lines 356 - 364, Handle a NULL result from mk_default_config_dir before assigning server->path_conf_root: detect the failed resolution, report a configuration-directory resolution error that identifies the actual cause, and abort or propagate failure through the surrounding server setup path instead of continuing with a NULL root. Preserve the existing path_config and successful default-directory branches.lib/monkey/tls/tls_mbedtls.c-854-880 (1)
854-880: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInitialize
server_context->mutexbefore use.
mk_mem_alloc_zonly zeroes the memory.tls_global_initthen locksserver_context->mutexat line 503, andmk_tls_exitcallspthread_mutex_destroyon it at line 962. A zeroedpthread_mutex_tis not a portable initialized mutex; on Windows and on some pthread implementations the lock and the destroy are undefined behavior. The OpenSSL backend callspthread_mutex_initatlib/monkey/tls/tls_openssl.cline 768.
mk_tls_initalso does not check the allocation result before writingserver_context->server.🛡️ Proposed fix
if (used) { /* If it's used, load certificates.. mandatory */ server_context = mk_mem_alloc_z(sizeof(struct polar_server_context)); + if (server_context == NULL) { + return -1; + } + pthread_mutex_init(&server_context->mutex, NULL); server_context->server = server;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/tls/tls_mbedtls.c` around lines 854 - 880, Update mk_tls_init to validate the mk_mem_alloc_z result before dereferencing server_context, and initialize server_context->mutex with pthread_mutex_init before calling tls_global_init. Handle allocation or mutex-initialization failure through the existing cleanup/error path so mk_tls_exit never destroys an uninitialized mutex.lib/monkey/mk_bin/monkey.c-141-159 (1)
141-159: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winThe resolver prefers a working-directory-relative
confover the installed configuration.Line 141 checks the relative path
confbefore the compiled-inMK_PATH_CONF. The server then loads configuration, MIME types, sites, and plugin definitions from whatever directory the process was started in. If the working directory is writable by another user, that user controls the server configuration, including listeners and TLS file paths.Check
MK_PATH_CONFfirst, and treat the relativeconfdirectory as a development fallback only.
realpath(path, buf)at line 142 also requiresbufto hold at leastPATH_MAXbytes; it ignores thesizeparameter. The current caller passes aPATH_MAXbuffer, so no overflow occurs today. Add a guard so a future caller with a smaller buffer cannot overflow. The same applies at lines 183 and 199.🔒️ Proposed reordering
static const char *mk_default_config_dir(char *buf, size_t size, const char *argv0) { char exe_path[PATH_MAX]; char candidate[PATH_MAX]; char *last_slash; int written; ssize_t len; + if (size < PATH_MAX) { + return NULL; + } + + if (MK_PATH_CONF[0] != '\0' && mk_dir_exists(MK_PATH_CONF) == MK_TRUE) { + written = snprintf(buf, size, "%s", MK_PATH_CONF); + if (written < 0 || (size_t) written >= size) { + return NULL; + } + return buf; + } + if (mk_dir_exists("conf") == MK_TRUE) { if (realpath("conf", buf) != NULL) { return buf; } written = snprintf(buf, size, "%s", "conf"); if (written < 0 || (size_t) written >= size) { return NULL; } return buf; } - - if (MK_PATH_CONF[0] != '\0' && mk_dir_exists(MK_PATH_CONF) == MK_TRUE) { - written = snprintf(buf, size, "%s", MK_PATH_CONF); - if (written < 0 || (size_t) written >= size) { - return NULL; - } - return buf; - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_bin/monkey.c` around lines 141 - 159, Update the configuration-directory resolver to check the compiled-in MK_PATH_CONF location before the working-directory-relative conf fallback, ensuring installed configuration takes precedence. In the relative conf branch, validate that the supplied buffer size is at least PATH_MAX before calling realpath, and apply the same guard to the realpath calls at the other referenced locations. Preserve the existing fallback and failure behavior for invalid or insufficient paths.lib/monkey/plugins/dirlisting/dirlisting.c-811-828 (1)
811-828: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDeallocate the dirlisting request on HTML-title allocation failures.
After
request->file_listis allocated, failures inmk_string_html_escape()and the firstmk_dirhtml_tag_assign()currentlymk_api->mem_free(request)directly. This skipsmk_dirhtml_cleanup()logic, leavingsr->handler_datapointing at freed request state and leakingrequest->file_listplusrequest->toc. Clear both paths likemk_http_cb_request_free()clearssr->handler_data, and freerequest->file_list/request->toc; otherwise keep the request lifecycle throughmk_dirhtml_cleanup().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/plugins/dirlisting/dirlisting.c` around lines 811 - 828, Update the failure paths after request->file_list allocation, including mk_string_html_escape() and the first mk_dirhtml_tag_assign() failure, to release the request through mk_dirhtml_cleanup(). Ensure cleanup clears sr->handler_data and frees request->file_list and request->toc before releasing the request, matching mk_http_cb_request_free() lifecycle behavior.lib/monkey/mk_server/mk_http.c-1678-1681 (1)
1678-1681: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConfirm the polarity of this
mk_list_is_emptycheck.
mk_list_is_empty()returns0for an empty list and-1for a populated list. The stated intent is to run the stage-40 hook withsr_fixedwhen the request list is empty.!= 0selects the populated case, which is the opposite. It also runs the hook withsr_fixedwhile real requests exist in the list.Use
== 0if the intent is the empty-list case.Based on learnings:
mk_list_is_empty()returns0for an empty list and-1for a populated list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_server/mk_http.c` around lines 1678 - 1681, Update the mk_list_is_empty check surrounding mk_plugin_stage_run_40 so it uses the empty-list return value, selecting session->sr_fixed only when request_list is empty; preserve the existing stage-40 invocation and populated-list behavior.Source: Learnings
lib/monkey/plugins/tls/tls_openssl.c-106-172 (1)
106-172: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the configuration directory separator handling consistent.
Line 106, Line 134, Line 155, and Line 165 concatenate
confdirand the file name without a separator ("%stls.conf"). Line 140, Line 150, and Line 171 insert a separator ("%s/%s"). Only one of these two assumptions about a trailing slash inconfdircan be correct. Ifconfdirhas no trailing slash, the fallback paths resolve to the wrong location. Ifconfdirhas a trailing slash, the relative-key paths produce a double slash.Normalize
confdironce, then use one format for all paths.🐛 Proposed direction
static int config_parse(const char *confdir, struct tls_config *conf) { long unsigned int len; char *conf_path = NULL; + const char *sep; ... - mk_api->str_build(&conf_path, &len, "%stls.conf", confdir); + sep = (confdir[0] != '\0' && + confdir[strlen(confdir) - 1] == '/') ? "" : "/"; + mk_api->str_build(&conf_path, &len, "%s%stls.conf", confdir, sep);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/plugins/tls/tls_openssl.c` around lines 106 - 172, Normalize confdir once before building any paths, ensuring it has exactly one directory separator at the boundary. Update the tls.conf path and all fallback assignments in the surrounding configuration-loading flow, including cert_file, cert_chain_file, key_file, and dh_param_file, to use the same normalized path format without producing missing or duplicate separators.lib/monkey/plugins/tls/tls_openssl.c-396-411 (1)
396-411: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSend only the bytes that were copied.
Line 408 writes
lenbytes, but the loop at Line 403 copiesusedbytes.lencomes frommk_io->total_len, andusedis the sum of the firstiov_idxentries. If these two values ever differ,SSL_writetransmits uninitialized heap memory to the client.Use
usedas the write length, and bound the copy bylen.🔒 Proposed fix
used = 0; for (i = 0; i < mk_io->iov_idx; i++) { + if (used + mk_io->io[i].iov_len > len) { + break; + } memcpy(buf + used, mk_io->io[i].iov_base, mk_io->io[i].iov_len); used += mk_io->io[i].iov_len; } - ret = SSL_write(ssl, buf, len); + ret = SSL_write(ssl, buf, used); mk_api->mem_free(buf);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/plugins/tls/tls_openssl.c` around lines 396 - 411, Update the TLS write logic around the iovec-copy loop and SSL_write call to copy no more than the allocated len bytes, ensuring used cannot exceed len, and pass used—not len—as the SSL_write length so only initialized copied bytes are transmitted.lib/monkey/plugins/tls/tls_openssl.c-567-571 (1)
567-571: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse the plugin API allocator and unlink the list node.
Line 570 calls
mk_mem_freedirectly. Every other allocation in this file usesmk_api->mem_free. A shared plugin object must resolve allocator symbols throughmk_api. The loop also does not callmk_list_delbefore it frees the node.♻️ Proposed fix
mk_list_foreach_safe(cur, tmp, &server_context->threads) { thctx = mk_list_entry(cur, struct tls_thread_context, _head); contexts_free(thctx->contexts); - mk_mem_free(thctx); + mk_list_del(&thctx->_head); + mk_api->mem_free(thctx); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/plugins/tls/tls_openssl.c` around lines 567 - 571, Update the cleanup loop in the thread-context teardown to unlink each node with mk_list_del before releasing it, and replace direct mk_mem_free with the plugin allocator mk_api->mem_free. Preserve the existing contexts_free cleanup and safe iteration behavior.lib/monkey/mk_server/mk_http_parser.c-326-337 (1)
326-337: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject
Content-LengthwhenTransfer-Encoding: chunkedis present.The parser now accepts
Content-LengthandTransfer-Encodingvalues independently, andmk_http_parser_content_length()returns the chunked body size when chunked encoding is present. RFC 9112 section 6.1 requires rejecting requests with both headers to avoid request-smuggling through different message-length expectations.Also reject duplicate
Content-Lengthheaders instead of letting the later value overwrite the previous one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/mk_server/mk_http_parser.c` around lines 326 - 337, Update the Content-Length handling in mk_http_parser_content_length() to return MK_CLIENT_BAD_REQUEST when Transfer-Encoding: chunked is already present, and reject a second Content-Length header instead of overwriting p->header_content_length. Preserve the existing empty-value and parsing-error checks for the first Content-Length.lib/monkey/plugins/tls/tls_openssl.c-225-233 (1)
225-233: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse the OpenSSL 3 temporary DH API.
PEM_read_bio_DHparams,SSL_CTX_set_tmp_dh, andDH_freeuse the deprecatedDH/PEM DH APIs. Use OpenSSL 3-compatible parameter loading, such asPEM_read_bio_Parameters_ex, andSSL_CTX_set0_tmp_dh_pkey, with a compatible fallback branch if you still support OpenSSL 1.1.x. Also remove the redundantSSL_load_error_strings()afterOPENSSL_init_ssl().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/monkey/plugins/tls/tls_openssl.c` around lines 225 - 233, The temporary DH setup in the TLS initialization flow must use OpenSSL 3 APIs: replace PEM_read_bio_DHparams, SSL_CTX_set_tmp_dh, and DH_free with parameter loading via PEM_read_bio_Parameters_ex and SSL_CTX_set0_tmp_dh_pkey, while retaining an OpenSSL 1.1.x fallback if supported. Also remove the redundant SSL_load_error_strings call following OPENSSL_init_ssl.
| EVBASE_ACQUIRE_LOCK(base, th_base_lock); | ||
| if (activate) | ||
| event_active_nolock_(&eonce->ev, EV_TIMEOUT, 1); | ||
| else | ||
| res = event_add_nolock_(&eonce->ev, tv, 0); | ||
|
|
||
| if (res != 0) { | ||
| mm_free(eonce); | ||
| return (res); | ||
| } else { | ||
| LIST_INSERT_HEAD(&base->once_events, eonce, next_once); | ||
| } | ||
| EVBASE_RELEASE_LOCK(base, th_base_lock); | ||
|
|
||
| return (0); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Release the base lock on the event_add_nolock_ failure path.
Line 2182 acquires th_base_lock. If event_add_nolock_ returns non-zero, the function frees eonce and returns at line 2190 while the lock is still held. The base then stays locked forever, and any later operation on that base deadlocks. Fix the early return to release the lock.
🔒️ Proposed fix
if (res != 0) {
mm_free(eonce);
+ EVBASE_RELEASE_LOCK(base, th_base_lock);
return (res);
} else {
LIST_INSERT_HEAD(&base->once_events, eonce, next_once);
}
EVBASE_RELEASE_LOCK(base, th_base_lock);This code is vendored under lib/. Confirm the change against upstream libevent before you apply it, and keep the patch isolated with a note about the upstream project and path.
As per coding guidelines: "Ask for explicit user confirmation before editing bundled or separately maintained library code under lib/; keep such patches isolated and document their upstream project/path."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| EVBASE_ACQUIRE_LOCK(base, th_base_lock); | |
| if (activate) | |
| event_active_nolock_(&eonce->ev, EV_TIMEOUT, 1); | |
| else | |
| res = event_add_nolock_(&eonce->ev, tv, 0); | |
| if (res != 0) { | |
| mm_free(eonce); | |
| return (res); | |
| } else { | |
| LIST_INSERT_HEAD(&base->once_events, eonce, next_once); | |
| } | |
| EVBASE_RELEASE_LOCK(base, th_base_lock); | |
| return (0); | |
| EVBASE_ACQUIRE_LOCK(base, th_base_lock); | |
| if (activate) | |
| event_active_nolock_(&eonce->ev, EV_TIMEOUT, 1); | |
| else | |
| res = event_add_nolock_(&eonce->ev, tv, 0); | |
| if (res != 0) { | |
| mm_free(eonce); | |
| EVBASE_RELEASE_LOCK(base, th_base_lock); | |
| return (res); | |
| } else { | |
| LIST_INSERT_HEAD(&base->once_events, eonce, next_once); | |
| } | |
| EVBASE_RELEASE_LOCK(base, th_base_lock); | |
| return (0); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/mk_core/deps/libevent/event.c` around lines 2182 - 2196, In the
event_once scheduling flow around event_add_nolock_, release th_base_lock via
EVBASE_RELEASE_LOCK(base, th_base_lock) before freeing eonce and returning on a
nonzero result. Confirm the fix against upstream libevent first, obtain explicit
user confirmation before modifying this vendored library code, and keep the
isolated patch documented with its upstream project and path.
Source: Coding guidelines
| err: | ||
| evrpc_hook_context_free_(store); | ||
| if (meta) { | ||
| mm_free(meta->data); | ||
| mm_free(meta->key); | ||
| mm_free(meta); | ||
| } | ||
| return 1; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
The err: path leaves req->hook_meta dangling and can dereference NULL.
store aliases req->hook_meta (Line 1055-1056). The error path frees store through evrpc_hook_context_free_ but never clears req->hook_meta. Two consequences follow:
evrpc_hook_find_meta(Line 1092) andevrpc_hook_get_connection(Line 1110) then read freed memory.test/regress_rpc.ccallsevrpc_hook_add_metaandevrpc_hook_get_connectionin exactly that order.- If
storealready held metadata from an earlier successful call, a failure while adding a later entry discards all prior entries and leaves the same dangling pointer. Request teardown then double-frees the context.
Separately, when Line 1058 takes goto err because store is NULL, evrpc_hook_context_free_ computes &ctx->meta_data on a NULL pointer at Line 1042.
All three paths require an allocation failure, so normal operation is unaffected. This file is a vendored upstream import. Do not patch it locally. Report the defect to the libevent project and pick up the fix in the next vendor drop.
As per path instructions, "Ask for explicit user confirmation before editing bundled or separately maintained library code under lib/; keep such patches isolated and document their upstream project/path."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/mk_core/deps/libevent/evrpc.c` around lines 1076 - 1083, Do not
modify the vendored libevent code in this change. Report the dangling
req->hook_meta and NULL-handling defects to the libevent project, then schedule
the correction for the next vendor drop; if an update is prepared later, keep it
isolated and document the upstream project and path, requesting explicit
confirmation before editing code under lib/.
Source: Path instructions
| line = evbuffer_readln(input, &nread, EVBUFFER_EOL_CRLF); | ||
| if (!strncmp(line, "HTTP/1.1 401 ", strlen("HTTP/1.1 401 "))) { | ||
| test_ok++; | ||
| } | ||
| if (line) | ||
| free(line); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Null pointer dereference in http_ws_readcb_bad.
Line 340 assigns the result of evbuffer_readln, which returns NULL when the input buffer holds no complete CRLF-terminated line. Line 341 then calls strncmp(line, ...) before the NULL check at line 344.
The read callback fires whenever data arrives. A response split across TCP segments delivers a partial line first, so readln returns NULL and the test process crashes.
Move the NULL check before the comparison.
🐛 Proposed fix
line = evbuffer_readln(input, &nread, EVBUFFER_EOL_CRLF);
- if (!strncmp(line, "HTTP/1.1 401 ", strlen("HTTP/1.1 401 "))) {
- test_ok++;
- }
- if (line)
+ if (!line)
+ return;
+ if (!strncmp(line, "HTTP/1.1 401 ", strlen("HTTP/1.1 401 "))) {
+ test_ok++;
+ }
free(line);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| line = evbuffer_readln(input, &nread, EVBUFFER_EOL_CRLF); | |
| if (!strncmp(line, "HTTP/1.1 401 ", strlen("HTTP/1.1 401 "))) { | |
| test_ok++; | |
| } | |
| if (line) | |
| free(line); | |
| } | |
| line = evbuffer_readln(input, &nread, EVBUFFER_EOL_CRLF); | |
| if (!line) | |
| return; | |
| if (!strncmp(line, "HTTP/1.1 401 ", strlen("HTTP/1.1 401 "))) { | |
| test_ok++; | |
| } | |
| free(line); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/mk_core/deps/libevent/test/regress_ws.c` around lines 340 - 346,
In http_ws_readcb_bad, check whether evbuffer_readln returned a non-NULL line
before passing it to strncmp; only perform the HTTP/1.1 401 comparison and
increment test_ok for a valid line, then free it safely afterward.
| } else if (events & BEV_EVENT_EOF) { | ||
| bufferevent_free(bev); | ||
| if (num_requests == MAX_REQUESTS) { | ||
| event_base_loopbreak(bufferevent_get_base(bev)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Fix the use-after-free of bev in server_event_cb.
Line 99 frees bev. Line 101 then calls bufferevent_get_base(bev) on the freed object. Read the base before the free.
🐛 Proposed fix
} else if (events & BEV_EVENT_EOF) {
+ struct event_base *base = bufferevent_get_base(bev);
bufferevent_free(bev);
if (num_requests == MAX_REQUESTS) {
- event_base_loopbreak(bufferevent_get_base(bev));
+ event_base_loopbreak(base);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (events & BEV_EVENT_EOF) { | |
| bufferevent_free(bev); | |
| if (num_requests == MAX_REQUESTS) { | |
| event_base_loopbreak(bufferevent_get_base(bev)); | |
| } | |
| } else if (events & BEV_EVENT_EOF) { | |
| struct event_base *base = bufferevent_get_base(bev); | |
| bufferevent_free(bev); | |
| if (num_requests == MAX_REQUESTS) { | |
| event_base_loopbreak(base); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/mk_core/deps/libevent/test/test-fdleak.c` around lines 98 - 102,
Update server_event_cb so it obtains and stores the event base from bev before
calling bufferevent_free(bev), then use the stored base in the num_requests ==
MAX_REQUESTS loopbreak path instead of accessing the freed bufferevent.
| num_requests++; | ||
| if (num_requests == MAX_REQUESTS) { | ||
| event_base_loopbreak(base); | ||
| } else { | ||
| if (++num_requests < MAX_REQUESTS) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicate increment of num_requests.
Line 185 increments num_requests. Line 186 increments it again. Each completed request advances the counter by two. The client then starts about half the intended requests. The server check num_requests == MAX_REQUESTS at line 100 can also skip the exact value and never break the loop.
🐛 Proposed fix
num_requests++;
- if (++num_requests < MAX_REQUESTS) {
+ if (num_requests < MAX_REQUESTS) {
start_client(base);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| num_requests++; | |
| if (num_requests == MAX_REQUESTS) { | |
| event_base_loopbreak(base); | |
| } else { | |
| if (++num_requests < MAX_REQUESTS) { | |
| num_requests++; | |
| if (num_requests < MAX_REQUESTS) { | |
| start_client(base); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/mk_core/deps/libevent/test/test-fdleak.c` around lines 185 - 186,
Remove the standalone num_requests++ immediately before the condition, leaving
the increment in the if condition as the sole update per completed request.
Preserve the existing MAX_REQUESTS comparison and request-loop behavior so the
counter can reach the exact limit.
| if (mk_channel_is_empty(&conn->channel) == 0 && | ||
| mk_net_transport_event_interest(conn->net, | ||
| event->fd, | ||
| MK_EVENT_READ) == MK_EVENT_WRITE) { | ||
| ret = mk_sched_event_read(conn, sched, server); | ||
| } | ||
| else { | ||
| ret = mk_sched_event_write(conn, sched, server); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
A combined READ and WRITE event can call the read handler twice.
An event mask can carry both MK_EVENT_WRITE and MK_EVENT_READ. When the new branch at Line 521 runs, Line 528 still matches and calls mk_sched_event_read a second time in the same iteration. If the first call returned -1, the protocol handler may have already removed the session, so the second call operates on freed memory. The previous code always ran the write handler in the write branch, so this path did not exist.
Guard the read branch at Line 528, or return the connection state before it runs again.
🐛 Proposed fix
if (event->mask & MK_EVENT_WRITE) {
MK_TRACE("[FD %i] Event WRITE", event->fd);
if (mk_channel_is_empty(&conn->channel) == 0 &&
mk_net_transport_event_interest(conn->net,
event->fd,
MK_EVENT_READ) == MK_EVENT_WRITE) {
ret = mk_sched_event_read(conn, sched, server);
+ goto check_result;
}
else {
ret = mk_sched_event_write(conn, sched, server);
}
}
- if (event->mask & MK_EVENT_READ) {
+ if (ret != -1 && (event->mask & MK_EVENT_READ)) {
MK_TRACE("[FD %i] Event READ", event->fd);
ret = mk_sched_event_read(conn, sched, server);
}Use one of the two guards, not both.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/mk_server/mk_server.c` around lines 517 - 525, Prevent the
combined READ/WRITE event path from invoking mk_sched_event_read twice in one
iteration. Update the surrounding event-dispatch logic near the shown
mk_sched_event_read/mk_sched_event_write calls so that, after the first read
handling, the later read branch is skipped or the connection state is returned
before it can run again; use only one guard strategy.
| sent = 0; | ||
| remain = file_count; | ||
|
|
||
| do { | ||
| used = pread(file_fd, buf, SENDFILE_BUF_SIZE, *file_offset); | ||
| if (used == 0) { | ||
| ret = 0; | ||
| } | ||
| else if (used < 0) { | ||
| ret = -1; | ||
| } | ||
| else if (remain > 0) { | ||
| ret = SSL_write(ssl, buf, used < remain ? used : remain); | ||
| } | ||
| else { | ||
| ret = SSL_write(ssl, buf, used); | ||
| } | ||
|
|
||
| if (ret > 0) { | ||
| if (remain > 0) { | ||
| remain -= ret; | ||
| } | ||
| sent += ret; | ||
| *file_offset += ret; | ||
| } | ||
| } while (ret > 0); | ||
|
|
||
| mk_api->mem_free(buf); | ||
|
|
||
| if (sent > 0) { | ||
| return sent; | ||
| } | ||
|
|
||
| return tls_handle_return(ssl, ret); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
The send loop sends more bytes than file_count.
remain starts at file_count. When remain reaches 0, the loop does not stop. The next iteration takes the else branch at Line 450 and writes the full used bytes. The server then sends data past the requested length, which corrupts the response body for Content-Length and Range responses.
Also, sent is ssize_t but the function returns int. For a transfer above INT_MAX bytes, the return value overflows.
Stop the loop when remain reaches 0, and clamp the return value.
🐛 Proposed fix
sent = 0;
remain = file_count;
- do {
+ while (remain > 0) {
used = pread(file_fd, buf, SENDFILE_BUF_SIZE, *file_offset);
if (used == 0) {
ret = 0;
+ break;
}
- else if (used < 0) {
+ if (used < 0) {
ret = -1;
+ break;
}
- else if (remain > 0) {
- ret = SSL_write(ssl, buf, used < remain ? used : remain);
- }
- else {
- ret = SSL_write(ssl, buf, used);
- }
+ ret = SSL_write(ssl, buf, used < remain ? used : remain);
- if (ret > 0) {
- if (remain > 0) {
- remain -= ret;
- }
- sent += ret;
- *file_offset += ret;
+ if (ret <= 0) {
+ break;
}
- } while (ret > 0);
+ remain -= ret;
+ sent += ret;
+ *file_offset += ret;
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/plugins/tls/tls_openssl.c` around lines 436 - 469, Update the send
loop around remain and sent so it stops once the requested file_count has been
transmitted and never calls SSL_write with bytes beyond the remaining count.
Preserve partial-write handling and clamp the final sent return to the
function’s int range before returning, including the successful transfer path.
| static mbedtls_ssl_context *context_new(int fd) | ||
| { | ||
| struct polar_thread_context *thctx = local_thread_context(); | ||
| struct polar_context_head **cur = &thctx->contexts; | ||
| mbedtls_ssl_context *ssl = NULL; | ||
| mbedtls_ssl_cache_context cache; | ||
|
|
||
| mbedtls_ssl_cache_init(&cache); | ||
|
|
||
| assert(cur != NULL); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Guard against a NULL thread context and remove the unused cache variable.
context_new dereferences thctx->contexts at line 606 without checking thctx for NULL. local_thread_context() returns NULL for any thread that did not run mk_tls_thread_init. The comment at line 560 states that contexts may be requested from outside workers, and context_get already handles that case. The call path mk_tls_read → context_get (returns NULL when thctx is NULL) → context_new therefore crashes.
The local cache at line 608 is initialized and never used.
🛡️ Proposed fix
static mbedtls_ssl_context *context_new(int fd)
{
struct polar_thread_context *thctx = local_thread_context();
- struct polar_context_head **cur = &thctx->contexts;
+ struct polar_context_head **cur;
mbedtls_ssl_context *ssl = NULL;
- mbedtls_ssl_cache_context cache;
- mbedtls_ssl_cache_init(&cache);
+ if (thctx == NULL) {
+ return NULL;
+ }
- assert(cur != NULL);
+ cur = &thctx->contexts;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static mbedtls_ssl_context *context_new(int fd) | |
| { | |
| struct polar_thread_context *thctx = local_thread_context(); | |
| struct polar_context_head **cur = &thctx->contexts; | |
| mbedtls_ssl_context *ssl = NULL; | |
| mbedtls_ssl_cache_context cache; | |
| mbedtls_ssl_cache_init(&cache); | |
| assert(cur != NULL); | |
| static mbedtls_ssl_context *context_new(int fd) | |
| { | |
| struct polar_thread_context *thctx = local_thread_context(); | |
| struct polar_context_head **cur; | |
| mbedtls_ssl_context *ssl = NULL; | |
| if (thctx == NULL) { | |
| return NULL; | |
| } | |
| cur = &thctx->contexts; | |
| assert(cur != NULL); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/tls/tls_mbedtls.c` around lines 603 - 612, Update context_new to
check the result of local_thread_context() before dereferencing it, returning
the existing failure value when no thread context is available; preserve the
context allocation flow for initialized threads. Remove the unused
mbedtls_ssl_cache_context cache declaration and its mbedtls_ssl_cache_init call.
| static int mk_tls_read(struct mk_plugin *plugin, int fd, void *buf, int count) | ||
| { | ||
| size_t avail; | ||
| mbedtls_ssl_context *ssl = context_get(fd); | ||
|
|
||
| (void) plugin; | ||
|
|
||
| if (!ssl) { | ||
| ssl = context_new(fd); | ||
| } | ||
|
|
||
| int ret = handle_return(ssl, mbedtls_ssl_read(ssl, buf, count)); | ||
| MK_TRACE("IN: %i SSL READ: %i ; CORE COUNT: %i", | ||
| ssl->in_msglen, | ||
| ret, count); | ||
|
|
||
| /* Check if the caller read less than the available data */ | ||
| if (ret > 0) { | ||
| avail = polar_get_bytes_avail(ssl); | ||
| if (avail > 0) { | ||
| /* | ||
| * A read callback would never read in buffer more than | ||
| * the size specified in 'count', but it aims to return | ||
| * as value the total information read in the buffer plugin | ||
| */ | ||
| ret += avail; | ||
| } | ||
| } | ||
| return ret; | ||
| } | ||
|
|
||
| static int mk_tls_write(struct mk_plugin *plugin, int fd, const void *buf, size_t count) | ||
| { | ||
| mbedtls_ssl_context *ssl = context_get(fd); | ||
| (void) plugin; | ||
| if (!ssl) { | ||
| ssl = context_new(fd); | ||
| } | ||
|
|
||
| return handle_return(ssl, mbedtls_ssl_write(ssl, buf, count)); | ||
| } | ||
|
|
||
| static int mk_tls_writev(struct mk_plugin *plugin, int fd, struct mk_iov *mk_io) | ||
| { | ||
| mbedtls_ssl_context *ssl = context_get(fd); | ||
| const int iov_len = mk_io->iov_idx; | ||
| const struct mk_iovec *io = mk_io->io; | ||
| const size_t len = mk_io->total_len; | ||
| unsigned char *buf; | ||
| size_t used = 0; | ||
| int ret = 0, i; | ||
|
|
||
| (void) plugin; | ||
|
|
||
| if (!ssl) { | ||
| ssl = context_new(fd); | ||
| } | ||
|
|
||
| buf = mk_mem_alloc(len); | ||
| if (buf == NULL) { | ||
| mk_err("malloc failed: %s", strerror(errno)); | ||
| return -1; | ||
| } | ||
|
|
||
| for (i = 0; i < iov_len; i++) { | ||
| memcpy(buf + used, io[i].iov_base, io[i].iov_len); | ||
| used += io[i].iov_len; | ||
| } | ||
|
|
||
| assert(used == len); | ||
| ret = mbedtls_ssl_write(ssl, buf, len); | ||
| mk_mem_free(buf); | ||
|
|
||
| return handle_return(ssl, ret); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Check the SSL context for NULL before use in the transport callbacks.
context_new returns NULL when mk_mem_alloc fails, and it returns NULL after the fix for the missing thread context. mk_tls_read, mk_tls_write, and mk_tls_writev pass ssl straight to handle_return and to the mbedTLS calls. handle_return dereferences ssl->p_bio at line 178, so a NULL context crashes the worker. mk_tls_send_file at line 763 has the same gap. The OpenSSL backend in lib/monkey/tls/tls_openssl.c already returns -1 on a NULL context; apply the same pattern here.
🛡️ Proposed fix for `mk_tls_read` (apply the same pattern to write, writev, and send_file)
if (!ssl) {
ssl = context_new(fd);
+ if (!ssl) {
+ return -1;
+ }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/tls/tls_mbedtls.c` around lines 684 - 758, Add an immediate NULL
check after context creation in mk_tls_read, mk_tls_write, mk_tls_writev, and
mk_tls_send_file; return -1 when context_new or context_get yields no SSL
context, before any mbedTLS call or handle_return invocation. Match the existing
OpenSSL backend behavior and leave normal context handling unchanged.
| static const char tls_builtin_key[] = | ||
| "-----BEGIN PRIVATE KEY-----\n" | ||
| "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCpkF9Ks9aKiZao\n" | ||
| "yX+2CgOcAkX+6zkSonRvWfYtrS3m24zsc+co2xGx29nUW3sRSg6uNe1Jpi/8PKZw\n" | ||
| "ucZNXnQiKbab2pyLbWDfchakGeiYrO2EnIemdEYTB9MUUM1LtD/8LevLzKV8OBAa\n" | ||
| "vSHAdvv17OuIK5vgxEDmRF9nlXNrvvNoMxURsOSz0Rtoe/H1uhaLcNkBUjueM5ay\n" | ||
| "pKUFuiUbggTWHtEzBbPN584Wz1ewN/f+hXHOtoLc5cC+zhr2IVV0Z8J8v0T4a+51\n" | ||
| "DJ8FeZ+fG1H+N/Zwyv/EcjajhavMcnE8b+qKgNkWqaArRz6cagQSiA6I2roQVbKs\n" | ||
| "BUsXfWgpAgMBAAECggEADWuazyvKqC5ZmURRclP6kydu6M0vODVZZ9LD9DuHrYTk\n" | ||
| "83X87rPgA6a15+PRqr2kyc8E19ZqZ9lZBwT9F/SI1odcp5s21qYyi5zZA+X1Ddhp\n" | ||
| "+Bv3dIoxXaI555q5lOtQQSJVTk0FL/6z75nWiQghywYUYjOpY7HEvTTeJDGk7/sN\n" | ||
| "Bozc5Bczn5W6z7asKaXt7nC0WwauNMJ18WwKRjJlwOgBhb0/Qj5fqy1IxPI/kPwY\n" | ||
| "eAoIi91ARg/MCkUb4Yh6LTCfkwkElbpEIPr1T+2hYYl/x3wlvOUuEb+eIoZLNZ5x\n" | ||
| "K7kHPBcDEtGyl4yKPV4ltlJ0/ie0lp8pmNmXDNfV8QKBgQDUDx5dn833A1k1JMpm\n" | ||
| "XIjKOtIeaTRT58a2yM/T7de9oKRgC4hgEQ4cXG+R6r8n9/zLm3/m+l8+rH/tdBMl\n" | ||
| "C1Ekn8dL1be5zPpxDJlUSyQcYEJyDo2RHCW6Jgd6hrh73RmeFF5Tr4dFJcVVybZN\n" | ||
| "qAJPX3UcSrgmwrvqsXKkVXjrVQKBgQDMsw8eJzWDGErFLN43iE8UVD2oymgr8pq9\n" | ||
| "RcdoTX/MDHjwqUj0vGoY+IFE/sFXr100kqxH9ao5UNHn6yxed425m3wNP7O1rRqi\n" | ||
| "Nsu4WVsfsJ1J5n1Gbs6ujJ2TSiMG+a3fVr93T+6c+ysxMsWcz3gWZcbs4duRQ3Iw\n" | ||
| "NtM2KOCRhQKBgQDCTgAS5WSB222YBlf2pv8n3fG9r8QkxZEM1r+nfp1ZwaIb5zVU\n" | ||
| "YQw+7GvGlgQFiXL21UrCx9MRyFmHp/4KyW3WUxj34aHw+2LWxyaPWDKEVadMfw00\n" | ||
| "U0g2YrYjjOHpjNP2Rs+PepxFvbAtRSBn03QaamsSO1y1F2W8TE+xSCf96QKBgFlQ\n" | ||
| "a3E5pGSdzcn4iMDsLazuELU8E3XRdejNsHL3FaK/cml3Q4jdSOG6VBT5nvyWXHGa\n" | ||
| "6abALtSxSdUKTKKvQVxR1i+lstC7RdqvU/YMrvDFy+s5sUFxCacpXXutpljdyhqf\n" | ||
| "rAzwCGngQXlG8Og5sej74W7sITRhnEojMcb40PtNAoGAGawnWAi0AkIOVtRMBema\n" | ||
| "QZmW3tdVj798XHCI/8cl0CvsgctDdFmku759j4AUIlrAcn/R+umQwSnPAwwNXGV3\n" | ||
| "spDlSVoSDk7lYS4lVCWYUH+BCxqF+Ytb3IlJlv/FtKxCP4eiD1aCIbm9D/WCwBtf\n" | ||
| "36AgOGpW2UA1O65QO4j7HeU=\n" | ||
| "-----END PRIVATE KEY-----\n"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Remove the embedded private key and do not silently fall back to it.
Lines 105-133 store a complete RSA private key in source. The matching certificate is at lines 84-103. Any deployment that uses this key has no confidentiality or authenticity, because the key is public in the repository. Static analysis also flags this key.
tls_load_credentials at lines 435-443 makes this reachable in production: when SSL_CTX_use_certificate_chain_file fails for any reason, for example a missing or unreadable file, the function loads the built-in credentials and returns 0. The server then starts and serves HTTPS with a known key. A warning is printed, but startup is not blocked.
Fail startup when the configured certificate cannot be loaded. If a development fallback is required, gate it behind an explicit opt-in option and generate the key at runtime instead of committing it.
🔒️ Proposed change to the failure path
static int tls_load_credentials(struct tls_server_context *ctx)
{
if (SSL_CTX_use_certificate_chain_file(ctx->ctx, ctx->config.cert_file) != 1) {
mk_warn("[tls] failed to load certificate chain from %s",
ctx->config.cert_file);
- mk_warn("[tls] using built-in development certificate, please configure CertificateFile/RSAKeyFile for production");
- if (tls_load_builtin_credentials(ctx) != 0) {
- return -1;
- }
- return 0;
+ mk_err("[tls] configure CertificateFile and RSAKeyFile before enabling TLS");
+ return -1;
}🧰 Tools
🪛 Betterleaks (1.7.0)
[high] 106-133: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/monkey/tls/tls_openssl.c` around lines 105 - 133, Remove the
tls_builtin_key and its matching embedded certificate, then update
tls_load_credentials to return failure when SSL_CTX_use_certificate_chain_file
or related configured-credential loading fails. Do not silently load built-in
credentials; if a development fallback is required, require an explicit opt-in
and generate credentials at runtime.
Source: Linters/SAST tools
cosmo0920
left a comment
cosmo0920
left a comment
There was a problem hiding this comment.
Without static linking settings of libevent, Windows tests are failed and introduced dynamic dependency of libevent:
Run dumpbin /dependents .\bin\fluent-bit.exe
dumpbin /dependents .\bin\fluent-bit.exe
shell: C:\Program Files\PowerShell\7\pwsh.EXE -command ". '{0}'"
env:
PATH: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\bin\HostX86\x86;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\VC\VCPackages;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TestWindow;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer;C:\Program Files\Microsoft Visual Studio\18\Enterprise\MSBuild\Current\bin\Roslyn;C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8.1 Tools\;C:\Program Files (x86)\HTML Help Workshop;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Team Tools\DiagnosticsHub\Collector;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\Extensions\Microsoft\CodeCoverage.Console;C:\Program Files (x86)\Windows Kits\10\bin\10.0.26100.0\\x86;C:\Program Files (x86)\Windows Kits\10\bin\\x86;C:\Program Files\Microsoft Visual Studio\18\Enterprise\\MSBuild\Current\Bin\amd64;C:\Windows\Microsoft.NET\Framework\v4.0.30319;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\Tools\;C:\WinFlexBison;C:\ProgramData\Chocolatey\bin;c:/Program Files/Git/cmd;c:/Windows/system32;C:/Windows/System32/WindowsPowerShell/v1.0;$ENV:WIX/bin;C:/Program Files/CMake/bin;C:\vcpkg;;C:\Program Files (x86)\Microsoft Visual Studio\Installer;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\Llvm\x64\bin;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\CommonExtensions\Microsoft\CMake\Ninja;C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\VC\Linux\bin\ConnectionManagerExe;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\vcpkg
CommandPromptType: Native
DevEnvDir: C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\
ExtensionSdkDir: C:\Program Files (x86)\Microsoft SDKs\Windows Kits\10\ExtensionSDKs
EXTERNAL_INCLUDE: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\include;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\ATLMFC\include;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Auxiliary\VS\include;C:\Program Files (x86)\Windows Kits\10\include\10.0.26100.0\ucrt;C:\Program Files (x86)\Windows Kits\10\\include\10.0.26100.0\\um;C:\Program Files (x86)\Windows Kits\10\\include\10.0.26100.0\\shared;C:\Program Files (x86)\Windows Kits\10\\include\10.0.26100.0\\winrt;C:\Program Files (x86)\Windows Kits\10\\include\10.0.26100.0\\cppwinrt;C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8.1\include\um
Framework40Version: v4.0
FrameworkDir: C:\Windows\Microsoft.NET\Framework\
FrameworkDir32: C:\Windows\Microsoft.NET\Framework\
FrameworkVersion: v4.0.30319
FrameworkVersion32: v4.0.30319
FSHARPINSTALLDIR: C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools
HTMLHelpDir: C:\Program Files (x86)\HTML Help Workshop
INCLUDE: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\include;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\ATLMFC\include;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Auxiliary\VS\include;C:\Program Files (x86)\Windows Kits\10\include\10.0.26100.0\ucrt;C:\Program Files (x86)\Windows Kits\10\\include\10.0.26100.0\\um;C:\Program Files (x86)\Windows Kits\10\\include\10.0.26100.0\\shared;C:\Program Files (x86)\Windows Kits\10\\include\10.0.26100.0\\winrt;C:\Program Files (x86)\Windows Kits\10\\include\10.0.26100.0\\cppwinrt;C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8.1\include\um
LIB: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\ATLMFC\lib\x86;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\lib\x86;C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8.1\lib\um\x86;C:\Program Files (x86)\Windows Kits\10\lib\10.0.26100.0\ucrt\x86;C:\Program Files (x86)\Windows Kits\10\\lib\10.0.26100.0\\um\x86
LIBPATH: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\ATLMFC\lib\x86;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\lib\x86;C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\lib\x86\store\references;C:\Program Files (x86)\Windows Kits\10\UnionMetadata\10.0.26100.0;C:\Program Files (x86)\Windows Kits\10\References\10.0.26100.0;C:\Windows\Microsoft.NET\Framework\v4.0.30319
llvmArm64: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\Llvm\ARM64\bin
llvmX64: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\Llvm\x64\bin
NETFXSDKDir: C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8.1\
Platform: x86
UCRTVersion: 10.0.26100.0
UniversalCRTSdkDir: C:\Program Files (x86)\Windows Kits\10\
use_x64_llvm: true
VCIDEInstallDir: C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\IDE\VC\
VCINSTALLDIR: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\
VCPKG_ROOT: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\vcpkg
VCToolsInstallDir: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Tools\MSVC\14.51.36231\
VCToolsRedistDir: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VC\Redist\MSVC\14.51.36231\
VCToolsVersion: 14.51.36231
VisualStudioVersion: 18.0
VS180COMNTOOLS: C:\Program Files\Microsoft Visual Studio\18\Enterprise\Common7\Tools\
VSCMD_ARG_app_plat: Desktop
VSCMD_ARG_HOST_ARCH: x86
VSCMD_ARG_TGT_ARCH: x86
VSCMD_VER: 18.8.2
VSINSTALLDIR: C:\Program Files\Microsoft Visual Studio\18\Enterprise\
VSSDK150INSTALL: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VSSDK
VSSDKINSTALL: C:\Program Files\Microsoft Visual Studio\18\Enterprise\VSSDK
WindowsLibPath: C:\Program Files (x86)\Windows Kits\10\UnionMetadata\10.0.26100.0;C:\Program Files (x86)\Windows Kits\10\References\10.0.26100.0
WindowsSdkBinPath: C:\Program Files (x86)\Windows Kits\10\bin\
WindowsSdkDir: C:\Program Files (x86)\Windows Kits\10\
WindowsSDKLibVersion: 10.0.26100.0\
WindowsSdkVerBinPath: C:\Program Files (x86)\Windows Kits\10\bin\10.0.26100.0\
WindowsSDKVersion: 10.0.26100.0\
WindowsSDK_ExecutablePath_x64: C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8.1 Tools\x64\
WindowsSDK_ExecutablePath_x86: C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8.1 Tools\
__DOTNET_ADD_32BIT: 1
__DOTNET_PREFERRED_BITNESS: 32
__VSCMD_PREINIT_PATH: C:\WinFlexBison;C:\ProgramData\Chocolatey\bin;c:/Program Files/Git/cmd;c:/Windows/system32;C:/Windows/System32/WindowsPowerShell/v1.0;$ENV:WIX/bin;C:/Program Files/CMake/bin;C:\vcpkg;;C:\Program Files (x86)\Microsoft Visual Studio\Installer
Microsoft (R) COFF/PE Dumper Version 14.51.36252.0
Copyright (C) Microsoft Corporation. All rights reserved.
Dump of file .\bin\fluent-bit.exe
File Type: EXECUTABLE IMAGE
Image has the following dependencies:
WS2_32.dll
CRYPT32.dll
bcrypt.dll
SHLWAPI.dll
tdh.dll
ole32.dll
wevtapi.dll
NETAPI32.dll
pdh.dll
Secur32.dll
event.dll
ADVAPI32.dll
IPHLPAPI.DLL
KERNEL32.dll
USER32.dll
OLEAUT32.dll
LINK : warning LNK4078: multiple '.text' sections found with different attributes (C0000040)
Summary
36000 .data
1000 .fptable
32D000 .rdata
4B000 .reloc
1000 .rsrc
1000 .text
82A000 .text
This could be ideal for us.
So, we need to add a setting for setting up static linking of libevent like as:
# Monkey uses its bundled libevent backend on Windows. Link it statically so
# Fluent Bit binaries do not require event.dll at runtime.
if(FLB_SYSTEM_WINDOWS)
set(EVENT__LIBRARY_TYPE STATIC CACHE STRING
"Build Monkey's bundled libevent as a static library" FORCE)
endif()
Monkey's parser fix was merged upstream in monkey/monkey#444 and is being bundled by fluent#12212. This PR was therefore rebased onto fluent#12212's lib-monkey-1.8.9 branch, removing its direct modifications to the vendored Monkey sources. This commit keeps only Fluent Bit's parser-error propagation. The PR temporarily depends on fluent#12212 and must be rebased onto master again after fluent#12212 merges and before this PR is merged. Signed-off-by: kimonus <kimonus@users.noreply.github.com>
Send real HTTP/1 requests containing empty and whitespace-only generic headers and require the accepted request to reach the input callback. Also verify that an invalid empty Upgrade field is not ingested. Use portable socket types, handle partial writes and reads, and reject timeouts or incomplete HTTP status lines so the regression runs on the supported runtime-test platforms. This test is prepared against Monkey 1.8.9 from fluent#12212. The PR remains stacked on that bundle update until fluent#12212 merges, then it will be rebased onto master before merge. Signed-off-by: kimonus <kimonus@users.noreply.github.com>
Add real-server POST coverage matching fluent#12174 for empty and whitespace-only generic fields, and verify that the request body is forwarded successfully. Cover empty Connection and Transfer-Encoding values plus empty and whitespace-only Content-Length fields followed immediately by numeric header or body data. Rejected requests must close or return 400, never hang or forward a payload. These tests were prepared and validated against Monkey 1.8.9 from fluent#12212. The PR will be rebased from that temporary branch onto master after fluent#12212 merges. Signed-off-by: kimonus <kimonus@users.noreply.github.com>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
The Monkey parser fix was merged upstream in monkey/monkey#444 and is bundled in Fluent Bit by #12212 as part of Monkey 1.8.9. Propagate MK_HTTP_PARSER_ERROR to the HTTP server provider so malformed requests are closed instead of resetting the parser and remaining pending. Keep this Fluent Bit-specific error handling separate from the bundled Monkey sources. Signed-off-by: kimonus <kimonus@users.noreply.github.com>
Send real HTTP/1 requests containing empty and whitespace-only generic headers and require the accepted request to reach the input callback. Also verify that an invalid empty Upgrade field is not ingested. Use portable socket types, handle partial writes and reads, and reject timeouts or incomplete HTTP status lines so the regression runs on the supported runtime-test platforms. The test exercises Monkey 1.8.9 now bundled in Fluent Bit by #12212. Signed-off-by: kimonus <kimonus@users.noreply.github.com>
Add real-server POST coverage matching #12174 for empty and whitespace-only generic fields, and verify that the request body is forwarded successfully. Cover empty Connection and Transfer-Encoding values plus empty and whitespace-only Content-Length fields followed immediately by numeric header or body data. Rejected requests must close or return 400, never hang or forward a payload. These tests exercise Monkey 1.8.9 now bundled in Fluent Bit by #12212. Signed-off-by: kimonus <kimonus@users.noreply.github.com>
2 participants
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests