Skip to content

Commit 973f5ec

Browse files
sodabrewsodabrew
authored
Merge pull request #360 from sodabrew/fix-memc-check-key
2 parents 43bbf00 + be93a45 commit 973f5ec

File tree

5 files changed

+396
-127
lines changed

5 files changed

+396
-127
lines changed

Diff for: ‎package.xml

+2-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ Fixes
147147
<file role='test' name='incrdecr_invalid_key.phpt'/>
148148
<file role='test' name='incrdecr_bykey.phpt'/>
149149
<file role='test' name='invalid_options.phpt'/>
150-
<file role='test' name='keys.phpt'/>
150+
<file role='test' name='keys_ascii.phpt'/>
151+
<file role='test' name='keys_binary.phpt'/>
151152
<file role='test' name='testdata.res'/>
152153
<file role='test' name='config.inc'/>
153154
<file role='test' name='sasl_basic.phpt'/>

Diff for: ‎php_memcached.c

+14-8
Original file line numberDiff line numberDiff line change
@@ -198,24 +198,30 @@ static inline php_memc_object_t *php_memc_fetch_object(zend_object *obj) {
198198
(void)memc_user_data; /* avoid unused variable warning */
199199

200200
static
201-
zend_bool s_memc_valid_key_binary(const char *key)
201+
zend_bool s_memc_valid_key_binary(zend_string *key)
202202
{
203-
return strchr(key, '\n') == NULL;
203+
return memchr(ZSTR_VAL(key), '\n', ZSTR_LEN(key)) == NULL;
204204
}
205205

206206
static
207-
zend_bool s_memc_valid_key_ascii(const char *key)
207+
zend_bool s_memc_valid_key_ascii(zend_string *key)
208208
{
209-
while (*key && !iscntrl(*key) && !isspace(*key)) ++key;
210-
return *key == '\0';
209+
const char *str = ZSTR_VAL(key);
210+
size_t i, len = ZSTR_LEN(key);
211+
212+
for (i = 0; i < len; i++) {
213+
if (iscntrl(str[i]) || isspace(str[i]))
214+
return 0;
215+
}
216+
return 1;
211217
}
212218

213219
#define MEMC_CHECK_KEY(intern, key) \
214220
if (UNEXPECTED(ZSTR_LEN(key) == 0 || \
215221
ZSTR_LEN(key) > MEMC_OBJECT_KEY_MAX_LENGTH || \
216222
(memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL) \
217-
? !s_memc_valid_key_binary(ZSTR_VAL(key)) \
218-
: !s_memc_valid_key_ascii(ZSTR_VAL(key)) \
223+
? !s_memc_valid_key_binary(key) \
224+
: !s_memc_valid_key_ascii(key) \
219225
))) { \
220226
intern->rescode = MEMCACHED_BAD_KEY_PROVIDED; \
221227
RETURN_FALSE; \
@@ -309,7 +315,7 @@ PHP_INI_MH(OnUpdateSessionPrefixString)
309315
php_error_docref(NULL, E_WARNING, "memcached.sess_prefix too long (max: %d)", MEMCACHED_MAX_KEY - 1);
310316
return FAILURE;
311317
}
312-
if (!s_memc_valid_key_ascii(ZSTR_VAL(new_value))) {
318+
if (!s_memc_valid_key_ascii(new_value)) {
313319
php_error_docref(NULL, E_WARNING, "memcached.sess_prefix cannot contain whitespace or control characters");
314320
return FAILURE;
315321
}

Diff for: ‎tests/keys.phpt

-118
This file was deleted.

Diff for: ‎tests/keys_ascii.phpt

+190
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
--TEST--
2+
Test valid and invalid keys - ascii
3+
--SKIPIF--
4+
<?php include "skipif.inc";?>
5+
--FILE--
6+
<?php
7+
8+
include dirname (__FILE__) . '/config.inc';
9+
$ascii = memc_get_instance (array (
10+
Memcached::OPT_BINARY_PROTOCOL => false,
11+
Memcached::OPT_VERIFY_KEY => false
12+
));
13+
// libmemcached can verify keys, but these are tests are for our own
14+
// function s_memc_valid_key_ascii, so explicitly disable the checks
15+
// that libmemcached can perform.
16+
17+
echo 'ASCII: SPACES' . PHP_EOL;
18+
var_dump ($ascii->set ('ascii key with spaces', 'this is a test'));
19+
var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
20+
21+
echo 'ASCII: NEWLINE' . PHP_EOL;
22+
var_dump ($ascii->set ('asciikeywithnewline' . PHP_EOL, 'this is a test'));
23+
var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
24+
25+
echo 'ASCII: EMPTY' . PHP_EOL;
26+
var_dump ($ascii->set (''/*empty key*/, 'this is a test'));
27+
var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
28+
29+
echo 'ASCII: TOO LONG' . PHP_EOL;
30+
var_dump ($ascii->set (str_repeat ('1234567890', 512), 'this is a test'));
31+
var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
32+
33+
echo 'ASCII: GET' . PHP_EOL;
34+
for ($i=0;$i<32;$i++) {
35+
var_dump ($ascii->get ('asciikeywithnonprintablechar-' . chr($i) . '-here'));
36+
var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
37+
}
38+
39+
echo 'ASCII: SET' . PHP_EOL;
40+
for ($i=0;$i<32;$i++) {
41+
var_dump ($ascii->set ('asciikeywithnonprintablechar-' . chr($i) . '-here', 'this is a test'));
42+
var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
43+
}
44+
45+
echo 'OK' . PHP_EOL;
46+
47+
--EXPECT--
48+
ASCII: SPACES
49+
bool(false)
50+
bool(true)
51+
ASCII: NEWLINE
52+
bool(false)
53+
bool(true)
54+
ASCII: EMPTY
55+
bool(false)
56+
bool(true)
57+
ASCII: TOO LONG
58+
bool(false)
59+
bool(true)
60+
ASCII: GET
61+
bool(false)
62+
bool(true)
63+
bool(false)
64+
bool(true)
65+
bool(false)
66+
bool(true)
67+
bool(false)
68+
bool(true)
69+
bool(false)
70+
bool(true)
71+
bool(false)
72+
bool(true)
73+
bool(false)
74+
bool(true)
75+
bool(false)
76+
bool(true)
77+
bool(false)
78+
bool(true)
79+
bool(false)
80+
bool(true)
81+
bool(false)
82+
bool(true)
83+
bool(false)
84+
bool(true)
85+
bool(false)
86+
bool(true)
87+
bool(false)
88+
bool(true)
89+
bool(false)
90+
bool(true)
91+
bool(false)
92+
bool(true)
93+
bool(false)
94+
bool(true)
95+
bool(false)
96+
bool(true)
97+
bool(false)
98+
bool(true)
99+
bool(false)
100+
bool(true)
101+
bool(false)
102+
bool(true)
103+
bool(false)
104+
bool(true)
105+
bool(false)
106+
bool(true)
107+
bool(false)
108+
bool(true)
109+
bool(false)
110+
bool(true)
111+
bool(false)
112+
bool(true)
113+
bool(false)
114+
bool(true)
115+
bool(false)
116+
bool(true)
117+
bool(false)
118+
bool(true)
119+
bool(false)
120+
bool(true)
121+
bool(false)
122+
bool(true)
123+
bool(false)
124+
bool(true)
125+
ASCII: SET
126+
bool(false)
127+
bool(true)
128+
bool(false)
129+
bool(true)
130+
bool(false)
131+
bool(true)
132+
bool(false)
133+
bool(true)
134+
bool(false)
135+
bool(true)
136+
bool(false)
137+
bool(true)
138+
bool(false)
139+
bool(true)
140+
bool(false)
141+
bool(true)
142+
bool(false)
143+
bool(true)
144+
bool(false)
145+
bool(true)
146+
bool(false)
147+
bool(true)
148+
bool(false)
149+
bool(true)
150+
bool(false)
151+
bool(true)
152+
bool(false)
153+
bool(true)
154+
bool(false)
155+
bool(true)
156+
bool(false)
157+
bool(true)
158+
bool(false)
159+
bool(true)
160+
bool(false)
161+
bool(true)
162+
bool(false)
163+
bool(true)
164+
bool(false)
165+
bool(true)
166+
bool(false)
167+
bool(true)
168+
bool(false)
169+
bool(true)
170+
bool(false)
171+
bool(true)
172+
bool(false)
173+
bool(true)
174+
bool(false)
175+
bool(true)
176+
bool(false)
177+
bool(true)
178+
bool(false)
179+
bool(true)
180+
bool(false)
181+
bool(true)
182+
bool(false)
183+
bool(true)
184+
bool(false)
185+
bool(true)
186+
bool(false)
187+
bool(true)
188+
bool(false)
189+
bool(true)
190+
OK

0 commit comments

Comments
 (0)