Bug Fix: The value in the setvar should be able to start with - or +. #1125
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1125 +/- ##
==========================================
+ Coverage 82.72% 83.14% +0.42%
==========================================
Files 162 164 +2
Lines 9080 7691 -1389
==========================================
- Hits 7511 6395 -1116
+ Misses 1319 1042 -277
- Partials 250 254 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| Err(err). | ||
| Msg("Invalid value") | ||
| // If the variable doesn't exist, we would need to raise an error. Otherwise, it should be the same value. | ||
| if strings.HasPrefix(value[1:], "tx.") { |
There was a problem hiding this comment.
shouldn't this be %{tx. instead of tx. I don't see any of the tests having tx. as prefix removing the first byte.
There was a problem hiding this comment.
It is either resolve to the value or stays as tx.missingValue.
There was a problem hiding this comment.
But according to https://github.com/corazawaf/coraza/pull/1125/files#diff-488435c221aebedf4137cb98424234aab81decfd749827ab96e188c53bd53c9fR90-R92 it returns a syntax error.
There was a problem hiding this comment.
Here's the value: 
There was a problem hiding this comment.
I see. It is still not clear to me how we should deal with the interpolation error in this code path. Let me check it.
There was a problem hiding this comment.
Hey @jcchavezs ,
Any update on how this error could be handled?
Thanks!
There was a problem hiding this comment.
Ok I found the issue, when we compile a macro e.g. %{tx.var}, the text for the macro becomes tx.var and not %{tx.var} and when the key does not exist we return the text which in this case is tx.var. I will discuss that over another PR.
|
thanks, please update this line https://github.com/corazawaf/coraza/pull/1125/files#diff-4f480854f968ff92545ff3bc611fb34a15a657f6caf11dc203c64b76c0861827R148 with the appropriate comment. |
| name: "Non Numerical Operation - If the value starts with -", | ||
| init: "TX.newvar=----expected_value", | ||
| expectInvalidSyntaxError: false, | ||
| }, |
There was a problem hiding this comment.
should we include expectNewVarValue in the test swell?
|
Sorry for the delay. Prioritizing this for now. |
Make sure that you've checked the boxes below before you submit PR:
Thanks for your contribution ❤️