[lozatto]

Fix critical memory corruption crashes, resolve OpenSSL thread-safety issues, separate HTTP/FTP session caches, and introduce a safer JSON ownership model for Pawn developers.

What was updated

• The ezhttp_option_set_body_from_json() native now requires an explicit EzJSONOwnership tag instead of a raw EzJSON handle.
• Developers must explicitly use ezjson_move() or ezjson_copy() to pass JSON objects to HTTP options. This forces compiler validation (preventing silent handle misuse) and enables the C++ module to safely consume and invalidate the handle upon a move.
• Added strict length bounds validation for Pawn arrays in ReadCallbackData and ezhttp_option_set_user_data to prevent Out-Of-Bounds (OOB) memory reads and massive heap allocations.

What was improved

• Thread Safety: Fixed OpenSSL crashes on Linux by properly registering locking callbacks, preventing SIGSEGV during concurrent HTTPS requests.
• Signals: Enforced CURLOPT_NOSIGNAL across all cURL operations to prevent unpredictable SIGALRM crashes.
• Server Pause Starvation: Prevented unbounded memory growth when the AMXX engine is paused (sv_paused 1) by implementing queue backpressure. Worker threads now block gracefully if the completion queue hits its limit, instead of endlessly hogging RAM.
• Dangling Callbacks: Eliminated FTP Use-After-Free vulnerabilities by explicitly nullifying CURLOPT_READFUNCTION and CURLOPT_WRITEFUNCTION (and their DATA counterparts) immediately after FTP transfers, preventing cURL from executing destroyed stack lambdas.
• Session Cache Leakage: Separated HTTP and FTP session caches by injecting the protocol scheme (https://, ftp://) into the cache key.
• Memory Leaks: Fixed a persistent memory leak in UrlUtils::GetHostByUrl where libcurl host strings were not being freed.
• Shutdown Lifecycle: Refactored the module destruction and worker thread join sequence (CancelAllRequests()) to prevent deadlocks and ensure proper cleanup during plugin unload or map changes.
• Build System: Fixed CMake build failures on Linux by explicitly finding and linking OpenSSL::Crypto.

Test improvements

• Updated ez_http_test.sma to comply with the new ezjson_move() ownership API.
• Removed redundant ezjson_free() calls in the test plugin, as the JSON handle is now inherently consumed and freed by the C++ backend when moved.

Documentation

• easy_http.inc and easy_http_json.inc were updated with the new EzJSONOwnership tag and helper stocks.
• The documentation now makes it explicitly clear that using ezjson_move() transfers the ownership to the HTTP request and invalidates the handle on the Pawn side.

#20

#32

[lozatto]

I was experiencing several server crashes that I traced back to missing validations. I also took the opportunity to address two related issues in the same update.

I have been testing these changes for about a week now, and since implementing them, we have not experienced any of the crashes mentioned previously.

Another issue I fixed was related to cURL. When the server crashed for any reason, EasyHTTP was sometimes unable to recover because there were multiple pending requests during the EasyHTTP module's shutdown/recovery process. After fixing this behavior, I have not experienced the issue again.

[lozatto]

@Polarhigh Fixed a critical SIGBUS crash that occurred during ReHLDS/ReGameDLL updates. The issue was caused by calling the engine API CVarGetFloat to check the trace log setting on every single server frame (StartFrame). The module now reads the locally cached cvar_t value directly, which not only prevents memory corruption crashes on engine mismatches but also significantly improves overall server performance.

[lozatto]

@Polarhigh If you'd like, instead of using EzJSONOwnership, I can change it for you. However, it has been working well as it is.

[Polarhigh]

Sorry for the long wait. Unfortunately, I still have not been able to find time to review this project. I asked AI to prepare a review report. Please take a look. 🙏

---

AI-generated review report

Reviewed commit: 4abf078b9465b45f72e077f2df268178ea06f9c7.

This report combines the current source review with an earlier analysis, with the findings checked against the commit above. It has not received a full maintainer review. The recommendation is to address the regressions below before merging.

Regressions introduced by this PR

1. [P1] The existing JSON native changes ownership semantics for already-compiled plugins

Location: src/module.cpp, lines 241-250.

ezhttp_option_set_body_from_json() now frees the supplied JSON handle. Under the previous contract, it copied the serialized body and left the JSON object owned by the plugin.

Pawn tags are not passed to the native at runtime. Already-compiled .amxx plugins therefore receive the new behavior without detecting the contract change. Reusing the JSON object after setting a request body stops working. More seriously, if its handle number has been recycled, a later ezjson_free() using the old variable can free a different live object.

An immediate second ezjson_free() does not necessarily cause a native double-free because it checks handle validity. Handle recycling is the more important failure scenario.

Suggested change: preserve the existing native's semantics and introduce a separately named native for ownership transfer.

2. [P2] ezjson_move() does not invalidate the caller's variable

Location: easy_http_json.inc, lines 214-216.

The helper takes its argument by value and only casts its tag. After the native consumes the JSON, the caller's variable still contains the freed handle number. The JSON manager reuses these numbers, allowing the stale variable to refer to another document later.

Suggested change: accept the variable by reference, retain the transferred handle, and set the caller's variable to EzInvalid_JSON. This does not replace the compatibility fix in finding 1.

3. [P2] Invalid callback-data length does not stop request dispatch

Location: src/module.cpp, lines 125-128.

For lengths above 1024, ReadCallbackData() logs an error and returns nullptr. Its callers still invoke DispatchRequest(), which interprets that result as no callback data and registers a one-argument forward instead of a two-argument forward.

Consequently, a POST/PUT/DELETE can still be sent after argument validation fails, and a callback expecting data[] can be invoked without that argument.

Suggested change: distinguish an absent data array from a validation failure and return from the native before registering a forward or dispatching the request.

4. [P2] The new user-data size limit is an undocumented compatibility change

Location: src/module.cpp, lines 347-351.

The new 1024-cell limit rejects previously accepted data, but the public include does not document that limit. When ezhttp_option_set_user_data() rejects a length, it leaves the previous data in the options object. If those options are reused, the next request can carry the old data instead of the intended replacement.

Suggested change: define and document the limit and failure semantics, and provide a way to check whether the update succeeded while accounting for API compatibility.

5. [P2] Shutdown now cancels EZH_FORGET_REQUEST transfers

Location: src/EasyHttpModule.cpp, lines 267-272.

ShutdownWithoutCallbacks() now calls CancelAllRequests() on the ForgetRequests instance. The public API describes this mode as completing the request while ignoring its result.

At module shutdown, in-flight transfers are now aborted and pending requests may never be sent. This can lose a final statistics POST, for example. The map-change path still preserves the previous behavior, making the semantics differ between map changes and module shutdown.

Suggested change: retain the distinction between canceling a request and completing it without a callback. If shutdown must have a time limit, explicitly define a grace period and the behavior after it expires.

6. [P2] The trace cvar no longer enables logging

Location: src/module.cpp, line 110.

CVAR_GET_FLOAT() was replaced with a read of cvar_ezhttp_trace.value. Metamod registers its own copy of the cvar_t structure, so the module's original structure does not track subsequent cvar updates. Its value stays zero and ezhttp_trace_log 1 does not enable tracing.

Suggested change: restore the engine API lookup or read through a pointer obtained with CVAR_GET_POINTER().

Claims that need correction or narrower wording

These are not all separate regressions, but they affect whether the proposed fixes address their stated causes.

• OpenSSL locking: the dependency lock pins OpenSSL 3.5.6, while the added locking-callback installation only runs for versions below 1.1.0. It does not provide the claimed fix in the shipped configuration.
• Array bounds: limiting a caller-supplied length to 1024 does not validate the readable source range. An undersized array with an excessive length below that limit can still be overread. The original defect predates this PR, but the claimed fix is incomplete.
• FTP callback cleanup: constructing a cpr::Session around a cached CurlHolder already calls curl_easy_reset(). The proposed clearing of callback options does not establish a fix for the described stale-callback scenario on session reuse, because that reset was already present.
• Pawn ownership tags: a tag mismatch normally produces a compiler warning. The tag does not enforce runtime ownership and cannot protect already-compiled plugins. Implicit tag declaration is not itself a bug.
• Memory bounds: the new backpressure limits the number of completed responses per EasyHttp instance. It does not bound response sizes or the pending-request queue. It is useful, but does not establish a total memory bound for the module.

Relevant pre-existing issues

These should be tracked separately rather than presented as regressions introduced by this PR:

• ezhttp_get_user_data() copies the entire stored array without accepting the destination buffer's capacity, allowing an out-of-bounds write.
• The thread_local CURLU* in UrlUtils is never released with curl_url_cleanup(). New worker threads create additional leaked URL objects.
• The module destructor actively waits for forgotten requests to finish. Without request timeouts, that wait can be unbounded.

The earlier analysis also raised repeated curl_global_cleanup() calls and SIGPIPE handling. Those are not included as confirmed crash findings here: multiple cleanup call sites alone do not establish a crash, and the absence of a handler inside this module does not establish the process-wide signal disposition.

Useful changes worth retaining

• curl_free(host) fixes a real string leak.
• Explicit libcurl global initialization makes its lifecycle clearer; its return value should be checked and cleanup calls should be paired consistently.
• Rejecting negative user-data lengths prevents passing those values into vector::resize().
• Backpressure on completed requests is a useful partial resource-control measure.
• Making the allocated array type consistently use cell is appropriate.

Validation and missing coverage

The review inspected the diff and the related JSON, cpr, Metamod, and shutdown implementations. A separate executable C++ reproduction using the actual JSONMngr confirmed that a freed handle number can be reassigned and that freeing the stale number then destroys the replacement object.

A full build of this PR and HTTP/FTP integration tests were not performed. The PR does not add automated tests for the changed behavior.

Before merging, coverage is needed for old compiled-plugin compatibility, JSON ownership and handle recycling, rejection of invalid callback data without dispatch, EZH_FORGET_REQUEST during shutdown, trace-cvar updates, and worker shutdown with a full completion queue.

[lozatto]

@Polarhigh , thank you for the detailed review report and for pointing out these regressions. I have analyzed all 6 points raised and plan to apply the following fixes to ensure everything works as expected:

• [P1] JSON ownership semantics for compiled plugins: Restore the original behavior of the ezhttp_option_set_body_from_json native so that it only copies the string and does not free the JSON handle. For ownership transfer, I'll introduce a new explicit native named ezhttp_option_set_body_from_json_move.
• [P2] Variable invalidation in ezjson_move(): Modify the ezjson_move stock helper (in the .inc file) to accept the variable by reference (&EzJSON:handle). This way, when the handle is transferred, the caller's original variable will be automatically set to EzInvalid_JSON, preventing unintended reuse.
• [P2] Stop request dispatch on validation failure: Refactor the ReadCallbackData() function to return a bool. If the callback data length is invalid (e.g. > 1024), the dispatch functions (ezhttp_get, ezhttp_post, etc.) will receive a false return and immediately abort execution (return 0) before creating any request or registering forwards.
• [P2] User-Data size limit documentation: Formally document the 1024-cell limit in the easy_http.inc include file and change the return type of the ezhttp_option_set_user_data native to a bool (1 on success, 0 on failure). This ensures the caller will know exactly whether the update succeeded.
• [P2] Shutdown behavior with EZH_FORGET_REQUEST: Remove the CancelAllRequests() call during full module shutdown (ShutdownWithoutCallbacks()) for forgettable requests. This ensures that a module shutdown mirrors a map change, allowing these requests to gracefully complete in the background (crucial for things like end-of-map statistics).
• [P2] Trace CVAR logging failure: Modify the RefreshTraceLogSetting() function to use the engine API CVAR_GET_FLOAT("ezhttp_trace_log") directly. This resolves the desync issue caused by Metamod shadowing the local cvar_t structure.

Are you okay with this approach to resolve the pending issues before we merge?