Patch stomp-patch-content-length for stomp Bug #64670

Patch version 2013-04-18 18:41 UTC

Obsoleted by patches:

• stomp-patch-content-length, revision 2013-05-06 15:35 UTC

• 2013-05-06 21:20 UTC [diff to current]
• 2013-05-06 15:35 UTC [diff to current]
• 2013-04-18 18:41 UTC [diff to current]

Developer: mi+php@aldan.algebra.com

--- stomp.h	2012-11-18 17:35:40.000000000 -0500
+++ stomp.h	2013-04-17 16:41:22.000000000 -0400
@@ -83,5 +83,5 @@
 int stomp_valid_receipt(stomp_t *connection, stomp_frame_t *frame);
 int stomp_select(stomp_t *connection);
-void stomp_set_error(stomp_t *stomp, const char *error, int errnum, const char *details);
+void stomp_set_error(stomp_t *stomp, const char *error, int errnum, const char *fmt, ...) ZEND_ATTRIBUTE_PTR_FORMAT(printf, 4, 0);
 void stomp_free_frame(stomp_frame_t *frame);
 #endif /* _STOMP_H_ */
--- stomp.c	2012-11-18 17:35:40.000000000 -0500
+++ stomp.c	2013-04-18 12:22:03.000000000 -0400
@@ -36,5 +36,6 @@
 /* {{{ stomp_init
  */
-stomp_t *stomp_init() 
+stomp_t *
+stomp_init()
 {
 	/* Memory allocation */
@@ -67,5 +68,6 @@
 /* {{{ stomp_frame_buffer_push
  */
-void stomp_frame_buffer_push(stomp_frame_cell_t **pcell, stomp_frame_t *frame)
+static void
+stomp_frame_buffer_push(stomp_frame_cell_t **pcell, stomp_frame_t *frame)
 {
 	stomp_frame_cell_t *cell = (stomp_frame_cell_t *) emalloc(sizeof(stomp_frame_cell_t));
@@ -85,5 +87,6 @@
 /* {{{ stomp_frame_buffer_shift
  */
-stomp_frame_t *stomp_frame_buffer_shift(stomp_frame_cell_t **pcell) {
+static stomp_frame_t *
+stomp_frame_buffer_shift(stomp_frame_cell_t **pcell) {
 	stomp_frame_t *frame = NULL;
 	if (*pcell) {
@@ -99,5 +102,6 @@
 /* {{{ stomp_frame_buffer_clear
  */
-void stomp_frame_buffer_clear(stomp_frame_cell_t **pcell) {
+static void
+stomp_frame_buffer_clear(stomp_frame_cell_t **pcell) {
 	stomp_frame_t *frame = NULL;
 	while (frame = stomp_frame_buffer_shift(pcell)) efree(frame);
@@ -105,12 +109,17 @@
 /* }}} */
 
-/* {{{ stomp_set_error 
+/* {{{ stomp_set_error
  */
-void stomp_set_error(stomp_t *stomp, const char *error, int errnum, const char *details) 
+void
+stomp_set_error(stomp_t *stomp, const char *error, int errnum,
+    const char *fmt, ...)
 {
+	va_list	ap;
+	int	len;
+
 	if (stomp->error != NULL) {
 		efree(stomp->error);
 		stomp->error = NULL;
-	}   
+	}
 	if (stomp->error_details != NULL) {
 		efree(stomp->error_details);
@@ -121,13 +130,26 @@
 		stomp->error = estrdup(error);
 	}
-	if (details != NULL) {
-		stomp->error_details = estrdup(details);
+	if (fmt != NULL) {
+		stomp->error_details = emalloc(STOMP_BUFSIZE);
+		if (stomp->error_details == NULL)
+			return; /* Nothing else can be done */
+		va_start(ap, fmt);
+		/*
+		 * Would've been better to call vasprintf(), but that
+		 * function is missing on some platforms...
+		 */
+		len = vsnprintf(stomp->error_details, STOMP_BUFSIZE, fmt, ap);
+		va_end(ap);
+		if (len < STOMP_BUFSIZE) /* shrink the buffer down */
+			stomp->error_details =
+			    erealloc(stomp->error_details, len + 1);
 	}
 }
 /* }}} */
 
-/* {{{ stomp_writeable 
+/* {{{ stomp_writeable
  */
-int stomp_writeable(stomp_t *stomp) 
+static int
+stomp_writeable(stomp_t *stomp)
 {
 	int     n;
@@ -147,7 +169,8 @@
 /* }}} */
 
-/* {{{ stomp_connect 
+/* {{{ stomp_connect
  */
-int stomp_connect(stomp_t *stomp, const char *host, unsigned short port TSRMLS_DC)
+int
+stomp_connect(stomp_t *stomp, const char *host, unsigned short port TSRMLS_DC)
 {
 	char error[1024];
@@ -159,5 +182,5 @@
 		efree(stomp->host);
 	}
-	stomp->host = (char *) emalloc(strlen(host) + 1);
+	stomp->host = emalloc(strlen(host) + 1);
 	memcpy(stomp->host, host, strlen(host));
 	stomp->host[strlen(host)] = '\0';
@@ -171,5 +194,5 @@
 	if (stomp->fd == -1) {
 		snprintf(error, sizeof(error), "Unable to connect to %s:%ld", stomp->host, stomp->port);
-		stomp_set_error(stomp, error, errno, NULL);
+		stomp_set_error(stomp, error, errno, "%s", strerror(errno));
 		return 0;
 	}
@@ -179,6 +202,6 @@
 	if (getsockname(stomp->fd, (struct sockaddr*) &stomp->localaddr, &size) == -1) {
 		snprintf(error, sizeof(error), "getsockname failed: %s (%d)", strerror(errno), errno);
-		stomp_set_error(stomp, error, errno, NULL); 
-		return 0; 
+		stomp_set_error(stomp, error, errno, NULL);
+		return 0;
 	}
 
@@ -187,4 +210,6 @@
 		if (stomp->options.use_ssl) {
 			SSL_CTX *ctx = SSL_CTX_new(SSLv23_client_method());
+			int	 ret;
+
 			if (NULL == ctx) {
 				stomp_set_error(stomp, "failed to create the SSL context", 0, NULL);
@@ -200,18 +225,19 @@
 				return 0;
 			}
-			
+
 			SSL_set_fd(stomp->ssl_handle, stomp->fd);
 
-			if (SSL_connect(stomp->ssl_handle) <= 0) {
-				stomp_set_error(stomp, "SSL/TLS handshake failed", 0, NULL);
+			if ((ret = SSL_connect(stomp->ssl_handle)) <= 0) {
+				stomp_set_error(stomp, "SSL/TLS handshake failed", 0,
+				    "SSL error %d", SSL_get_error(stomp->ssl_handle, ret));
 				SSL_shutdown(stomp->ssl_handle);
 				return 0;
 			}
 		}
-#endif        
+#endif
 		return 1;
 	} else {
 		snprintf(error, sizeof(error), "Unable to connect to %s:%ld", stomp->host, stomp->port);
-		stomp_set_error(stomp, error, errno, NULL); 
+		stomp_set_error(stomp, error, errno, "%s", strerror(errno));
 		return 0;
 	}
@@ -221,5 +247,6 @@
 /* {{{ stomp_close
  */
-void stomp_close(stomp_t *stomp)
+void
+stomp_close(stomp_t *stomp)
 {
 	if (NULL == stomp) {
@@ -254,5 +281,6 @@
 /* {{{ stomp_send
  */
-int stomp_send(stomp_t *stomp, stomp_frame_t *frame TSRMLS_DC)
+int
+stomp_send(stomp_t *stomp, stomp_frame_t *frame TSRMLS_DC)
 {
 	smart_str buf = {0};
@@ -265,15 +293,15 @@
 	if (frame->headers) {
 
-		char *key; 
+		char *key;
 		ulong pos;
 		zend_hash_internal_pointer_reset(frame->headers);
 
 		while (zend_hash_get_current_key(frame->headers, &key, &pos, 0) == HASH_KEY_IS_STRING) {
-			char *value = NULL;
+			void *value = NULL;
 
 			smart_str_appends(&buf, key);
 			smart_str_appendc(&buf, ':');
 
-			if (zend_hash_get_current_data(frame->headers, (void **)&value) == SUCCESS) {
+			if (zend_hash_get_current_data(frame->headers, &value) == SUCCESS) {
 				smart_str_appends(&buf, value);
 			}
@@ -298,8 +326,6 @@
 
 	if (!stomp_writeable(stomp)) {
-		char error[1024];
-		snprintf(error, sizeof(error), "Unable to send data");
-		stomp_set_error(stomp, error, errno, NULL);
 		smart_str_free(&buf);
+		stomp_set_error(stomp, "Unable to send data", errno, "%s", strerror(errno));
 		return 0;
 	}
@@ -307,23 +333,23 @@
 #ifdef HAVE_STOMP_SSL
 	if (stomp->options.use_ssl) {
-		if (-1 == SSL_write(stomp->ssl_handle, buf.c, buf.len) || -1 == SSL_write(stomp->ssl_handle, "\0\n", 2)) {
-			char error[1024];
-			snprintf(error, sizeof(error), "Unable to send data");
-			stomp_set_error(stomp, error, errno, NULL);
+		int	ret;
+		if (-1 == (ret = SSL_write(stomp->ssl_handle, buf.c, buf.len)) ||
+		    -1 == (ret = SSL_write(stomp->ssl_handle, "\0\n", 2))) {
 			smart_str_free(&buf);
+			stomp_set_error(stomp, "Unable to send data", errno,
+			    "SSL error %d", SSL_get_error(stomp->ssl_handle, ret));
 			return 0;
 		}
 	} else {
-#endif        
+#endif
 		if (-1 == send(stomp->fd, buf.c, buf.len, 0) || -1 == send(stomp->fd, "\0\n", 2, 0)) {
-			char error[1024];
-			snprintf(error, sizeof(error), "Unable to send data");
-			stomp_set_error(stomp, error, errno, NULL);
 			smart_str_free(&buf);
+			stomp_set_error(stomp, "Unable to send data", errno, "%s",
+			    strerror(errno));
 			return 0;
 		}
 #ifdef HAVE_STOMP_SSL
 	}
-#endif        
+#endif
 
 	smart_str_free(&buf);
@@ -335,5 +361,6 @@
 /* {{{ stomp_recv
  */
-int stomp_recv(stomp_t *stomp, char *msg, size_t length)
+static int
+stomp_recv(stomp_t *stomp, char *msg, size_t length)
 {
 	int len;
@@ -349,8 +376,23 @@
 #endif
 
-	if (len == 0) {
-		TSRMLS_FETCH();
-		zend_throw_exception_ex(stomp_ce_exception, errno TSRMLS_CC, "Unexpected EOF while reading from socket");
+	/* -1 means error, 0 means normal shutdown, but for us it is all error */
+	switch (len) {
+	case -1:
+		stomp_set_error(stomp, "Error reading from socket", errno,
+		    "%s. (SSL %sin use)",
+		    strerror(errno),
+#if HAVE_STOMP_SSL
+		    stomp->options.use_ssl ? "" :
+#else
+		    "not "
+#endif
+		);
+		stomp->status = -1;
+		break;
+	case 0:
+		stomp_set_error(stomp, "Sender closed connection unexpectedly",
+		    0, NULL);
 		stomp->status = -1;
+		break;
 	}
 	return len;
@@ -358,55 +400,77 @@
 /* }}} */
 
-/* {{{ stomp_read_buffer 
+/* {{{ stomp_recv_full
  */
-static int stomp_read_buffer(stomp_t *stomp, char **data)
+static int
+stomp_recv_full(stomp_t *stomp, char *msg, size_t length)
 {
-	int rc = 0;
+	int	i;
+	size_t	length_read = 0;
+
+	while (length_read < length) {
+		i = stomp_recv(stomp, msg + length_read, length - length_read);
+		switch (i) {
+		/*
+		 * stomp_recv already raised awareness
+		 */
+		case -1:
+			return i; /* error */
+		case 0:
+			return length_read; /* partial read */
+		}
+		length_read += i;
+	}
+	return length_read;
+}
+/* }}} */
+
+
+/* {{{ stomp_read_buffer
+ */
+static int
+stomp_read_buffer(stomp_t *stomp, char **data)
+{
+	int rc;
 	size_t i = 0;
 	size_t bufsize = STOMP_BUFSIZE + 1;
-	char *buffer = (char *) emalloc(STOMP_BUFSIZE + 1);
+	char *buffer = emalloc(STOMP_BUFSIZE + 1);
 
 	while (1) {
-
-		size_t length = 1;
-		rc = stomp_recv(stomp, buffer + i, length);
+		rc = stomp_recv(stomp, buffer + i, 1);
 		if (rc < 1) {
-			efree(buffer);
+			efree(buffer); /* stomp_recv already threw */
 			return -1;
 		}
 
-		if (1 == length) {
-			i++;
-
-			if (buffer[i-1] == 0) {
-				char endline[1];
-				if (1 != stomp_recv(stomp, endline, 1) && '\n' != endline[0]) {
-					efree(buffer);
-					return 0;
-				}
-				break;
+		if (buffer[i] == '\0') {
+			char endline[1];
+			if (1 != stomp_recv(stomp, endline, 1) && '\n' != endline[0]) {
+				efree(buffer);
+				stomp_set_error(stomp, "Protocol violation", 0,
+				    "The byte after nil is %hu, rather than newline",
+				    (unsigned short)endline[0]);
+				return 0;
 			}
+			break;
+		}
 
-			if (i >= bufsize) {
-				buffer = (char *) erealloc(buffer, bufsize + STOMP_BUFSIZE);
-				bufsize += STOMP_BUFSIZE;
-			}
+		i++;
 
+		if (i >= bufsize) {
+			buffer = erealloc(buffer, bufsize + STOMP_BUFSIZE);
+			bufsize += STOMP_BUFSIZE;
 		}
 	}
 
-	if (i > 1) {
-		*data = (char *) emalloc(i);
+	if (i > 0) {
+		*data = erealloc(buffer, i + 1);
 		if (NULL == *data) {
 			efree(buffer);
 			return -1;
 		}
+	} else
+		efree(buffer);
 
-		memcpy(*data, buffer, i);
-	}
-
-	efree(buffer);
-
-	return i-1;
+	return i;
 }
 /* }}} */
@@ -414,15 +478,14 @@
 /* {{{ stomp_read_line
  */
-static int stomp_read_line(stomp_t *stomp, char **data)
+static int
+stomp_read_line(stomp_t *stomp, char **data)
 {
-	int rc = 0;
+	int rc;
 	size_t i = 0;
 	size_t bufsize = STOMP_BUFSIZE + 1;
-	char *buffer = (char *) emalloc(STOMP_BUFSIZE + 1);
+	char *buffer = emalloc(STOMP_BUFSIZE + 1);
 
 	while (1) {
-
-		size_t length = 1;
-		rc = stomp_recv(stomp, buffer + i, length);
+		rc = stomp_recv(stomp, buffer + i, 1);
 		if (rc < 1) {
 			efree(buffer);
@@ -430,36 +493,32 @@
 		}
 
-		if (1 == length) {
-			i++; 
+		if (buffer[i] == '\n') {
+			buffer[i] = '\0';
+			break;
+		} else if (buffer[i] == '\0') {
+			efree(buffer);
+			stomp_set_error(stomp, "Protocol violation", 0,
+			    "Sender sent 0-byte before the newline");
+			return 0;
+		}
 
-			if (buffer[i-1] == '\n') {
-				buffer[i-1] = 0;
-				break;
-			} else if (buffer[i-1] == 0) {
-				efree(buffer);
-				return 0;
-			}
+		i++;
 
-			if (i >= bufsize) {
-				buffer = (char *) erealloc(buffer, bufsize + STOMP_BUFSIZE);
-				bufsize += STOMP_BUFSIZE;
-			}
+		if (i >= bufsize) {
+			buffer = erealloc(buffer, bufsize + STOMP_BUFSIZE);
+			bufsize += STOMP_BUFSIZE;
 		}
-
 	}
 
-	if (i > 1) {
-		*data = (char *) emalloc(i);
+	if (i > 0) {
+		*data = erealloc(buffer, i + 1);
 		if (NULL == *data) {
 			efree(buffer);
 			return -1;
 		}
+	} else
+		efree(buffer);
 
-		memcpy(*data, buffer, i);
-	}
-
-	efree(buffer);
-
-	return i-1;
+	return i;
 }
 /* }}} */
@@ -467,5 +526,6 @@
 /* {{{ stomp_free_frame
  */
-void stomp_free_frame(stomp_frame_t *frame)
+void
+stomp_free_frame(stomp_frame_t *frame)
 {
 	if (frame) {
@@ -485,11 +545,13 @@
 /* }}} */
 
-/* {{{ stomp_read_frame 
+/* {{{ stomp_read_frame
  */
-stomp_frame_t *stomp_read_frame(stomp_t *stomp)
+stomp_frame_t *
+stomp_read_frame(stomp_t *stomp)
 {
 	stomp_frame_t *f = NULL;
-	char *cmd = NULL, *length_str = NULL;
-	int length = 0;
+	char *cmd = NULL;
+	int length;
+	char endbuffer[2];
 
 	if (stomp->buffer) {
@@ -507,4 +569,6 @@
 	}
 
+	f->body_length = -1;
+
 	/* Parse the command */
 	length = stomp_read_line(stomp, &cmd);
@@ -518,54 +582,76 @@
 	/* Parse the header */
 	while (1) {
-		char *p = NULL;
+		char	*p, *p2, *key, *value;
+		size_t	 keylen, vallen;
+
 		length = stomp_read_line(stomp, &p);
-		
+
 		if (length < 0) {
 			RETURN_READ_FRAME_FAIL;
 		}
 
-		if (0 == length) {
+		if (0 == length)
 			break;
-		} else {  
-			char *p2 = NULL;
-			char *key;
-			char *value;
-
-			p2 = strstr(p,":");
-			
-			if (p2 == NULL) {
-				efree(p);
-				RETURN_READ_FRAME_FAIL;
-			}
 
-			/* Null terminate the key */
-			*p2=0;
-			key = p;
+		p2 = strchr(p, ':');
 
-			/* The rest is the value. */
-			value = p2+1;
-
-			/* Insert key/value into hash table. */
-			zend_hash_add(f->headers, key, strlen(key) + 1, value, strlen(value) + 1, NULL);
+		if (p2 == NULL) {
 			efree(p);
+			RETURN_READ_FRAME_FAIL;
 		}
-	}
 
-	/* Check for the content length */
-	if (zend_hash_find(f->headers, "content-length", sizeof("content-length"), (void **)&length_str) == SUCCESS) {
-		char endbuffer[2];
-		length = 2;
+		/* Null terminate the key */
+		*p2 = '\0';
+		key = p;
+		keylen = p2 - p;
+
+		/* The rest is the value. */
+		value = p2 + 1;
+		vallen = length - keylen - 1;
+
+		/* Check, if the header specifies content-length */
+		if (keylen == sizeof("content-length") - 1 &&
+		    strcmp("content-length", key) == 0) {
+			char	*ep;
+			long	 lbodylen = strtol(value, &ep, 0);
+
+			if (*ep != '\0' || lbodylen < 0 ||
+			    lbodylen > INT_MAX) {
+				stomp_set_error(stomp, "Protocol violation", 0,
+				    "Invalid content-length header %s", value);
+				RETURN_READ_FRAME_FAIL;
+			}
+			f->body_length = lbodylen;
+		}
 
-		f->body_length = atoi(length_str);
-		f->body = (char *) emalloc(f->body_length);
+		/* Insert key/value into hash table. */
+		zend_hash_add(f->headers, key, keylen + 1, value, vallen + 1, NULL);
+		efree(p);
+	}
 
-		if (-1 == stomp_recv(stomp, f->body, f->body_length)) {
+	/* Check for the content length */
+	switch (f->body_length) {
+	default: /* Some positive number given as content-length */
+		f->body = emalloc(f->body_length); /* XXX check for NULL? */
+		length = stomp_recv_full(stomp, f->body, f->body_length);
+		if (length <= 0)
+			RETURN_READ_FRAME_FAIL;
+		if (length != f->body_length) {
+			stomp_set_error(stomp, "Protocol violation", 0,
+			    "Read %d bytes of body instead of the %d promised "
+			    "by %s header", length, f->body_length,
+			    "content-length");
 			RETURN_READ_FRAME_FAIL;
 		}
-
-		if (length != stomp_recv(stomp, endbuffer, length) || endbuffer[0] != '\0' || endbuffer[1] != '\n') {
+		/* FALLTHROUGH */
+	case 0:	/* Content-length is explicitly specified as zero */
+		if (stomp_recv(stomp, endbuffer, 2) != 2 ||
+		    endbuffer[0] != '\0' || endbuffer[1] != '\n') {
+			stomp_set_error(stomp, "Protocol violation", 0,
+			    "Could not read the two closing bytes of the frame");
 			RETURN_READ_FRAME_FAIL;
 		}
-	} else {
+		break;
+	case -1: /* Content-length not found among the headers */
 		f->body_length = stomp_read_buffer(stomp, &f->body);
 	}
@@ -577,10 +663,10 @@
 /* {{{ stomp_valid_receipt
  */
-int stomp_valid_receipt(stomp_t *stomp, stomp_frame_t *frame) {
+int
+stomp_valid_receipt(stomp_t *stomp, stomp_frame_t *frame) {
 	int success = 1;
-	char error[1024];
-	char *receipt = NULL;
+	void *receipt = NULL;
 
-	if (zend_hash_find(frame->headers, "receipt", sizeof("receipt"), (void **)&receipt) == SUCCESS) {
+	if (zend_hash_find(frame->headers, "receipt", sizeof("receipt"), &receipt) == SUCCESS) {
 		stomp_frame_cell_t *buffer = NULL;
 		success = 0;
@@ -589,12 +675,11 @@
 			if (res) {
 				if (0 == strncmp("RECEIPT", res->command, sizeof("RECEIPT") - 1)) {
-					char *receipt_id = NULL;
-					if (zend_hash_find(res->headers, "receipt-id", sizeof("receipt-id"), (void **)&receipt_id) == SUCCESS
+					void *receipt_id = NULL;
+					if (zend_hash_find(res->headers, "receipt-id", sizeof("receipt-id"), &receipt_id) == SUCCESS
 							&& strlen(receipt) == strlen(receipt_id)
 							&& !strcmp(receipt, receipt_id)) {
 						success = 1;
 					} else {
-						snprintf(error, sizeof(error), "Unexpected receipt id : %s", receipt_id);
-						stomp_set_error(stomp, error, 0, NULL);
+						stomp_set_error(stomp, "Unexpected receipt id", 0, "%s", receipt_id);
 					}
 					stomp_free_frame(res);
@@ -602,8 +687,8 @@
 					return success;
 				} else if (0 == strncmp("ERROR", res->command, sizeof("ERROR") - 1)) {
-					char *error_msg = NULL;
-					if (zend_hash_find(res->headers, "message", sizeof("message"), (void **)&error_msg) == SUCCESS) {
-						stomp_set_error(stomp, error_msg, 0, res->body);
-					}
+					void *error_msg = NULL;
+					zend_hash_find(res->headers, "message", sizeof("message"), &error_msg);
+					stomp_set_error(stomp, error_msg ? error_msg : "ERROR", 0,
+					    res->body ? "%s" : NULL, res->body);
 					stomp_free_frame(res);
 					stomp->buffer = buffer;
@@ -624,5 +709,6 @@
 /* {{{ stomp_select
  */
-int stomp_select(stomp_t *stomp)
+int
+stomp_select(stomp_t *stomp)
 {
 	int     n;
@@ -639,8 +725,8 @@
 	if (n < 1) {
 #if !defined(PHP_WIN32) && !(defined(NETWARE) && defined(USE_WINSOCK))
-		if (n == 0) { 
+		if (n == 0) {
 			errno = ETIMEDOUT;
-		}   
-#endif          
+		}
+#endif
 		return 0;
 	}