Merge pull request #297 from tmenjo/fix-sheep-p-z-V-options

sheep: Validate -p/--port, -z/--zone and -V/--vnodes values strictly

include/util.h

@@ -575,4 +575,6 @@ static inline uint64_t clock_get_time(void)
 }
 
 char *xstrdup(const char *s);
+uint32_t str_to_u32(const char *nptr);
+uint16_t str_to_u16(const char *nptr);
 #endif

lib/util.c

@@ -648,3 +648,45 @@ char *xstrdup(const char *s)
 	return ret;
 }
 
+/*
+ * Convert a decimal string like as strtoll to uint32_t/uint16_t
+ *
+ * returns:
+ *   - a converted value if success i.e. neither negative value nor overflow
+ *   - undefined if something went wrong and set errno accordingly
+ *
+ * errno:
+ *   - 0 if success
+ *   - EINVAL if one of the following:
+ *       - nptr was an empty string
+ *       - there was an unconvertible character in nptr
+ *   - ERANGE if negative/positive overflow occurred
+ */
+uint32_t str_to_u32(const char *nptr)
+{
+	char *endptr;
+	errno = 0;
+	const long long conv = strtoll(nptr, &endptr, 10);
+	/* empty string or unconvertible character */
+	if (nptr == endptr || *endptr != '\0') {
+		errno = EINVAL;
+		return (uint32_t)conv;
+	}
+	/* negative value or overflow */
+	if (conv < 0LL || UINT32_MAX < conv) {
+		errno = ERANGE;
+		return UINT32_MAX;
+	}
+	return (uint32_t)conv;
+}
+
+uint16_t str_to_u16(const char *nptr)
+{
+	const uint32_t conv = str_to_u32(nptr);
+	/* overflow */
+	if (UINT16_MAX < conv) {
+		errno = ERANGE;
+		return UINT16_MAX;
+	}
+	return (uint16_t)conv;
+}

sheep/sheep.c

@@ -697,7 +697,7 @@ int main(int argc, char **argv)
 	int ch, longindex, ret, port = SD_LISTEN_PORT, io_port = SD_LISTEN_PORT;
 	int rc = 1;
 	const char *dirp = DEFAULT_OBJECT_DIR, *short_options;
-	char *dir, *p, *pid_file = NULL, *bindaddr = NULL, log_path[PATH_MAX],
+	char *dir, *pid_file = NULL, *bindaddr = NULL, log_path[PATH_MAX],
 	     *argp = NULL;
 	bool explicit_addr = false;
 	bool daemonize = true;
@@ -730,9 +730,8 @@ int main(int argc, char **argv)
 				 &longindex)) >= 0) {
 		switch (ch) {
 		case 'p':
-			port = strtol(optarg, &p, 10);
-			if (optarg == p || port < 1 || UINT16_MAX < port
-				|| *p != '\0') {
+			port = str_to_u16(optarg);
+			if (errno != 0 || port < 1) {
 				sd_err("Invalid port number '%s'", optarg);
 				exit(1);
 			}
@@ -773,9 +772,8 @@ int main(int argc, char **argv)
 			nr_vnodes = 0;
 			break;
 		case 'z':
-			zone = strtol(optarg, &p, 10);
-			if (optarg == p || zone < 0 || UINT32_MAX < zone
-				|| *p != '\0') {
+			zone = str_to_u32(optarg);
+			if (errno != 0) {
 				sd_err("Invalid zone id '%s': must be "
 				       "an integer between 0 and %u", optarg,
 				       UINT32_MAX);
@@ -866,9 +864,8 @@ int main(int argc, char **argv)
 				sd_err("Options '-g' and '-V' can not be both specified");
 				exit(1);
 			}
-			nr_vnodes = strtol(optarg, &p, 10);
-			if (optarg == p || nr_vnodes < 1
-				|| UINT16_MAX < nr_vnodes || *p != '\0') {
+			nr_vnodes = str_to_u16(optarg);
+			if (errno != 0 || nr_vnodes < 1) {
 				sd_err("Invalid number of vnodes '%s': must be "
 					"an integer between 1 and %u",
 					optarg, UINT16_MAX);

tests/functional/108

@@ -0,0 +1,46 @@
+#!/bin/bash
+
+# Test sheep -p/--port
+
+. ./common
+
+err=0
+
+function testStartPortSucceeded
+{
+	port="$1"
+	mkdir -p "$STORE/0"
+	if $SHEEP "$STORE/0" -p "$port" ; then
+		sleep 1 # DO NOT use _wait_for_sheep
+		if grep daemon "$STORE/0/sheep.log" >/dev/null 2>&1 ; then
+			$DOG node list -p "$port"
+		else
+			err=1
+		fi
+	else
+		err=1
+	fi
+	_cleanup
+}
+
+function testStartPortFailed
+{
+	port="$1"
+	mkdir -p "$STORE/0"
+	$SHEEP "$STORE/0" -p "$port" && err=1
+	_cleanup
+}
+
+testStartPortSucceeded 7000
+testStartPortSucceeded 65535 # UINT16_MAX
+testStartPortFailed 0
+testStartPortFailed 65536 # UINT16_MAX + 1
+testStartPortFailed 65537 # UINT16_MAx + 2
+testStartPortFailed 4294967297 # UINT32_MAX + 2
+testStartPortFailed -1
+testStartPortFailed a
+testStartPortFailed 42a
+testStartPortFailed +
+testStartPortFailed -
+
+exit $err

tests/functional/108.out

@@ -0,0 +1,14 @@
+QA output created by 108
+  Id   Host:Port         V-Nodes       Zone
+   0   127.0.0.1:7000      	128   16777343
+  Id   Host:Port         V-Nodes       Zone
+   0   127.0.0.1:65535     	128   16777343
+Invalid port number '0'
+Invalid port number '65536'
+Invalid port number '65537'
+Invalid port number '4294967297'
+Invalid port number '-1'
+Invalid port number 'a'
+Invalid port number '42a'
+Invalid port number '+'
+Invalid port number '-'

tests/functional/109

@@ -0,0 +1,45 @@
+#!/bin/bash
+
+# Test sheep -z/--zone
+
+. ./common
+
+err=0
+
+function testStartZoneSucceeded
+{
+	zone="$1"
+	mkdir -p "$STORE/0"
+	if $SHEEP "$STORE/0" -z "$zone" ; then
+		_wait_for_sheep 1
+		if grep daemon "$STORE/0/sheep.log" >/dev/null 2>&1 ; then
+			$DOG node list
+		else
+			err=1
+		fi
+	else
+		err=1
+	fi
+	_cleanup
+}
+
+function testStartZoneFailed
+{
+	zone="$1"
+	mkdir -p "$STORE/0"
+	$SHEEP "$STORE/0" -z "$zone" && err=1
+	_cleanup
+}
+
+testStartZoneSucceeded 0
+testStartZoneSucceeded 1
+testStartZoneSucceeded 4294967295 # UINT32_MAX
+testStartZoneFailed 4294967296 # UINT32_MAX + 1
+testStartZoneFailed 9223372036854775808 # INT64_MAX + 1
+testStartZoneFailed -1
+testStartZoneFailed a
+testStartZoneFailed 42a
+testStartZoneFailed +
+testStartZoneFailed -
+
+exit $err

tests/functional/109.out

@@ -0,0 +1,14 @@
+QA output created by 109
+  Id   Host:Port         V-Nodes       Zone
+   0   127.0.0.1:7000      	128          0
+  Id   Host:Port         V-Nodes       Zone
+   0   127.0.0.1:7000      	128          1
+  Id   Host:Port         V-Nodes       Zone
+   0   127.0.0.1:7000      	128 4294967295
+Invalid zone id '4294967296': must be an integer between 0 and 4294967295
+Invalid zone id '9223372036854775808': must be an integer between 0 and 4294967295
+Invalid zone id '-1': must be an integer between 0 and 4294967295
+Invalid zone id 'a': must be an integer between 0 and 4294967295
+Invalid zone id '42a': must be an integer between 0 and 4294967295
+Invalid zone id '+': must be an integer between 0 and 4294967295
+Invalid zone id '-': must be an integer between 0 and 4294967295

tests/functional/110

@@ -0,0 +1,40 @@
+#!/bin/bash
+
+# Test sheep -V/--vnodes
+
+. ./common
+
+function testStartVnodesFailed
+{
+	( SHEEP_OPTIONS="-V $1" _start_sheep 0 ) && :
+	_wait_for_sheep_stop 0
+	_cleanup
+}
+
+function testStartVnodesSucceeded
+{
+	SHEEP_OPTIONS="-V $1" _start_sheep 0
+	_wait_for_sheep 1
+	$DOG node list
+	_cleanup
+}
+
+function testStartVnodesGatewayFailed
+{
+	( SHEEP_OPTIONS='-g -V 42' _start_sheep 0 ) && :
+	_wait_for_sheep_stop 0
+	_cleanup
+}
+
+testStartVnodesSucceeded 1
+testStartVnodesSucceeded 65535 # UINT16_MAX
+testStartVnodesFailed 0
+testStartVnodesFailed 65536 # UINT16_MAX + 1
+testStartVnodesFailed 65537 # UINT16_MAX + 2
+testStartVnodesFailed 4294967297 # UINT32_MAX + 2
+testStartVnodesFailed -1
+testStartVnodesFailed a
+testStartVnodesFailed 42a
+testStartVnodesFailed +
+testStartVnodesFailed -
+testStartVnodesGatewayFailed

tests/functional/110.out

@@ -0,0 +1,25 @@
+QA output created by 110
+  Id   Host:Port         V-Nodes       Zone
+   0   127.0.0.1:7000      	 1          0
+  Id   Host:Port         V-Nodes       Zone
+   0   127.0.0.1:7000      	65535          0
+Invalid number of vnodes '0': must be an integer between 1 and 65535
+cannot start sheep 0
+Invalid number of vnodes '65536': must be an integer between 1 and 65535
+cannot start sheep 0
+Invalid number of vnodes '65537': must be an integer between 1 and 65535
+cannot start sheep 0
+Invalid number of vnodes '4294967297': must be an integer between 1 and 65535
+cannot start sheep 0
+Invalid number of vnodes '-1': must be an integer between 1 and 65535
+cannot start sheep 0
+Invalid number of vnodes 'a': must be an integer between 1 and 65535
+cannot start sheep 0
+Invalid number of vnodes '42a': must be an integer between 1 and 65535
+cannot start sheep 0
+Invalid number of vnodes '+': must be an integer between 1 and 65535
+cannot start sheep 0
+Invalid number of vnodes '-': must be an integer between 1 and 65535
+cannot start sheep 0
+Options '-g' and '-V' can not be both specified
+cannot start sheep 0

tests/functional/group

@@ -115,3 +115,6 @@
 105 auto quick vdi
 106 auto quick vdi
 107 auto quick dog
+108 auto quick
+109 auto quick
+110 auto quick