V3/do not add newline to test body lines by hnakamur · Pull Request #3486 · owasp-modsecurity/ModSecurity

hnakamur · 2026-01-25T23:03:54Z

what

Stop adding newline to each line of request or response body in regression tests.
Add format subcommand to regression_tests command.
Support updating Content-Length header by running (cd test; UPDATE_CONTENT_LENGTH=1 ./regression_tests format)
Update regression test JSON files:
- Add newlines to request bodies where needed.
- Add Content-Length headers.
- Adjust test casses to pass after above modifications.

why

Enable to test edge cases (such as request body lines with newline except the last line). This is important for multipart request bodies.

references

Refine SecRequestBodyNoFilesLimit test cases to cover edge scenarios #3479

This is needed to test edge cases like a body missing the final newline, for example: [ "a\r\n", "b\r" ] And it is clearer than to add implicit newline to each line except the final line.

when UPDATE_CONTENT_LENGTH env var is set

by running the following command: ``` (cd test; ./regression_tests format) ```

by running the following command: ``` (cd test; UPDATE_CONTENT_LENGTH=1 ./regression_tests format) ```

hnakamur · 2026-01-26T00:01:16Z

I found JSON was corrupted since some of http_version was empty. I fixed it and force pushed.
However some of tests do not pass right now. I am trying to fix them.

hnakamur · 2026-01-26T00:15:01Z

I fixed tests at aefd4a4 and now all tests pass again.

airween · 2026-01-27T19:30:06Z

Hi @hnakamur,

thank you for this improved PR.

Unfortunately there are two warnings from cppcheck:

2026-01-26T00:29:37.2162370Z warning: test/regression/regression.cc,444,style,shadowVariable,Local variable 'test' shadows outer variable
2026-01-26T00:29:37.2943530Z warning: test/regression/regression_test.cc,366,style,functionConst,The member function 'modsecurity_test::RegressionTests::toJSON' can be made a const function. Making this function 'const' should not cause compiler errors. Even though the function can be made const function technically it may not make sense conceptually. Think about your design and the task of the function first - is it a function that must not change object internal state?

Also, SonarCloud reported a few new issues (some of them are same as cppcheck found):

https://sonarcloud.io/project/issues?id=owasp-modsecurity_ModSecurity&pullRequest=3486&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true

As I mentioned previously, I don't expect to fix all Sonar issues. If the cppcheck will be passed, I'll review all Sonar issues and try to mark the "old" issues as accepted.

Thank you again.

sonarqubecloud · 2026-01-28T14:43:59Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

This PR updates the regression-test JSON format to preserve request/response body line endings (no implicit newline per line), adds a format subcommand to reformat/normalize test JSON files, and introduces tooling to auto-update Content-Length headers during formatting.

Changes:

Added a format subcommand to the regression test runner and optional UPDATE_CONTENT_LENGTH behavior.
Updated regression-test parsing structures (e.g., std::unique_ptr, std::optional) and introduced a RegressionTests wrapper with JSON serialization.
Mass-updated regression test JSON files to use line-array bodies, normalize escaping, and add Content-Length headers.

Reviewed changes

Copilot reviewed 117 out of 202 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
test/test-cases/regression/transformations.json	Reformats request/response bodies as arrays; normalizes headers and adds Content-Length
test/test-cases/regression/transformation-none.json	Reformats request/response bodies as arrays; normalizes headers and adds Content-Length
test/test-cases/regression/secruleengine.json	Adds explicit client/server/request/response blocks for formatting/schema consistency
test/test-cases/regression/secmarker.json	Reformats request/response bodies as arrays; normalizes headers and adds Content-Length
test/test-cases/regression/secargumentslimit.json	Adds Content-Length and response skeletons to align with new formatting
test/test-cases/regression/secaction.json	Reformats request/response bodies as arrays; normalizes headers and adds Content-Length
test/test-cases/regression/sec_component_signature.json	Normalizes escaping and adds Content-Length headers
test/test-cases/regression/rule-920274.json	Adds Content-Length and moves to body-as-array representation
test/test-cases/regression/rule-920200.json	Adds Content-Length and moves to body-as-array representation
test/test-cases/regression/rule-920120.json	Updates multipart body line endings and Content-Length
test/test-cases/regression/request-body-parser-multipart-crlf.json	Updates multipart body line endings and Content-Length
test/test-cases/regression/operator-verifysvnr.json	Updates Content-Length values and expected http_code
test/test-cases/regression/operator-verifyssn.json	Updates Content-Length values and expected http_code
test/test-cases/regression/operator-verifycpf.json	Updates Content-Length values and expected http_code
test/test-cases/regression/operator-verifycc.json	Updates Content-Length values and expected http_code
test/test-cases/regression/operator-validate-byte-range.json	Adds Content-Length, changes empty body representation, adds expected http_code
test/test-cases/regression/operator-rxGlobal.json	Adds Content-Length, appends expected http_code
test/test-cases/regression/operator-rx.json	Normalizes headers/bodies, adds Content-Length/expected http_code across cases
test/test-cases/regression/operator-pmfromfile.json	Normalizes escaping and switches response to explicit headers/body
test/test-cases/regression/operator-pm.json	Normalizes escaping, adds Content-Length and explicit empty response body
test/test-cases/regression/operator-ipMatchFromFile.json	Normalizes paths/headers, adds Content-Length and expected http_code
test/test-cases/regression/operator-detectxss.json	Normalizes body escaping, updates Content-Length and expected http_code
test/test-cases/regression/operator-detectsqli.json	Updates Content-Length and expected http_code
test/test-cases/regression/operator-UnconditionalMatch.json	Adds Content-Length and expected http_code
test/test-cases/regression/misc.json	Adds explicit client/server/request/response blocks and expected http_code
test/test-cases/regression/misc-variable-under-quotes.json	Adds Content-Length/body arrays and expected http_code
test/test-cases/regression/issue-960.json	Fixes issue metadata fields and normalizes request/response schema
test/test-cases/regression/issue-849.json	Normalizes escaping and adds Content-Length/body arrays
test/test-cases/regression/issue-394.json	Normalizes request/response schema and expected fields
test/test-cases/regression/issue-3340.json	Normalizes escaping and response structure
test/test-cases/regression/issue-2427.json	Updates multipart body-to-lines representation and Content-Length
test/test-cases/regression/issue-2423-msg-in-chain.json	Adds explicit request/response blocks and normalizes expected fields
test/test-cases/regression/issue-2196.json	Adds explicit request/response blocks and normalizes expected fields
test/test-cases/regression/issue-2111.json	Adds explicit request/response blocks and normalizes expected fields
test/test-cases/regression/issue-2000.json	Adds explicit request/response blocks and normalizes expected fields
test/test-cases/regression/issue-1960.json	Adds Content-Length/body arrays and normalizes headers
test/test-cases/regression/issue-1943.json	Normalizes escaping, adds Content-Length/body arrays
test/test-cases/regression/issue-1850.json	Normalizes escaping and adds Content-Length/body arrays
test/test-cases/regression/issue-1844.json	Updates Content-Length and response headers
test/test-cases/regression/issue-1812.json	Normalizes escaping and adds expected http_code
test/test-cases/regression/issue-1785.json	Normalizes escaping and adds request/response schema
test/test-cases/regression/issue-1743.json	Normalizes escaping and adds request/response schema
test/test-cases/regression/issue-1725.json	Normalizes issue metadata and request/response schema
test/test-cases/regression/issue-1591.json	Normalizes issue metadata and request/response schema
test/test-cases/regression/issue-1576.json	Normalizes issue metadata; adds server/response blocks and Content-Length headers
test/test-cases/regression/issue-1565.json	Normalizes issue metadata and request/response schema
test/test-cases/regression/issue-1528.json	Normalizes issue metadata and request/response schema
test/test-cases/regression/fn-setHostname.json	Adds Content-Length/body array and normalizes client/server fields
test/test-cases/regression/debug_log.json	Normalizes escaping and adds Content-Length/body arrays
test/test-cases/regression/config-xml_external_entity.json	Normalizes resource formatting and adds server/response blocks
test/test-cases/regression/config-update-target-by-tag.json	Adds Content-Length/body arrays and normalizes expected fields
test/test-cases/regression/config-update-target-by-msg.json	Adds Content-Length/body arrays and normalizes expected fields
test/test-cases/regression/config-update-target-by-id.json	Adds Content-Length/body arrays and normalizes expected fields
test/test-cases/regression/config-secremoterules.json	Adds format-aligned request/response blocks and expected http_code for parser errors
test/test-cases/regression/config-response_type.json	Normalizes escaping; adds Content-Length/body arrays; updates expected http_code
test/test-cases/regression/config-remove_by_tag.json	Adds Content-Length/body arrays and expected http_code
test/test-cases/regression/config-remove_by_msg.json	Adds Content-Length/body arrays and expected http_code
test/test-cases/regression/config-remove_by_id.json	Adds Content-Length/body arrays and expected http_code
test/test-cases/regression/config-include-bad.json	Adds explicit request/response schema and expected http_code for parser errors
test/test-cases/regression/config-calling_phases_by_name.json	Adds Content-Length/body arrays and expected http_code
test/test-cases/regression/collection-resource.json	Normalizes escaping and adds content-length and empty response bodies
test/test-cases/regression/collection-regular_expression_selection.json	Normalizes escaping and adds Content-Length/body arrays
test/test-cases/regression/collection-lua.json	Normalizes empty body/headers handling and adds expected http_code
test/test-cases/regression/collection-case-insensitive.json	Normalizes escaping and adds Content-Length/body arrays
test/test-cases/regression/action-xmlns.json	Adds explicit request/response schema and expected http_code for parser errors
test/test-cases/regression/action-tnf-base64.json	Updates expected output due to newline handling; adds Content-Length/http_code
test/test-cases/regression/action-tag.json	Normalizes escaping and adds Content-Length/body arrays
test/test-cases/regression/action-skip.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-setuid.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-setsid.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-setrsc.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-setenv.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-msg.json	Normalizes escaping and expected debug output; adds expected http_code
test/test-cases/regression/action-initcol.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-id.json	Adds Content-Length and expected http_code to parser-error cases
test/test-cases/regression/action-expirevar.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-disruptive.json	Adds explicit client/server/request/response blocks for formatting/schema consistency
test/test-cases/regression/action-ctl_rule_remove_target_by_tag.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-ctl_rule_remove_by_tag.json	Adds Content-Length/body arrays and expected http_code
test/test-cases/regression/action-ctl_rule_remove_by_id.json	Adds explicit request/response schema and expected http_code
test/test-cases/regression/action-ctl_rule_engine.json	Normalizes request bodies/Content-Length and response headers
test/test-cases/regression/action-ctl_request_body_processor_urlencoded.json	Updates Content-Length and response headers
test/test-cases/regression/action-ctl_request_body_processor.json	Adds server/response blocks and normalizes expected structure
test/test-cases/regression/action-ctl_request_body_access.json	Updates Content-Length and expected structure
test/test-cases/regression/action-ctl_audit_engine.json	Adds explicit response skeleton and request body array
test/test-cases/regression/action-block.json	Adds explicit request/response schema and expected http_code for parser errors
test/test-cases/regression/action-allow.json	Adds explicit request/response schema and expected http_code
test/regression/regression_test.h	Adds formatting-related fields, switches to unique_ptr/optional, introduces RegressionTests
test/regression/regression.cc	Adds `format` mode that writes normalized JSON and optionally updates Content-Length
test/common/modsecurity_test.h	Modernizes member initialization and adds flags for `format`/content-length updating
test/common/modsecurity_test.cc	Adds format-mode JSON loading path and parses `format` positional + env var

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T16:55:09Z

test/test-cases/regression/fn-setHostname.json

-      "uri":"/foo?q=attack",
-      "http_version": 1.1
+      "uri": "/foo?q=attack",
+      "method": "",


An empty HTTP method is not a valid request line and is likely to break request construction/execution for this test. Use a concrete method (e.g., "GET") here (and ensure the rest of the test still sets/derives the expected hostname, since the expected debug log asserts [hostname: \"modsecurity.org\"]).

Suggested change

"method": "",

"method": "GET",

Copilot · 2026-02-06T16:55:10Z

test/test-cases/regression/secruleengine.json

+      "uri": "",
+      "method": "",
+      "body": [
+        ""
+      ]


These test cases assert HTTP outcomes (e.g., 403/200) but the newly added request fields use empty uri and method, which can produce an invalid HTTP request. Prefer a sane default like \"method\": \"GET\" and \"uri\": \"/\" (or omit these fields if the harness provides defaults) to keep the transaction structurally valid.

Copilot · 2026-02-06T16:55:10Z

test/test-cases/regression/request-body-parser-multipart-crlf.json

+      "uri": "/",
+      "method": "POST",
+      "body": [
+        "-----------------------------69343412719991675451336310646\n",


This test is explicitly named multipart-crlf, but the multipart boundary lines are terminated with \\n instead of \\r\\n while other lines use CRLF. If the intent is to test strict CRLF behavior, these boundaries should also end with \\r\\n (and the request Content-Length should be updated accordingly).

Copilot · 2026-02-06T16:55:10Z

test/test-cases/regression/request-body-parser-multipart-crlf.json

+        "1.1\r\n",
+        "1.2\r\n",
+        "1.3\r\n",
+        "-----------------------------69343412719991675451336310646\n",


This test is explicitly named multipart-crlf, but the multipart boundary lines are terminated with \\n instead of \\r\\n while other lines use CRLF. If the intent is to test strict CRLF behavior, these boundaries should also end with \\r\\n (and the request Content-Length should be updated accordingly).

Copilot · 2026-02-06T16:55:11Z

test/test-cases/regression/request-body-parser-multipart-crlf.json

+        "2.1\r\n",
+        "2.2\r\n",
+        "2.3\r\n",
+        "-----------------------------69343412719991675451336310646--\n"


This test is explicitly named multipart-crlf, but the multipart boundary lines are terminated with \\n instead of \\r\\n while other lines use CRLF. If the intent is to test strict CRLF behavior, these boundaries should also end with \\r\\n (and the request Content-Length should be updated accordingly).

Copilot · 2026-02-06T16:55:11Z

test/regression/regression.cc

+        for (const auto &[name, tests] : test2) {
+            std::ofstream ofs{name};


Using the unordered_map key (name) as the output path makes formatting behavior depend on how keys are constructed in load_test_json. Since you already set u->filename = file during load, it would be more robust to open tests[0]->filename (or the original file value) for writing, rather than relying on the map key.

Copilot · 2026-02-06T16:55:11Z

test/regression/regression_test.h

+    std::string filename;
+    std::string name;
+
+    std::vector<RegressionTest> tests;


Storing tests by value (std::vector<RegressionTest>) can cause expensive copies/moves when building or manipulating the collection, especially now that individual tests have multiple vectors/strings used for formatting. Consider storing std::vector<std::unique_ptr<RegressionTest>> (consistent with from_yajl_node ownership patterns elsewhere) or otherwise ensure construction avoids unnecessary copying.

Suggested change

std::vector<RegressionTest> tests;

std::vector<std::unique_ptr<RegressionTest>> tests;

airween · 2026-02-06T17:10:35Z

Hi @hnakamur,

finally I was able to start to review this PR, but it's very huge... 😃, so I asked Copilot (mainly to review the tests' changes).

It seems like everything is okay, Copilot made one suggestion - would you mind to take a look at that?

And thank you again.

hnakamur added 4 commits January 25, 2026 20:00

Do not add newline to test body Lines

a5a3db9

This is needed to test edge cases like a body missing the final newline, for example: [ "a\r\n", "b\r" ] And it is clearer than to add implicit newline to each line except the final line.

Add needed newlines in test request body lines or adjust tests

d7cad95

Replace CRLF to LF in test case JSON files

e03f8a1

Add format subcommand to regression_test

b1c59b7

hnakamur mentioned this pull request Jan 25, 2026

Refine SecRequestBodyNoFilesLimit test cases to cover edge scenarios #3479

Closed

hnakamur added 4 commits January 26, 2026 08:49

Modify RegressionTest and use it for output formatted JSON

488370c

Update content-length in regeression_tests format subcommand

37e1f13

when UPDATE_CONTENT_LENGTH env var is set

Format regression test JSON files

191dba9

by running the following command: ``` (cd test; ./regression_tests format) ```

Update Content-Length in regression test JSON files

9b49187

by running the following command: ``` (cd test; UPDATE_CONTENT_LENGTH=1 ./regression_tests format) ```

hnakamur force-pushed the v3/do_not_add_newline_to_test_body_lines branch from 8fefe12 to 9b49187 Compare January 25, 2026 23:59

Adjust tests to pass after adding Content-Length

aefd4a4

airween added the 3.x Related to ModSecurity version 3.x label Jan 27, 2026

hnakamur added 12 commits January 28, 2026 22:12

Make RegressionTests::toJSON method const

2ef033b

Do not shadow outer variable

03c3e75

Use in-class initializer in ModSecurityTest

d8fd7d6

Use a structured binding in regression_test.cc

48fbc1b

Use emplace_back in regression_test.cc

1492841

Use std::optional has_value to clarify the code

8d5a80d

Use unique_ptr in from_yajl_node in test/common/

57689a0

Use default implementation for ModSecurityTest constructor

2a84d52

Use std::any_of in has_chunked_header

c0fdf7b

Do not shadow variable

22c7d78

Reduce complexity in RegressionTest::from_yajl_node

cf46967

Reduce complexity more in RegressionTest::from_yajl_node

550b0be

airween requested a review from Copilot February 6, 2026 16:52

Copilot AI reviewed Feb 6, 2026

View reviewed changes

		for (const auto &[name, tests] : test2) {
		std::ofstream ofs{name};

	std::vector<RegressionTest> tests;
	std::vector<std::unique_ptr<RegressionTest>> tests;

Conversation

hnakamur commented Jan 25, 2026

what

why

references

Uh oh!

hnakamur commented Jan 26, 2026

Uh oh!

hnakamur commented Jan 26, 2026

Uh oh!

airween commented Jan 27, 2026

Uh oh!

sonarqubecloud bot commented Jan 28, 2026

Quality Gate passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

airween commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants