diff --git a/src/request_body_processor/xml.cc b/src/request_body_processor/xml.cc index db3f74d6d..858fd7069 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 ad8a5741c..56d3df775 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\"" + ] } ]