Update warning for touch command in binary protocol mode with libmemcached < 1.0.18 (#322)

sodabrew · web-flow · commit f7c92b06997d · 2017-02-12T15:56:22.000Z
diff --git a/package.xml b/package.xml
@@ -157,6 +157,7 @@ Fixes
     <file role='test' name='gh_155.phpt'/>
     <file role='test' name='get_flags.phpt'/>
     <file role='test' name='session_lock.phpt'/>
+    <file role='test' name='session_lazy_warning.phpt'/>
     <file role='test' name='session_regenerate.phpt'/>
     <file role='test' name='stats.phpt'/>
     <file role='test' name='default_behavior.phpt'/>
diff --git a/php_libmemcached_compat.c b/php_libmemcached_compat.c
@@ -35,3 +35,24 @@ memcached_return php_memcached_exist (memcached_st *memc, zend_string *key)
 	return rc;
 #endif
 }
+
+memcached_return php_memcached_touch(memcached_st *memc, const char *key, size_t key_len, time_t expiration)
+{
+#if defined(LIBMEMCACHED_VERSION_HEX) && LIBMEMCACHED_VERSION_HEX < 0x01000018
+	if (memcached_behavior_get(memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL)) {
+		php_error_docref(NULL, E_WARNING, "using touch command with binary protocol is not recommended with libmemcached versions below 1.0.18, please use ascii protocol or upgrade libmemcached");
+	}
+#endif
+	return memcached_touch(memc, key, key_len, expiration);
+}
+
+memcached_return php_memcached_touch_by_key(memcached_st *memc, const char *server_key, size_t server_key_len, const char *key, size_t key_len, time_t expiration)
+{
+#if defined(LIBMEMCACHED_VERSION_HEX) && LIBMEMCACHED_VERSION_HEX < 0x01000018
+	if (memcached_behavior_get(memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL)) {
+		php_error_docref(NULL, E_WARNING, "using touch command with binary protocol is not recommended with libmemcached versions below 1.0.18, please use ascii protocol or upgrade libmemcached");
+	}
+#endif
+	return memcached_touch_by_key(memc, server_key, server_key_len, key, key_len, expiration);
+}
+
diff --git a/php_libmemcached_compat.h b/php_libmemcached_compat.h
@@ -22,6 +22,9 @@
 
 memcached_return php_memcached_exist (memcached_st *memc, zend_string *key);
 
+memcached_return php_memcached_touch(memcached_st *memc, const char *key, size_t key_len, time_t expiration);
+memcached_return php_memcached_touch_by_key(memcached_st *memc, const char *server_key, size_t server_key_len, const char *key, size_t key_len, time_t expiration);
+
 #if defined(LIBMEMCACHED_VERSION_HEX) && LIBMEMCACHED_VERSION_HEX >= 0x01000017
 typedef const memcached_instance_st * php_memcached_instance_st;
 #else
diff --git a/php_memcached.c b/php_memcached.c
@@ -1081,7 +1081,7 @@ zend_bool s_memc_write_zval (php_memc_object_t *intern, php_memc_write_op op, ze
 			break;
 
 			case MEMC_OP_TOUCH:
-				status = memcached_touch_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), expiration);
+				status = php_memcached_touch_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), expiration);
 			break;
 			
 			case MEMC_OP_ADD:
@@ -1113,7 +1113,7 @@ zend_bool s_memc_write_zval (php_memc_object_t *intern, php_memc_write_op op, ze
 			break;
 
 			case MEMC_OP_TOUCH:
-				status = memcached_touch(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), expiration);
+				status = php_memcached_touch(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), expiration);
 			break;
 			
 			case MEMC_OP_ADD:
@@ -1959,15 +1959,6 @@ static void php_memc_store_impl(INTERNAL_FUNCTION_PARAMETERS, int op, zend_bool
 		}
 	}
 
-
-	if (op == MEMC_OP_TOUCH) {
-#if defined(LIBMEMCACHED_VERSION_HEX) && LIBMEMCACHED_VERSION_HEX < 0x01000016
-		if (memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL)) {
-			php_error_docref(NULL, E_WARNING, "using touch command with binary protocol is not recommended with libmemcached versions below 1.0.16");
-		}
-#endif
-	}
-
 	if (!s_memc_write_zval (intern, op, server_key, key, value, expiration)) {
 		RETURN_FALSE;
 	}
diff --git a/php_memcached_session.c b/php_memcached_session.c
@@ -542,7 +542,7 @@ PS_UPDATE_TIMESTAMP_FUNC(memcached)
 	memcached_st *memc = PS_GET_MOD_DATA();
 	time_t expiration = s_session_expiration(maxlifetime);
 
-	if (memcached_touch(memc, key->val, key->len, expiration) == MEMCACHED_FAILURE) {
+	if (php_memcached_touch(memc, key->val, key->len, expiration) == MEMCACHED_FAILURE) {
 		return FAILURE;
 	}
 	return SUCCESS;
diff --git a/tests/gh_155.phpt b/tests/gh_155.phpt
@@ -4,15 +4,18 @@ Test for bug 155
 <?php 
 $min_version = "1.4.8";
 include dirname(__FILE__) . "/skipif.inc";
-if (Memcached::LIBMEMCACHED_VERSION_HEX < 0x01000016) die ('skip too old libmemcached');
+// The touch command in binary mode will work in libmemcached 1.0.16, but runs out the timeout clock
+// See https://github.com/php-memcached-dev/php-memcached/issues/310 for further explanation
+// The problem is fixed fully in libmemcached 1.0.18, so we'll focus tests on that version
+if (Memcached::LIBMEMCACHED_VERSION_HEX < 0x01000018) die ('skip too old libmemcached');
 ?>
 --FILE--
 <?php
 include dirname (__FILE__) . '/config.inc';
 
 $m = new Memcached ();
 
-$m->setOption(Memcached::OPT_BINARY_PROTOCOL,true);
+$m->setOption(Memcached::OPT_BINARY_PROTOCOL, true);
 $m->addServer(MEMC_SERVER_HOST, MEMC_SERVER_PORT);
 
 $key = 'bug_155_' . uniqid();
diff --git a/tests/session_basic.phpt b/tests/session_basic.phpt
@@ -7,6 +7,7 @@ if (!Memcached::HAVE_SESSION) print "skip";
 ?>
 --INI--
 session.save_handler = memcached
+memcached.sess_binary_protocol = Off
 --FILE--
 <?php
 include dirname (__FILE__) . '/config.inc';
diff --git a/tests/session_basic2.phpt b/tests/session_basic2.phpt
@@ -7,6 +7,7 @@ if (!Memcached::HAVE_SESSION) print "skip";
 ?>
 --INI--
 session.save_handler = memcached
+memcached.sess_binary_protocol = Off
 --FILE--
 <?php
 include dirname (__FILE__) . '/config.inc';
diff --git a/tests/session_basic3.phpt b/tests/session_basic3.phpt
@@ -7,6 +7,7 @@ if (!Memcached::HAVE_SESSION) print "skip";
 ?>
 --INI--
 session.save_handler = memcached
+memcached.sess_binary_protocol = Off
 --FILE--
 <?php
 include dirname (__FILE__) . '/config.inc';
diff --git a/tests/session_lazy_warning.phpt b/tests/session_lazy_warning.phpt
@@ -0,0 +1,47 @@
+--TEST--
+Session lazy binary warning old libmemcached
+--SKIPIF--
+<?php
+include dirname(__FILE__) . "/skipif.inc";
+if (!Memcached::HAVE_SESSION) print "skip";
+if (Memcached::LIBMEMCACHED_VERSION_HEX >= 0x01000018) die ('skip too old libmemcached');
+?>
+--INI--
+session.save_handler = memcached
+memcached.sess_binary_protocol = On
+--FILE--
+<?php
+include dirname (__FILE__) . '/config.inc';
+ini_set ('session.save_path', MEMC_SERVER_HOST . ':' . MEMC_SERVER_PORT);
+
+ob_start();
+
+session_start(['lazy_write'=>TRUE]);
+$_SESSION['foo'] = 1;
+session_write_close();
+
+$_SESSION = NULL;
+
+var_dump($_SESSION);
+session_start();
+var_dump($_SESSION);
+session_write_close();
+
+session_start();
+session_destroy();
+
+session_start();
+var_dump($_SESSION);
+session_write_close();
+
+
+--EXPECTF--
+NULL
+array(1) {
+  ["foo"]=>
+  int(1)
+}
+
+Warning: session_write_close(): using touch command with binary protocol is not recommended with libmemcached versions below 1.0.18, please use ascii protocol or upgrade libmemcached in %s on line %d
+array(0) {
+}
diff --git a/tests/session_lock-php71.phpt b/tests/session_lock-php71.phpt
@@ -14,6 +14,10 @@ memcached.sess_lock_wait_max = 1000
 memcached.sess_lock_retries  = 3
 memcached.sess_prefix        = "memc.test."
 
+# Turn off binary protocol while the test matrix has older versions of
+# libmemcached for which the extension warns of a broken touch command.
+memcached.sess_binary_protocol = Off
+
 session.save_handler = memcached
 
 --FILE--
diff --git a/tests/session_lock.phpt b/tests/session_lock.phpt
@@ -14,6 +14,10 @@ memcached.sess_lock_wait_max = 1000
 memcached.sess_lock_retries  = 3
 memcached.sess_prefix        = "memc.test."
 
+# Turn off binary protocol while the test matrix has older versions of
+# libmemcached for which the extension warns of a broken touch command.
+memcached.sess_binary_protocol = Off
+
 session.save_handler = memcached
 
 --FILE--
diff --git a/tests/touch_binary.phpt b/tests/touch_binary.phpt
@@ -4,7 +4,10 @@ Touch in binary mode
 <?php
 $min_version = "1.4.8"; //TOUCH is added since 1.4.8
 include dirname(__FILE__) . "/skipif.inc";
-if (Memcached::LIBMEMCACHED_VERSION_HEX < 0x01000016) die ('skip too old libmemcached');
+// The touch command in binary mode will work in libmemcached 1.0.16, but runs out the timeout clock
+// See https://github.com/php-memcached-dev/php-memcached/issues/310 for further explanation
+// The problem is fixed fully in libmemcached 1.0.18, so we'll focus tests on that version
+if (Memcached::LIBMEMCACHED_VERSION_HEX < 0x01000018) die ('skip too old libmemcached');
 ?>
 --FILE--
 <?php

Original file line number	Diff line number	Diff line change
`@@ -542,7 +542,7 @@ PS_UPDATE_TIMESTAMP_FUNC(memcached)`
`542`	`542`	`memcached_st *memc = PS_GET_MOD_DATA();`
`543`	`543`	`time_t expiration = s_session_expiration(maxlifetime);`
`544`	`544`
`545`		`- if (memcached_touch(memc, key->val, key->len, expiration) == MEMCACHED_FAILURE) {`
	`545`	`+ if (php_memcached_touch(memc, key->val, key->len, expiration) == MEMCACHED_FAILURE) {`
`546`	`546`	`return FAILURE;`
`547`	`547`	`}`
`548`	`548`	`return SUCCESS;`