Skip to content

Commit 9cac167

Browse files
martinhsvFelipe Zimmerle
martinhsv
authored and
Felipe Zimmerle
committed
Fix argument key-value pair parsing cases
1 parent 68c995c commit 9cac167

File tree

6 files changed

+148
-22
lines changed

6 files changed

+148
-22
lines changed

‎CHANGES

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
v3.0.4 - YYYY-MMM-DD (to be released)
22
-------------------------------------
33

4+
- Fix argument key-value pair parsing cases
5+
[Issue #1904 - @martinhsv]
46
- Fix: audit log part for response body for JSON format to be E
57
[Issue #2066 - @martinhsv, @zimmerle]
68
- Make sure m_rulesMessages is filled after successfull match

‎src/transaction.cc

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -294,17 +294,9 @@ bool Transaction::extractArguments(const std::string &orig,
294294

295295
std::string key;
296296
std::string value;
297-
std::vector<std::string> key_value = utils::string::ssplit(t, sep2);
298-
for (auto& a : key_value) {
299-
if (i == 0) {
300-
key = a;
301-
} else if (i == 1) {
302-
value = a;
303-
} else {
304-
value = value + "=" + a;
305-
}
306-
i++;
307-
}
297+
std::pair<std::string, std::string> key_value_pair = utils::string::ssplit_pair(t, sep2);
298+
key = key_value_pair.first;
299+
value = key_value_pair.second;
308300

309301
key_s = (key.length() + 1);
310302
value_s = (value.length() + 1);

‎src/utils/string.cc

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,20 +182,29 @@ std::vector<std::string> ssplit(std::string str, char delimiter) {
182182
std::vector<std::string> internal;
183183
std::stringstream ss(str); // Turn the string into a stream.
184184
std::string tok;
185-
ssize_t n = str.length();
186-
int i = 0;
187185

188186
while (getline(ss, tok, delimiter)) {
189-
n -= tok.length();
190-
if (i > 0) n--;
191-
internal.push_back(n == 1 ? tok + delimiter : tok);
192-
i++;
187+
internal.push_back(tok);
193188
}
194189

195190
return internal;
196191
}
197192

198193

194+
std::pair<std::string, std::string> ssplit_pair(const std::string& str, char delimiter) {
195+
std::stringstream ss(str); // Turn the string into a stream.
196+
std::string key;
197+
std::string value;
198+
199+
getline(ss, key, delimiter);
200+
if (key.length() < str.length()) {
201+
value = str.substr(key.length()+1);
202+
}
203+
204+
return std::make_pair(key, value);
205+
}
206+
207+
199208
std::vector<std::string> split(std::string str, char delimiter) {
200209
std::vector<std::string> internal = ssplit(str, delimiter);
201210

‎src/utils/string.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ std::string toHexIfNeeded(const std::string &str);
6565
std::string tolower(std::string str);
6666
std::string toupper(std::string str);
6767
std::vector<std::string> ssplit(std::string str, char delimiter);
68+
std::pair<std::string, std::string> ssplit_pair(const std::string& str, char delimiter);
6869
std::vector<std::string> split(std::string str, char delimiter);
6970
void chomp(std::string *str);
7071
void replaceAll(std::string *str, const std::string& from,

‎test/test-cases/regression/variable-ARGS_GET.json

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{
33
"enabled":1,
44
"version_min":300000,
5-
"title":"Testing Variables :: ARGS_GET (1/3)",
5+
"title":"Testing Variables :: ARGS_GET (1/5)",
66
"client":{
77
"ip":"200.249.12.31",
88
"port":123
@@ -41,7 +41,7 @@
4141
{
4242
"enabled":1,
4343
"version_min":300000,
44-
"title":"Testing Variables :: ARGS_GET (2/3)",
44+
"title":"Testing Variables :: ARGS_GET (2/5)",
4545
"client":{
4646
"ip":"200.249.12.31",
4747
"port":123
@@ -80,7 +80,7 @@
8080
{
8181
"enabled":1,
8282
"version_min":300000,
83-
"title":"Testing Variables :: ARGS_GET (3/3)",
83+
"title":"Testing Variables :: ARGS_GET (3/5)",
8484
"client":{
8585
"ip":"200.249.12.31",
8686
"port":123
@@ -115,6 +115,84 @@
115115
"SecRuleEngine On",
116116
"SecRule ARGS_GET \"@contains test \" \"id:1,pass,t:trim\""
117117
]
118+
},
119+
{
120+
"enabled":1,
121+
"version_min":300000,
122+
"title":"Testing Variables :: ARGS_GET (4/5)",
123+
"client":{
124+
"ip":"200.249.12.31",
125+
"port":123
126+
},
127+
"server":{
128+
"ip":"200.249.12.31",
129+
"port":80
130+
},
131+
"request":{
132+
"headers":{
133+
"Host":"localhost",
134+
"User-Agent":"curl/7.38.0",
135+
"Accept":"*/*"
136+
},
137+
"uri":"/?key=value&secondkey=&key3=val3",
138+
"method":"GET"
139+
},
140+
"response":{
141+
"headers":{
142+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
143+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
144+
"Content-Type":"text/html"
145+
},
146+
"body":[
147+
"no need."
148+
]
149+
},
150+
"expected":{
151+
"debug_log":"Target value: \"0\""
152+
},
153+
"rules":[
154+
"SecRuleEngine On",
155+
"SecRule ARGS_GET:secondkey \"0\" \"id:1,phase:2,pass,t:none,t:length\""
156+
]
157+
},
158+
{
159+
"enabled":1,
160+
"version_min":300000,
161+
"title":"Testing Variables :: ARGS_GET (5/5)",
162+
"client":{
163+
"ip":"200.249.12.31",
164+
"port":123
165+
},
166+
"server":{
167+
"ip":"200.249.12.31",
168+
"port":80
169+
},
170+
"request":{
171+
"headers":{
172+
"Host":"localhost",
173+
"User-Agent":"curl/7.38.0",
174+
"Accept":"*/*"
175+
},
176+
"uri":"/?key=value&secondkey=othervalue&",
177+
"method":"GET"
178+
},
179+
"response":{
180+
"headers":{
181+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
182+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
183+
"Content-Type":"text/html"
184+
},
185+
"body":[
186+
"no need."
187+
]
188+
},
189+
"expected":{
190+
"debug_log":"Target value: \"othervalue\""
191+
},
192+
"rules":[
193+
"SecRuleEngine On",
194+
"SecRule ARGS_GET \"@rx ^othervalue$ \" \"id:1,pass,t:none\""
195+
]
118196
}
119197
]
120198

‎test/test-cases/regression/variable-ARGS_POST.json

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{
33
"enabled":1,
44
"version_min":300000,
5-
"title":"Testing Variables :: ARGS_POST (1/2)",
5+
"title":"Testing Variables :: ARGS_POST (1/3)",
66
"client":{
77
"ip":"200.249.12.31",
88
"port":123
@@ -46,7 +46,7 @@
4646
{
4747
"enabled":1,
4848
"version_min":300000,
49-
"title":"Testing Variables :: ARGS_POST (1/2)",
49+
"title":"Testing Variables :: ARGS_POST (2/3)",
5050
"client":{
5151
"ip":"200.249.12.31",
5252
"port":123
@@ -86,6 +86,50 @@
8686
"SecRuleEngine On",
8787
"SecRule ARGS_POST \"@contains test \" \"id:1,phase:3,pass,t:trim\""
8888
]
89+
},
90+
{
91+
"enabled":1,
92+
"version_min":300000,
93+
"title":"Testing Variables :: ARGS_POST (3/3)",
94+
"client":{
95+
"ip":"200.249.12.31",
96+
"port":123
97+
},
98+
"server":{
99+
"ip":"200.249.12.31",
100+
"port":80
101+
},
102+
"request":{
103+
"headers":{
104+
"Host":"localhost",
105+
"User-Agent":"curl/7.38.0",
106+
"Accept":"*/*",
107+
"Content-Length": "27",
108+
"Content-Type": "application/x-www-form-urlencoded"
109+
},
110+
"uri":"/",
111+
"method":"POST",
112+
"body": [
113+
"param1=value1&param2=&param3=value3"
114+
]
115+
},
116+
"response":{
117+
"headers":{
118+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
119+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
120+
"Content-Type":"text/html"
121+
},
122+
"body":[
123+
"no need."
124+
]
125+
},
126+
"expected":{
127+
"debug_log":"Target value: \"0\""
128+
},
129+
"rules":[
130+
"SecRuleEngine On",
131+
"SecRule ARGS_POST:param2 \"0\" \"id:1,phase:2,pass,t:none,t:length\""
132+
]
89133
}
90134
]
91135

0 commit comments

Comments
 (0)