V3/remove this throw call transaction h mk2 #3207
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 5 commits
August 4, 2024 22:04
…ements. - SonarCloud analysis identified standalone `throw;` calls without accompanying `try-catch` blocks, used inconsistently as placeholders or for premature termination under specific conditions. - Removed these `throw;` instances to prevent potential runtime issues in future development phases, where such configurations might inadvertently be created. - Introduced `assert` statements as a more appropriate mechanism for asserting preconditions in the affected class member functions, ensuring clearer intent and safer code behavior during development. - Refactor action_kind processing to use switch() instead of if-else chains; add assertion in default case. - Fix SonarCloud issue: Make this variable a const reference. https://sonarcloud.io/project/issues?resolved=false&pullRequest=3104&id=owasp-modsecurity_ModSecurity&open=AY8Vpgy4f6U6E7VKL4Cn
Implemented a new configuration option --enable-assertions=[yes|no] within config.ac, enabling controlled inclusion of -DNDEBUG in CPPFLAGS. The default setting suppresses assertions (by adding -DNDEBUG to CPPFLAGS), preserving the original behavior. This enhancement allows for the optional enabling of assertions during development or debugging by setting --enable-assertions=yes, thereby excluding -DNDEBUG from CPPFLAGS.
|
airween
requested changes
Aug 5, 2024
eduar-hte
added a commit
to eduar-hte/ModSecurity
that referenced
this pull request
Aug 8, 2024
…on Unix builds - This configuration flag was introduced in commit d47185d in the context of PR owasp-modsecurity#3207. - Moved to the configure step's 'run' command in order to be shared across configurations. - For the sake of reference, matrix.platform.configure should be used for configuration flags that are needed for a specific platform/architecture (which was the reason it was introduced in commit d9255d8, PR owasp-modsecurity#3144).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introducing the use of assertions to address throw; calls that lack try-catch blocks. Upon examining the caller code that utilized methods containing the questioned throw; calls, it became clear that, in the current state of development, there are no scenarios where execution could reach these throw; calls. However, we cannot guarantee this for future development. For instance, if someone attempts to use getCurrentMarker() without first verifying isInsideAMarker(), ModSecurity would encounter the throw; and terminate. The issue with the other throw; call is similar in that it is, fortunately, unreachable at the moment. However, it differs because this throw; is intended to handle a case that has not yet been developed.
Fortunately, I found an article titled "Effective Use of Assertions in C++" by Mike A. Martin in (ACM SIGPLAN Language Tips, page 3), which offers a neat way to handle such cases, specifically regarding argument validation and unreachable code. https://dl.acm.org/doi/pdf/10.1145/240964.240969).
Following the guidance from this article, I addressed the issues and also included modifications to enrich assert error messages. Furthermore, I updated configure.ac to maintain the usual build procedure and modified README.md to introduce the new configure flag.