Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing throw in Operator::instantiate? #2106

Closed
marduone opened this issue Jun 1, 2019 · 1 comment
Closed

Missing throw in Operator::instantiate? #2106

marduone opened this issue Jun 1, 2019 · 1 comment
Assignees
Labels
3.x Related to ModSecurity version 3.x enhancement workaround available The issue has either a temporary or permanent workaround available
Milestone

Comments

@marduone
Copy link

marduone commented Jun 1, 2019

Building ModSecurity 3.0.3 package (openSUSE Tumbleweed, GCC 9) produced the following compiler warning which the build system flagged as an error:

[  293s] operators/operator.cc: In static member function 'static modsecurity::operators::Operator* modsecurity::operators::Operator::instantiate(std::string, std::string)':
[  293s] operators/operator.cc:140:48: warning: control reaches end of non-void function [-Wreturn-type]
[  293s]   140 |     std::string op_ = utils::string::tolower(op);
[  293s]       |                                                ^

Looking at the code in question, it seems that the warning is caused by a missing throw keyword right at the end of the function, where I suppose you want to throw an invalid argument exception if none of operator checks match.

The following patch got rid of that compiler warning:

--- modsecurity-v3.0.3.orig/src/operators/operator.cc
+++ modsecurity-v3.0.3/src/operators/operator.cc
@@ -191,7 +191,7 @@ Operator *Operator::instantiate(std::str
         return new UnconditionalMatch();
     }
 
-    std::invalid_argument("Operator not found.");
+    throw std::invalid_argument("Operator not found.");
 }
 
 }  // namespace operators

While the above patch seems correct to me, I am not 100% sure whether this is a genuine bug in the code or just newer GCC being stricter and flagging non-issues as warnings.

@zimmerle zimmerle self-assigned this Jun 4, 2019
@zimmerle zimmerle added the 3.x Related to ModSecurity version 3.x label Jun 4, 2019
@victorhora victorhora added this to the v3.0.4 milestone Jun 8, 2019
@victorhora victorhora self-assigned this Jun 8, 2019
@victorhora victorhora added enhancement workaround available The issue has either a temporary or permanent workaround available labels Jun 8, 2019
zimmerle added a commit that referenced this issue Jun 17, 2019
@zimmerle
Copy link
Contributor

Thank you @marduone. The code is now on our master branch.

vladbukin pushed a commit to vladbukin/ModSecurity that referenced this issue Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x enhancement workaround available The issue has either a temporary or permanent workaround available
Projects
None yet
Development

No branches or pull requests

3 participants