Skip to content

Commit f062f5f

Browse files
fzipiclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 48e561b commit f062f5f

2 files changed

Lines changed: 114 additions & 7 deletions

File tree

‎src/request_body_processor/xml.cc‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include "src/request_body_processor/xml.h"
1717

18+
#include <cctype>
1819
#include <list>
1920
#include <iostream>
2021
#include <string>
@@ -151,6 +152,25 @@ extern "C" {
151152
}
152153
}
153154

155+
156+
/* Build the error text from libxml2's last error on this context, so the
157+
* actual parse failure ("Extra content at the end of the document", a tag
158+
* mismatch, ...) reaches REQBODY_ERROR_MSG and the debug log instead of a
159+
* fixed string. Falls back to "XML: <what>." when libxml2 has no message.
160+
* xmlCtxtGetLastError() keeps working under XML_PARSE_NOERROR. */
161+
static std::string parseError(xmlParserCtxtPtr ctx, const std::string &what) {
162+
const xmlError *e = (ctx != NULL) ? xmlCtxtGetLastError(ctx) : NULL;
163+
if (e == NULL || e->message == NULL || *e->message == '\0') {
164+
return "XML: " + what + ".";
165+
}
166+
std::string detail(e->message);
167+
while (!detail.empty() && isspace(static_cast<unsigned char>(detail.back()))) {
168+
detail.pop_back();
169+
}
170+
return "XML: " + what + ": " + detail + " (line " + std::to_string(e->line)
171+
+ ", column " + std::to_string(e->int2) + ")";
172+
}
173+
154174
XML::XML(Transaction *transaction)
155175
: m_transaction(transaction) {
156176
m_data.doc = NULL;
@@ -287,8 +307,9 @@ bool XML::processChunk(const char *buf, unsigned int size,
287307
xmlParseChunk(m_data.parsing_ctx, buf, size, 0);
288308
m_data.xml_parser_state->parsing_ctx_arg = m_data.parsing_ctx_arg;
289309
if (m_data.parsing_ctx->wellFormed != 1) {
290-
error->assign("XML: Failed to parse document.");
291-
ms_dbg_a(m_transaction, 4, "XML: Failed to parse document.");
310+
error->assign(parseError(m_data.parsing_ctx,
311+
"Failed to parse document"));
312+
ms_dbg_a(m_transaction, 4, *error);
292313
return false;
293314
}
294315
}
@@ -303,8 +324,9 @@ bool XML::processChunk(const char *buf, unsigned int size,
303324
) {
304325
xmlParseChunk(m_data.parsing_ctx_arg, buf, size, 0);
305326
if (m_data.parsing_ctx_arg->wellFormed != 1) {
306-
error->assign("XML: Failed to parse document for ARGS.");
307-
ms_dbg_a(m_transaction, 4, "XML: Failed to parse document for ARGS.");
327+
error->assign(parseError(m_data.parsing_ctx_arg,
328+
"Failed to parse document for ARGS"));
329+
ms_dbg_a(m_transaction, 4, *error);
308330
return false;
309331
}
310332
}
@@ -325,6 +347,11 @@ bool XML::complete(std::string *error) {
325347
/* Preserve the results for our reference. */
326348
m_data.well_formed = m_data.parsing_ctx->wellFormed;
327349
m_data.doc = m_data.parsing_ctx->myDoc;
350+
std::string parse_error;
351+
if (m_data.well_formed != 1) {
352+
parse_error = parseError(m_data.parsing_ctx,
353+
"Failed to parse document");
354+
}
328355

329356
/* Clean up everything else. */
330357
xmlFreeParserCtxt(m_data.parsing_ctx);
@@ -333,8 +360,8 @@ bool XML::complete(std::string *error) {
333360
+ std::to_string(m_data.well_formed) + ").");
334361

335362
if (m_data.well_formed != 1) {
336-
error->assign("XML: Failed to parse document.");
337-
ms_dbg_a(m_transaction, 4, "XML: Failed to parse document.");
363+
error->assign(parse_error);
364+
ms_dbg_a(m_transaction, 4, *error);
338365
return false;
339366
}
340367
}
@@ -352,7 +379,8 @@ bool XML::complete(std::string *error) {
352379
error->assign(m_data.xml_error);
353380
}
354381
else {
355-
error->assign("XML: Failed to parse document for ARGS.");
382+
error->assign(parseError(m_data.parsing_ctx_arg,
383+
"Failed to parse document for ARGS"));
356384
}
357385
xmlFreeParserCtxt(m_data.parsing_ctx_arg);
358386
m_data.parsing_ctx_arg = NULL;

‎test/test-cases/regression/variable-REQBODY_PROCESSOR_ERROR.json‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,5 +140,84 @@
140140
"SecRule REQBODY_PROCESSOR_ERROR \"@contains test\" \"phase:2,id:1,pass,t:trim\"",
141141
"SecRule REQBODY_PROCESSOR_ERROR_MSG \"@contains test\" \"phase:2,id:2,pass,t:trim\""
142142
]
143+
},
144+
{
145+
"enabled": 1,
146+
"version_min": 300000,
147+
"title": "Testing Variables :: REQBODY_ERROR_MSG carries the libxml2 parse error",
148+
"resource": "libxml2",
149+
"client": {
150+
"ip": "200.249.12.31",
151+
"port": 123
152+
},
153+
"server": {
154+
"ip": "200.249.12.31",
155+
"port": 80
156+
},
157+
"request": {
158+
"headers": {
159+
"Host": "localhost",
160+
"User-Agent": "curl/7.38.0",
161+
"Accept": "*/*",
162+
"Content-Type": "text/xml",
163+
"Content-Length": "736"
164+
},
165+
"uri": "/?key=value&key=other_value",
166+
"method": "POST",
167+
"http_version": 1.1,
168+
"body": [
169+
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
170+
"<bookstore>",
171+
"<book category=\"COOKING\">",
172+
"<title lang=\"en\">Everyday Italian</title>",
173+
"<author>Giada De Laurentiis</author>",
174+
"<year>2005</year>",
175+
"<price>30.00</price>",
176+
"</book>",
177+
"<book category=\"CHILDREN\">",
178+
"<title lang=\"en\">Harry Potter</title>",
179+
"<author>J K. Rowling</author>",
180+
"<year>2005</year>",
181+
"<price>29.99</price>",
182+
"</book>",
183+
"<book category=\"WEB\">",
184+
"<title lang=\"en\">XQuery Kick Start</title>",
185+
"<author>James McGovern</author>",
186+
"<author>Per Bothner</author>",
187+
"<author>Kurt Cagle</author>",
188+
"<author>James Linn</author>",
189+
"<author>Vaidyanathan Nagarajan</author>",
190+
"<year>2003</year>",
191+
"<price>49.99</price>",
192+
"</book>",
193+
"<book category=\"WEB\">",
194+
"<title lang=\"en\">Learning XML</title>",
195+
"<author>Erik T. Ray</author>",
196+
"<year>2003</year>",
197+
"<price>39.95</price>",
198+
"</bookasdf>",
199+
"</bookstore>"
200+
]
201+
},
202+
"response": {
203+
"headers": {
204+
"Date": "Mon, 13 Jul 2015 20:02:41 GMT",
205+
"Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT",
206+
"Content-Type": "text/html",
207+
"Content-Length": "8"
208+
},
209+
"body": [
210+
"no need."
211+
]
212+
},
213+
"expected": {
214+
"debug_log": "XML: Failed to parse document: Opening and ending tag mismatch: book line 1 and bookasdf \\(line 1, column [0-9]+\\)",
215+
"http_code": 403
216+
},
217+
"rules": [
218+
"SecRuleEngine On",
219+
"SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500005,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
220+
"SecRule REQBODY_ERROR_MSG \"@contains Opening and ending tag mismatch: book line 1 and bookasdf\" \"phase:2,id:3,deny,status:403\""
221+
]
143222
}
144223
]

0 commit comments

Comments
 (0)