Repository navigation
Internal error codes are bare negative integers without names or messages (v2 and v3) #3655
Description
Summary
Several internal functions signal failure with bare negative integers (-1, -2, ... -19) that have no name and, in some paths, no description. In one case the raw number reaches users in the error message. This issue tracks giving those codes names and messages. It deliberately does not change the public C/C++ API contract; a possible public error enum is a separate discussion.
Findings
1. Multipart Content-Disposition parser leaks raw codes to users (v2 and v3)
parse_content_disposition returns -1 ... -19 (plus -99 in v3 for a caught exception). The only consumer formats the number into the error text, so users see:
Multipart: Invalid Content-Disposition header (-7): ...
in REQBODY_ERROR_MSG and the audit log, with nothing explaining what -7 means.
- v3:
src/request_body_processor/multipart.cc(Multipart::parse_content_disposition, consumer around line 812) - v2:
apache2/msc_multipart.c(same function, consumer around line 371)
2. Driver::parse reports success after a parser exception (v3 only)
src/parser/driver.cc: the function returns res == 0 on the normal path (true = success), but the catch (...) branch does return 1 after appending "Parser exception caught during rule parsing". RulesSet::load only treats 0 as failure, so a parser exception merges zero rules and msc_rules_add returns 0. A connector sees no error and runs with no rules. This is a real bug and should be fixed first, independently of the rest.
3. Lua C functions return -1 (v2 and v3)
m.setvar in v3 src/engine/lua.cc and l_setvar in v2 apache2/msc_lua.c return -1 on bad arguments. A lua_CFunction must return the number of values pushed on the stack, so -1 is not an error code there. These should return 0 (or raise with luaL_error).
4. Generic -1 with the descriptive message commented out (v3 only)
src/utils/msc_tree.cc has six return -1; sites whose apr_psprintf messages from v2 are still commented out, so @ipMatch setup failures carry no reason. The messages exist in v2 and just need to be restored into the error_msg out-param.
5. Inconsistent truthiness in int-returning functions (v3)
Transaction::process*, Driver::addSec*, and others are declared int but return true/false/1/0 interchangeably. Not a bug today, since callers only test for zero, but it makes the conventions hard to read next to the -1 sites. Worth normalising where touched, not as a sweep.
Already fine, listed for completeness: validate_url_encoding returns -1/-2/-3 but its operator switches on them and logs a proper message in both branches.
Scope of this issue
- v3: fix
Driver::parseexception path (own PR) - v3: name the multipart
Content-Dispositioncodes and emit a description instead of the number - v2: same for
msc_multipart.c - v3 + v2: fix Lua
setvarreturn values - v3: restore
msc_tree.ccerror messages
Out of scope
A public libmodsecurity error enum / documented return contract for the msc_* API. Today the public boundary only ever emits -1 plus a text out-param, and the connectors check < 0, so that is a separate design with connector impact.
Activity
fzipi commentedon Oct 12, 2026
Status: #3656 (the
Driver::parseexception path) and #3657 (multipartContent-Dispositionerror enum and messages,m.setvarreturn value, restoredmsc_treemessages) are open againstv3/masterand cover items 1 to 4 for 3.x. The 2.x counterpart for the multipart ladder inmsc_multipart.candl_setvarinmsc_lua.cis still to come as a separate PR againstv2/master.airween commentedon Oct 13, 2026
As I wrote under #3657 here, some of mentioned issues are fixed - I would like to finish those PR's soon.