Fix memc check key

package.xml

@@ -147,7 +147,8 @@ Fixes
     <file role='test' name='incrdecr_invalid_key.phpt'/>
     <file role='test' name='incrdecr_bykey.phpt'/>
     <file role='test' name='invalid_options.phpt'/>
-    <file role='test' name='keys.phpt'/>
+    <file role='test' name='keys_ascii.phpt'/>
+    <file role='test' name='keys_binary.phpt'/>
     <file role='test' name='testdata.res'/>
     <file role='test' name='config.inc'/>
     <file role='test' name='sasl_basic.phpt'/>

php_memcached.c

@@ -198,24 +198,30 @@ static inline php_memc_object_t *php_memc_fetch_object(zend_object *obj) {
 	(void)memc_user_data; /* avoid unused variable warning */
 
 static
-zend_bool s_memc_valid_key_binary(const char *key)
+zend_bool s_memc_valid_key_binary(zend_string *key)
 {
-	return strchr(key, '\n') == NULL;
+	return memchr(ZSTR_VAL(key), '\n', ZSTR_LEN(key)) == NULL;
 }
 
 static
-zend_bool s_memc_valid_key_ascii(const char *key)
+zend_bool s_memc_valid_key_ascii(zend_string *key)
 {
-	while (*key && !iscntrl(*key) && !isspace(*key)) ++key;
-	return *key == '\0';
+	const char *str = ZSTR_VAL(key);
+	size_t i, len = ZSTR_LEN(key);
+
+	for (i = 0; i < len; i++) {
+		if (iscntrl(str[i]) || isspace(str[i]))
+			return 0;
+	}
+	return 1;
 }
 
 #define MEMC_CHECK_KEY(intern, key)                                               \
 	if (UNEXPECTED(ZSTR_LEN(key) == 0 ||                                          \
 		ZSTR_LEN(key) > MEMC_OBJECT_KEY_MAX_LENGTH ||                             \
 		(memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL) \
-				? !s_memc_valid_key_binary(ZSTR_VAL(key))                         \
-				: !s_memc_valid_key_ascii(ZSTR_VAL(key))                          \
+				? !s_memc_valid_key_binary(key)                                   \
+				: !s_memc_valid_key_ascii(key)                                    \
 		))) {                                                                     \
 		intern->rescode = MEMCACHED_BAD_KEY_PROVIDED;                             \
 		RETURN_FALSE;                                                             \
@@ -309,7 +315,7 @@ PHP_INI_MH(OnUpdateSessionPrefixString)
 			php_error_docref(NULL, E_WARNING, "memcached.sess_prefix too long (max: %d)", MEMCACHED_MAX_KEY - 1);
 			return FAILURE;
 		}
-		if (!s_memc_valid_key_ascii(ZSTR_VAL(new_value))) {
+		if (!s_memc_valid_key_ascii(new_value)) {
 			php_error_docref(NULL, E_WARNING, "memcached.sess_prefix cannot contain whitespace or control characters");
 			return FAILURE;
 		}

tests/keys_ascii.phpt

@@ -0,0 +1,190 @@
+--TEST--
+Test valid and invalid keys - ascii
+--SKIPIF--
+<?php include "skipif.inc";?>
+--FILE--
+<?php
+
+include dirname (__FILE__) . '/config.inc';
+$ascii = memc_get_instance (array (
+				Memcached::OPT_BINARY_PROTOCOL => false,
+				Memcached::OPT_VERIFY_KEY => false
+		));
+// libmemcached can verify keys, but these are tests are for our own
+// function s_memc_valid_key_ascii, so explicitly disable the checks
+// that libmemcached can perform.
+
+echo 'ASCII: SPACES' . PHP_EOL;
+var_dump ($ascii->set ('ascii key with spaces', 'this is a test'));
+var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
+
+echo 'ASCII: NEWLINE' . PHP_EOL;
+var_dump ($ascii->set ('asciikeywithnewline' . PHP_EOL, 'this is a test'));
+var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
+
+echo 'ASCII: EMPTY' . PHP_EOL;
+var_dump ($ascii->set (''/*empty key*/, 'this is a test'));
+var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
+
+echo 'ASCII: TOO LONG' . PHP_EOL;
+var_dump ($ascii->set (str_repeat ('1234567890', 512), 'this is a test'));
+var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
+
+echo 'ASCII: GET' . PHP_EOL;
+for ($i=0;$i<32;$i++) {
+	var_dump ($ascii->get ('asciikeywithnonprintablechar-' . chr($i) . '-here'));
+	var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
+}
+
+echo 'ASCII: SET' . PHP_EOL;
+for ($i=0;$i<32;$i++) {
+	var_dump ($ascii->set ('asciikeywithnonprintablechar-' . chr($i) . '-here', 'this is a test'));
+	var_dump ($ascii->getResultCode () == Memcached::RES_BAD_KEY_PROVIDED);
+}
+
+echo 'OK' . PHP_EOL;
+
+--EXPECT--
+ASCII: SPACES
+bool(false)
+bool(true)
+ASCII: NEWLINE
+bool(false)
+bool(true)
+ASCII: EMPTY
+bool(false)
+bool(true)
+ASCII: TOO LONG
+bool(false)
+bool(true)
+ASCII: GET
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+ASCII: SET
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+bool(false)
+bool(true)
+OK