Fix GCC -Wall warnings on Fedora/RHEL builds

[mikelolasagasti]

what

• Fix compiler warnings reported when building with GCC and Fedora/RHEL-style flags (-Wall, hardening, ...).
• Each commit targets one warning class / location where practical.
• Build log warnings reduced from ~1270 to ~57 (same Fedora 45 / 3.0.15 build); remainder left for follow-up PRs.

why

• Clean build logs are just better.
• Fewer noisy warnings makes real regressions easier to spot in CI/packaging builds.

references

• Fedora 45 build log without the patches: https://kojipkgs.fedoraproject.org//packages/libmodsecurity/3.0.15/1.fc45/data/logs/x86_64/build.log
• Fedora 45 build log with these patches (temp logs): https://kojipkgs.fedoraproject.org//work/tasks/231/145490231/build.log

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of IP addresses with and without network masks.
  • Strengthened UTF-8-to-Unicode escape formatting to handle output errors safely.
  • Improved cleanup when content-offset validation fails.

• Improvements

  • Updated size and index handling across request processing and rule evaluation to better accommodate varying input lengths.
  • Clarified initialization and parsing behavior without changing existing rule evaluation or transformation behavior.

[mikelolasagasti]

Fixed 3 SonarCloud warnings.

The "10.9% Duplication on New Code (required ≤ 3%)" Quality Gate is out of the scope of this PR.

[sonarqubecloud]

Quality Gate failed

Failed conditions
11.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

[airween]

Hi @mikelolasagasti,

could you pick these changes or update your branch?

Thanks!

[Copilot]

Pull request overview

This PR reduces GCC -Wall (Fedora/RHEL-style) build warnings across ModSecurity by addressing common warning sources (initializer reordering, signed/unsigned mismatches, unused variables, and safer string/offset handling) in core transaction/rules logic, operators, parser, utilities, and public headers.

Changes:

• Reordered constructor/base/member initializers and added explicit default member initialization to silence -Wreorder/uninitialized warnings.
• Replaced several int loop/index/length variables with size_t / std::string::size_type / unsigned types and added casts where needed to silence sign/size warnings.
• Fixed a correctness issue in utf8_to_unicode (snprintf buffer size) and made IP parsing more robust when no CIDR slash is present.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/variables/variable.h	Reorders initializer lists to match declaration order (-Wreorder).
src/utils/string.h	Uses std::string::size_type for truncation length.
src/utils/msc_tree.cc	Fixes signed/unsigned warnings and avoids strchr(...)-buffer when no / is present.
src/transaction.cc	Fixes base/member init order and widens request-body length type.
src/rules_set.cc	Uses size_t for rule vector indexing.
src/request_body_processor/multipart.cc	Fixes signed/unsigned comparison for upload file limit.
src/parser/seclang-parser.yy	Removes unused local variables in grammar actions.
src/parser/driver.cc	Uses size_t for rule vector indexing.
src/operators/validate_url_encoding.cc	Uses uint64_t index to match input_length type.
src/operators/rx.h	Moves m_re initialization to in-class default initializer.
src/operators/rx_global.h	Moves m_re initialization to in-class default initializer.
src/operators/pm_from_file.cc	Uses size_t loop index when scanning comment prefix.
src/modsecurity.cc	Adds explicit casts for offset bounds checks when parsing highlight data.
src/actions/transformations/utf8_to_unicode.cc	Fixes snprintf size argument and uses size_t for derived lengths/loops.
src/actions/transformations/html_entity_decode.cc	Uses std::string::size_type for loop index.
src/actions/transformations/compress_whitespace.cc	Fixes pointer-diff type warning via explicit cast and clarifies changed type.
src/actions/init_col.cc	Uses std::string::size_type for find() result.
headers/modsecurity/intervention.h	Switches header-defined helpers from static to inline to reduce warnings.
headers/modsecurity/anchored_set_variable_translation_proxy.h	Uses vector size_type for indexing.

---

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

[mikelolasagasti]

@airween rebased against current master. Also added fixes for CoPilot findings (commits 21 & 22) and fixed the githab AI findings with with previous approach.

[airween]

@mikelolasagasti thanks - meanwhile there were some changes recently (see 3.0.16), could you pick up those modifications?

For eg. utf8_to_unicode.cc is completely new.

[mikelolasagasti]

@airween the PR is rebased against current v3/master, so it includes all the recent changes.

For utf8_to_unicode.cc I had to make a few changes in my patch on top of your recent changes: 311e04a

[airween]

For utf8_to_unicode.cc I had to make a few changes in my patch on top of your recent changes: 311e04a

Yes - I think you should use the mentioned fix.

Also please upgrade your git submodules - I've added a test case against that bug to secrules-language-tests repository too (which is a submodule - it was suspicious that i386 tests were success in the CI workflow).

[airween]

Yes - I think you should use the mentioned fix.

sorry, now I reviewed your changes and it seems it built on my previous changes, but yours are more strict - which is better. So I would accept your changes in case of file utf8_to_unicode.cc, but it would be good to eliminate the Sonar's "Duplicated Lines (%) on New Code 31.6%" report.

[sonarqubecloud]

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

[mikelolasagasti]

Added an extra commit to address the duplicated code issue that was already present in the code. With this SonarQube shows all green.

[mikelolasagasti]

@airween should be all green now

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 8c71d76c-01b3-4865-8fdf-275026752de2

📥 Commits

Reviewing files that changed from the base of the PR and between 1925753 and 7e7a96e.

📒 Files selected for processing (19)

• headers/modsecurity/anchored_set_variable_translation_proxy.h
• headers/modsecurity/intervention.h
• src/actions/init_col.cc
• src/actions/transformations/compress_whitespace.cc
• src/actions/transformations/html_entity_decode.cc
• src/actions/transformations/utf8_to_unicode.cc
• src/modsecurity.cc
• src/operators/pm_from_file.cc
• src/operators/rx.h
• src/operators/rx_global.h
• src/operators/validate_url_encoding.cc
• src/parser/driver.cc
• src/parser/seclang-parser.yy
• src/request_body_processor/multipart.cc
• src/rules_set.cc
• src/transaction.cc
• src/utils/msc_tree.cc
• src/utils/string.h
• src/variables/variable.h

💤 Files with no reviewable changes (1)

• src/parser/seclang-parser.yy

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The pull request updates size and index types across several code paths, centralizes Unicode escape formatting, adjusts offset and IP parsing checks, and changes selected initialization and declaration details.

Changes

Code corrections

Layer / File(s)	Summary
Size and index types headers/modsecurity/anchored_set_variable_translation_proxy.h, src/actions/init_col.cc, src/actions/transformations/compress_whitespace.cc, src/actions/transformations/html_entity_decode.cc, src/operators/pm_from_file.cc, src/operators/validate_url_encoding.cc, src/parser/driver.cc, src/request_body_processor/multipart.cc, src/rules_set.cc, src/transaction.cc, src/utils/string.h	Loop indices, lengths, and the limitTo parameter now use size-related types. Comment scanning converts characters to unsigned char before calling std::isspace.
Unicode escape formatting src/actions/transformations/utf8_to_unicode.cc	A shared helper formats escapes for two-, three-, and four-byte UTF-8 sequences. Each encoding path returns the current changed value if formatting fails.
Offset bounds and generator cleanup src/modsecurity.cc	Content and variable offsets are converted to size_t for length checks. Both out-of-range paths free the YAJL generator before returning.
IP tree parsing and bit validation src/utils/msc_tree.cc	Bit validation and netmask-copy loops use unsigned or size-related types. IPv4 and IPv6 truncation now requires a slash.
Initialization and declaration updates headers/modsecurity/intervention.h, src/operators/rx.h, src/operators/rx_global.h, src/transaction.cc, src/variables/variable.h, src/parser/seclang-parser.yy	Intervention helpers change from static to inline. Regex and variable constructors revise initialization order. Runtime-variable grammar actions remove unused locals.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: airween

Merge Risk: ⚪ Minimal · up to 7e7a9

The warning fixes preserve the documented processing behavior and improve character handling and error-path cleanup. No actionable merge-blocking risk remains; merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 7e7a9

The examined changes preserve input bounds and logging limits while improving cleanup on invalid offsets. No introduced security concern was established, but incomplete parser and external-consumer coverage leaves some uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The examined interfaces operate on caller-supplied content and encoded offsets, request-derived message strings, and individual intervention objects. The truncation change reaches security detection and rule-message consumers through their existing utility calls.

Trust Boundaries and Controls

• inferred — The parameter-type change does not weaken truncation limits in the identified repository callers: potentially attacker-derived text remains the string argument, while the amount is a fixed literal controlled by application code.

Resilience and Maintainability Implications

• observed — Generator ownership is now released on both explicit offset-bound failures as well as normal completion. Numeric-conversion exceptions can still bypass cleanup, but base/head comparison establishes that limitation as pre-existing, not introduced by these changes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 2.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 18 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: fixing GCC -Wall warnings for Fedora/RHEL builds.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

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

[mikelolasagasti]

@airween rebased