Skip to content

Commit 9c29515

Browse files
Easton97-JensEaston97-Jens
authored
Merge pull request #77 from Easton97-Jens/codex/bearbeite-critical-sonar-befunde-in-pr-#3540
Address open CRITICAL Sonar nesting and cognitive-complexity findings
2 parents 7c4f24a + 8197d2b commit 9c29515

File tree

3 files changed

+186
-125
lines changed

3 files changed

+186
-125
lines changed

‎src/request_body_processor/json_backend_jsoncons.cc‎

Lines changed: 157 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -89,52 +89,84 @@ bool isDigit(char value) {
8989
return std::isdigit(static_cast<unsigned char>(value)) != 0;
9090
}
9191

92-
bool isValidJsonNumber(std::string_view token) {
93-
std::size_t index = 0;
92+
bool isExponentMarker(char value) {
93+
return value == 'e' || value == 'E';
94+
}
95+
96+
bool isSignedExponent(std::string_view token, std::size_t index) {
97+
return index < token.size() && (token[index] == '+' || token[index] == '-');
98+
}
99+
100+
bool parseIntegerPart(std::string_view token, std::size_t *index) {
101+
if (token[*index] == '0') {
102+
(*index)++;
103+
return true;
104+
}
105+
106+
if (!isDigit(token[*index]) || token[*index] == '0') {
107+
return false;
108+
}
109+
110+
while (*index < token.size() && isDigit(token[*index])) {
111+
(*index)++;
112+
}
113+
return true;
114+
}
115+
116+
bool parseFractionPart(std::string_view token, std::size_t *index) {
117+
if (*index >= token.size() || token[*index] != '.') {
118+
return true;
119+
}
94120

121+
(*index)++;
122+
if (*index == token.size() || !isDigit(token[*index])) {
123+
return false;
124+
}
125+
while (*index < token.size() && isDigit(token[*index])) {
126+
(*index)++;
127+
}
128+
return true;
129+
}
130+
131+
bool parseExponentPart(std::string_view token, std::size_t *index) {
132+
if (*index >= token.size() || !isExponentMarker(token[*index])) {
133+
return true;
134+
}
135+
136+
(*index)++;
137+
if (isSignedExponent(token, *index)) {
138+
(*index)++;
139+
}
140+
if (*index == token.size() || !isDigit(token[*index])) {
141+
return false;
142+
}
143+
while (*index < token.size() && isDigit(token[*index])) {
144+
(*index)++;
145+
}
146+
return true;
147+
}
148+
149+
bool isValidJsonNumber(std::string_view token) {
95150
if (token.empty()) {
96151
return false;
97152
}
98153

154+
std::size_t index = 0;
99155
if (token[index] == '-') {
100156
index++;
101157
if (index == token.size()) {
102158
return false;
103159
}
104160
}
105161

106-
if (token[index] == '0') {
107-
index++;
108-
} else {
109-
if (!isDigit(token[index]) || token[index] == '0') {
110-
return false;
111-
}
112-
while (index < token.size() && isDigit(token[index])) {
113-
index++;
114-
}
162+
if (!parseIntegerPart(token, &index)) {
163+
return false;
115164
}
116-
117-
if (index < token.size() && token[index] == '.') {
118-
index++;
119-
if (index == token.size() || !isDigit(token[index])) {
120-
return false;
121-
}
122-
while (index < token.size() && isDigit(token[index])) {
123-
index++;
124-
}
165+
if (!parseFractionPart(token, &index)) {
166+
return false;
125167
}
126-
127-
if (index < token.size() && (token[index] == 'e' || token[index] == 'E')) {
128-
index++;
129-
if (index < token.size() && (token[index] == '+' || token[index] == '-')) {
130-
index++;
131-
}
132-
if (index == token.size() || !isDigit(token[index])) {
133-
return false;
134-
}
135-
while (index < token.size() && isDigit(token[index])) {
136-
index++;
137-
}
168+
if (!parseExponentPart(token, &index)) {
169+
return false;
138170
}
139171

140172
return index == token.size();
@@ -272,6 +304,13 @@ class RawJsonTokenCursor {
272304
}
273305

274306
private:
307+
static bool setError(std::string *detail, const char *message) {
308+
if (detail != nullptr) {
309+
*detail = message;
310+
}
311+
return false;
312+
}
313+
275314
static bool isWhitespace(char value) {
276315
return std::isspace(static_cast<unsigned char>(value)) != 0;
277316
}
@@ -350,38 +389,18 @@ class RawJsonTokenCursor {
350389
const std::size_t start = *offset;
351390

352391
if (*offset >= m_input.size() || m_input[*offset] != '"') {
353-
if (detail != nullptr) {
354-
*detail = "Expected raw JSON string token while synchronizing jsoncons events.";
355-
}
356-
return false;
392+
return setError(detail,
393+
"Expected raw JSON string token while synchronizing jsoncons events.");
357394
}
358395

359396
(*offset)++;
360397
while (*offset < m_input.size()) {
361398
char current = m_input[*offset];
362399
(*offset)++;
363400
if (current == '\\') {
364-
if (*offset >= m_input.size()) {
365-
if (detail != nullptr) {
366-
*detail = "Truncated escape sequence while synchronizing raw JSON string token.";
367-
}
401+
if (!consumeEscapedCharacter(offset, detail)) {
368402
return false;
369403
}
370-
371-
char escaped = m_input[*offset];
372-
(*offset)++;
373-
if (escaped == 'u') {
374-
for (int i = 0; i < 4; i++) {
375-
if (*offset >= m_input.size()
376-
|| !isHexDigit(m_input[*offset])) {
377-
if (detail != nullptr) {
378-
*detail = "Invalid Unicode escape while synchronizing raw JSON string token.";
379-
}
380-
return false;
381-
}
382-
(*offset)++;
383-
}
384-
}
385404
continue;
386405
}
387406

@@ -392,17 +411,13 @@ class RawJsonTokenCursor {
392411
}
393412

394413
if (static_cast<unsigned char>(current) < 0x20) {
395-
if (detail != nullptr) {
396-
*detail = "Unexpected control character while synchronizing raw JSON string token.";
397-
}
398-
return false;
414+
return setError(detail,
415+
"Unexpected control character while synchronizing raw JSON string token.");
399416
}
400417
}
401418

402-
if (detail != nullptr) {
403-
*detail = "Unterminated string token while synchronizing jsoncons events.";
404-
}
405-
return false;
419+
return setError(detail,
420+
"Unterminated string token while synchronizing jsoncons events.");
406421
}
407422

408423
bool consumeNumber(std::string_view *raw_token, std::string *detail) {
@@ -413,63 +428,103 @@ class RawJsonTokenCursor {
413428
std::string *detail) const {
414429
const std::size_t start = *offset;
415430

431+
if (!consumeNumberSign(offset)
432+
|| !consumeIntegerComponent(offset, detail)
433+
|| !consumeFractionComponent(offset, detail)
434+
|| !consumeExponentComponent(offset, detail)) {
435+
return false;
436+
}
437+
438+
*raw_token = std::string_view(m_input.data() + start, *offset - start);
439+
return true;
440+
}
441+
442+
bool consumeEscapedCharacter(std::size_t *offset, std::string *detail) const {
443+
if (*offset >= m_input.size()) {
444+
return setError(detail,
445+
"Truncated escape sequence while synchronizing raw JSON string token.");
446+
}
447+
448+
const char escaped = m_input[*offset];
449+
(*offset)++;
450+
if (escaped != 'u') {
451+
return true;
452+
}
453+
454+
for (int i = 0; i < 4; i++) {
455+
if (*offset >= m_input.size() || !isHexDigit(m_input[*offset])) {
456+
return setError(detail,
457+
"Invalid Unicode escape while synchronizing raw JSON string token.");
458+
}
459+
(*offset)++;
460+
}
461+
return true;
462+
}
463+
464+
bool consumeNumberSign(std::size_t *offset) const {
416465
if (*offset < m_input.size() && m_input[*offset] == '-') {
417466
(*offset)++;
418467
}
468+
return true;
469+
}
419470

471+
bool consumeIntegerComponent(std::size_t *offset, std::string *detail) const {
420472
if (*offset >= m_input.size()) {
421-
if (detail != nullptr) {
422-
*detail = "Unexpected end of input while synchronizing raw JSON number token.";
423-
}
424-
return false;
473+
return setError(detail,
474+
"Unexpected end of input while synchronizing raw JSON number token.");
425475
}
426476

427477
if (m_input[*offset] == '0') {
428478
(*offset)++;
429-
} else {
430-
if (!isDigit(m_input[*offset]) || m_input[*offset] == '0') {
431-
if (detail != nullptr) {
432-
*detail = "Invalid integer component while synchronizing raw JSON number token.";
433-
}
434-
return false;
435-
}
436-
while (*offset < m_input.size() && isDigit(m_input[*offset])) {
437-
(*offset)++;
438-
}
479+
return true;
480+
}
481+
482+
if (!isDigit(m_input[*offset]) || m_input[*offset] == '0') {
483+
return setError(detail,
484+
"Invalid integer component while synchronizing raw JSON number token.");
439485
}
440486

441-
if (*offset < m_input.size() && m_input[*offset] == '.') {
487+
while (*offset < m_input.size() && isDigit(m_input[*offset])) {
442488
(*offset)++;
443-
if (*offset >= m_input.size() || !isDigit(m_input[*offset])) {
444-
if (detail != nullptr) {
445-
*detail = "Invalid fraction component while synchronizing raw JSON number token.";
446-
}
447-
return false;
448-
}
449-
while (*offset < m_input.size() && isDigit(m_input[*offset])) {
450-
(*offset)++;
451-
}
489+
}
490+
return true;
491+
}
492+
493+
bool consumeFractionComponent(std::size_t *offset, std::string *detail) const {
494+
if (*offset >= m_input.size() || m_input[*offset] != '.') {
495+
return true;
496+
}
497+
498+
(*offset)++;
499+
if (*offset >= m_input.size() || !isDigit(m_input[*offset])) {
500+
return setError(detail,
501+
"Invalid fraction component while synchronizing raw JSON number token.");
452502
}
453503

504+
while (*offset < m_input.size() && isDigit(m_input[*offset])) {
505+
(*offset)++;
506+
}
507+
return true;
508+
}
509+
510+
bool consumeExponentComponent(std::size_t *offset, std::string *detail) const {
511+
if (*offset >= m_input.size() || !isExponentMarker(m_input[*offset])) {
512+
return true;
513+
}
514+
515+
(*offset)++;
454516
if (*offset < m_input.size()
455-
&& (m_input[*offset] == 'e' || m_input[*offset] == 'E')) {
517+
&& (m_input[*offset] == '+' || m_input[*offset] == '-')) {
456518
(*offset)++;
457-
if (*offset < m_input.size()
458-
&& (m_input[*offset] == '+' || m_input[*offset] == '-')) {
459-
(*offset)++;
460-
}
461-
if (*offset >= m_input.size() || !isDigit(m_input[*offset])) {
462-
if (detail != nullptr) {
463-
*detail = "Invalid exponent component while synchronizing raw JSON number token.";
464-
}
465-
return false;
466-
}
467-
while (*offset < m_input.size() && isDigit(m_input[*offset])) {
468-
(*offset)++;
469-
}
519+
}
520+
if (*offset >= m_input.size() || !isDigit(m_input[*offset])) {
521+
return setError(detail,
522+
"Invalid exponent component while synchronizing raw JSON number token.");
470523
}
471524

472-
*raw_token = std::string_view(m_input.data() + start, *offset - start);
525+
while (*offset < m_input.size() && isDigit(m_input[*offset])) {
526+
(*offset)++;
527+
}
473528
return true;
474529
}
475530

‎src/request_body_processor/xml.cc‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@
2626
namespace modsecurity::RequestBodyProcessor {
2727

2828
#ifdef WITH_LIBXML2
29+
namespace {
30+
bool finalizeArgsParsingContext(xml_data *data, std::string *error) {
31+
if (xmlParseChunk(data->parsing_ctx_arg, nullptr, 0, 1) == 0) {
32+
xmlFreeParserCtxt(data->parsing_ctx_arg);
33+
data->parsing_ctx_arg = nullptr;
34+
return true;
35+
}
36+
37+
if (!data->xml_error.empty()) {
38+
error->assign(data->xml_error);
39+
} else {
40+
error->assign("XML: Failed to parse document for ARGS.");
41+
}
42+
xmlFreeParserCtxt(data->parsing_ctx_arg);
43+
data->parsing_ctx_arg = nullptr;
44+
return false;
45+
}
46+
} // namespace
2947

3048
/*
3149
* NodeData for parsing XML into args
@@ -310,19 +328,9 @@ bool XML::complete(std::string *error) {
310328
== RulesSetProperties::TrueConfigXMLParseXmlIntoArgs)
311329
) {
312330
/* This is how we signale the end of parsing to libxml. */
313-
if (xmlParseChunk(m_data.parsing_ctx_arg, nullptr, 0, 1) != 0) {
314-
if (!m_data.xml_error.empty()) {
315-
error->assign(m_data.xml_error);
316-
}
317-
else {
318-
error->assign("XML: Failed to parse document for ARGS.");
319-
}
320-
xmlFreeParserCtxt(m_data.parsing_ctx_arg);
321-
m_data.parsing_ctx_arg = nullptr;
331+
if (!finalizeArgsParsingContext(&m_data, error)) {
322332
return false;
323333
}
324-
xmlFreeParserCtxt(m_data.parsing_ctx_arg);
325-
m_data.parsing_ctx_arg = nullptr;
326334
}
327335
}
328336

0 commit comments

Comments
 (0)