From f062f5fd8ab635ac168564b5f893bc66c2650d96 Mon Sep 17 00:00:00 2001 From: Felipe Zipitria Date: Tue, 13 Oct 2026 09:58:27 -0300 Subject: [PATCH] feat: surface libxml2's parse error in REQBODY_ERROR_MSG When the XML body processor fails, REQBODY_ERROR_MSG and the debug log only said "XML: Failed to parse document." libxml2 knows the actual reason (tag mismatch, extra content, empty document, ...) with line and column, and until 3.0.15 that text leaked to stderr by accident. Read xmlCtxtGetLastError() after a failed xmlParseChunk() and build the message from it: XML: Failed to parse document: Opening and ending tag mismatch: book line 1 and bookasdf (line 1, column 725) The fixed text stays as the fallback when libxml2 has no message. The same applies to the ARGS parsing context. xmlCtxtGetLastError() is available and non-deprecated on every libxml2 we build against (checked 2.9.13 and 2.15.4) and keeps working under XML_PARSE_NOERROR, so no version conditionals are needed. Adds a regression case that matches REQBODY_ERROR_MSG on the libxml2 text and asserts the debug log line. Fixes #3678 Co-Authored-By: Claude Fable 5.1 --- src/request_body_processor/xml.cc | 42 ++++++++-- .../variable-REQBODY_PROCESSOR_ERROR.json | 79 +++++++++++++++++++ 2 files changed, 114 insertions(+), 7 deletions(-) diff --git a/src/request_body_processor/xml.cc b/src/request_body_processor/xml.cc index db3f74d6d1..858fd7069f 100644 --- a/src/request_body_processor/xml.cc +++ b/src/request_body_processor/xml.cc @@ -15,6 +15,7 @@ #include "src/request_body_processor/xml.h" +#include #include #include #include @@ -151,6 +152,25 @@ extern "C" { } } + +/* Build the error text from libxml2's last error on this context, so the + * actual parse failure ("Extra content at the end of the document", a tag + * mismatch, ...) reaches REQBODY_ERROR_MSG and the debug log instead of a + * fixed string. Falls back to "XML: ." when libxml2 has no message. + * xmlCtxtGetLastError() keeps working under XML_PARSE_NOERROR. */ +static std::string parseError(xmlParserCtxtPtr ctx, const std::string &what) { + const xmlError *e = (ctx != NULL) ? xmlCtxtGetLastError(ctx) : NULL; + if (e == NULL || e->message == NULL || *e->message == '\0') { + return "XML: " + what + "."; + } + std::string detail(e->message); + while (!detail.empty() && isspace(static_cast(detail.back()))) { + detail.pop_back(); + } + return "XML: " + what + ": " + detail + " (line " + std::to_string(e->line) + + ", column " + std::to_string(e->int2) + ")"; +} + XML::XML(Transaction *transaction) : m_transaction(transaction) { m_data.doc = NULL; @@ -287,8 +307,9 @@ bool XML::processChunk(const char *buf, unsigned int size, xmlParseChunk(m_data.parsing_ctx, buf, size, 0); m_data.xml_parser_state->parsing_ctx_arg = m_data.parsing_ctx_arg; if (m_data.parsing_ctx->wellFormed != 1) { - error->assign("XML: Failed to parse document."); - ms_dbg_a(m_transaction, 4, "XML: Failed to parse document."); + error->assign(parseError(m_data.parsing_ctx, + "Failed to parse document")); + ms_dbg_a(m_transaction, 4, *error); return false; } } @@ -303,8 +324,9 @@ bool XML::processChunk(const char *buf, unsigned int size, ) { xmlParseChunk(m_data.parsing_ctx_arg, buf, size, 0); if (m_data.parsing_ctx_arg->wellFormed != 1) { - error->assign("XML: Failed to parse document for ARGS."); - ms_dbg_a(m_transaction, 4, "XML: Failed to parse document for ARGS."); + error->assign(parseError(m_data.parsing_ctx_arg, + "Failed to parse document for ARGS")); + ms_dbg_a(m_transaction, 4, *error); return false; } } @@ -325,6 +347,11 @@ bool XML::complete(std::string *error) { /* Preserve the results for our reference. */ m_data.well_formed = m_data.parsing_ctx->wellFormed; m_data.doc = m_data.parsing_ctx->myDoc; + std::string parse_error; + if (m_data.well_formed != 1) { + parse_error = parseError(m_data.parsing_ctx, + "Failed to parse document"); + } /* Clean up everything else. */ xmlFreeParserCtxt(m_data.parsing_ctx); @@ -333,8 +360,8 @@ bool XML::complete(std::string *error) { + std::to_string(m_data.well_formed) + ")."); if (m_data.well_formed != 1) { - error->assign("XML: Failed to parse document."); - ms_dbg_a(m_transaction, 4, "XML: Failed to parse document."); + error->assign(parse_error); + ms_dbg_a(m_transaction, 4, *error); return false; } } @@ -352,7 +379,8 @@ bool XML::complete(std::string *error) { error->assign(m_data.xml_error); } else { - error->assign("XML: Failed to parse document for ARGS."); + error->assign(parseError(m_data.parsing_ctx_arg, + "Failed to parse document for ARGS")); } xmlFreeParserCtxt(m_data.parsing_ctx_arg); m_data.parsing_ctx_arg = NULL; diff --git a/test/test-cases/regression/variable-REQBODY_PROCESSOR_ERROR.json b/test/test-cases/regression/variable-REQBODY_PROCESSOR_ERROR.json index ad8a5741c5..56d3df775f 100644 --- a/test/test-cases/regression/variable-REQBODY_PROCESSOR_ERROR.json +++ b/test/test-cases/regression/variable-REQBODY_PROCESSOR_ERROR.json @@ -140,5 +140,84 @@ "SecRule REQBODY_PROCESSOR_ERROR \"@contains test\" \"phase:2,id:1,pass,t:trim\"", "SecRule REQBODY_PROCESSOR_ERROR_MSG \"@contains test\" \"phase:2,id:2,pass,t:trim\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: REQBODY_ERROR_MSG carries the libxml2 parse error", + "resource": "libxml2", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Type": "text/xml", + "Content-Length": "736" + }, + "uri": "/?key=value&key=other_value", + "method": "POST", + "http_version": 1.1, + "body": [ + "", + "", + "", + "Everyday Italian", + "Giada De Laurentiis", + "2005", + "30.00", + "", + "", + "Harry Potter", + "J K. Rowling", + "2005", + "29.99", + "", + "", + "XQuery Kick Start", + "James McGovern", + "Per Bothner", + "Kurt Cagle", + "James Linn", + "Vaidyanathan Nagarajan", + "2003", + "49.99", + "", + "", + "Learning XML", + "Erik T. Ray", + "2003", + "39.95", + "", + "" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "XML: Failed to parse document: Opening and ending tag mismatch: book line 1 and bookasdf \\(line 1, column [0-9]+\\)", + "http_code": 403 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500005,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"", + "SecRule REQBODY_ERROR_MSG \"@contains Opening and ending tag mismatch: book line 1 and bookasdf\" \"phase:2,id:3,deny,status:403\"" + ] } ]