php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #49447 php engine need to correctly check for socket API return status on windows
Submitted: 2009-09-03 01:36 UTC Modified: 2009-09-04 09:32 UTC
From: sriram dot natarajan at gmail dot com Assigned: srinatar
Status: Closed Package: Sockets related
PHP Version: 5.3.0 OS: win32 only - windows xp
Private report: No CVE-ID:
 [2009-09-03 01:36 UTC] sriram dot natarajan at gmail dot com
Description:
------------
Unlike bsd sockets, Win32 Socket API does not return -1 on failure for most of the common socket api calls like bind, accept, connect etc. in fact, the error status is another integer with numbers like 10035 etc. 
so, checking for returns status on these socket API's like -1 or <= 0 is wrong. 

i noticed this issue while debugging another problem on windows. hence, filing this bug to track this issue . checking for correct error status on these API is important during trouble shooting scenarios.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2009-09-03 01:39 UTC] srinatar@php.net
here is the patch to address this issue. 

Index: ext/openssl/xp_ssl.c
===================================================================
--- ext/openssl/xp_ssl.c	(revision 287975)
+++ ext/openssl/xp_ssl.c	(working copy)
@@ -259,6 +259,10 @@
 			SSL_CTX_free(sslsock->ctx);
 			sslsock->ctx = NULL;
 		}
+#ifdef PHP_WIN32
+		if (sslsock->s.socket == -1)
+			sslsock->s.socket = SOCK_ERR;
+#endif
 		if (sslsock->s.socket != SOCK_ERR) {
 #ifdef PHP_WIN32
 			/* prevent more data from coming in */
Index: ext/ftp/ftp.c
===================================================================
--- ext/ftp/ftp.c	(revision 287975)
+++ ext/ftp/ftp.c	(working copy)
@@ -147,7 +147,7 @@
 
 	size = sizeof(ftp->localaddr);
 	memset(&ftp->localaddr, 0, size);
-	if (getsockname(ftp->fd, (struct sockaddr*) &ftp->localaddr, &size) == -1) {
+	if (getsockname(ftp->fd, (struct sockaddr*) &ftp->localaddr, &size) != 0) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "getsockname failed: %s (%d)", strerror(errno), errno);
 		goto bail;
 	}
@@ -1387,7 +1387,7 @@
 
 	sa = (struct sockaddr *) &ftp->localaddr;
 	/* bind/listen */
-	if ((fd = socket(sa->sa_family, SOCK_STREAM, 0)) == -1) {
+	if ((fd = socket(sa->sa_family, SOCK_STREAM, 0)) == SOCK_ERR) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "socket() failed: %s (%d)", strerror(errno), errno);
 		goto bail;
 	}
@@ -1420,17 +1420,17 @@
 	php_any_addr(sa->sa_family, &addr, 0);
 	size = php_sockaddr_size(&addr);
 
-	if (bind(fd, (struct sockaddr*) &addr, size) == -1) {
+	if (bind(fd, (struct sockaddr*) &addr, size) != 0) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "bind() failed: %s (%d)", strerror(errno), errno);
 		goto bail;
 	}
 
-	if (getsockname(fd, (struct sockaddr*) &addr, &size) == -1) {
+	if (getsockname(fd, (struct sockaddr*) &addr, &size) != 0) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "getsockname() failed: %s (%d)", strerror(errno), errno);
 		goto bail;
 	}
 
-	if (listen(fd, 5) == -1) {
+	if (listen(fd, 5) != 0) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "listen() failed: %s (%d)", strerror(errno), errno);
 		goto bail;
 	}
Index: ext/sockets/sockets.c
===================================================================
--- ext/sockets/sockets.c	(revision 287975)
+++ ext/sockets/sockets.c	(working copy)
@@ -370,14 +370,14 @@
 
 	sock->type = PF_INET;
 
-	if (bind(sock->bsd_socket, (struct sockaddr *)&la, sizeof(la)) < 0) {
+	if (bind(sock->bsd_socket, (struct sockaddr *)&la, sizeof(la)) != 0) {
 		PHP_SOCKET_ERROR(sock, "unable to bind to given address", errno);
 		close(sock->bsd_socket);
 		efree(sock);
 		return 0;
 	}
 
-	if (listen(sock->bsd_socket, backlog) < 0) {
+	if (listen(sock->bsd_socket, backlog) != 0) {
 		PHP_SOCKET_ERROR(sock, "unable to listen on socket", errno);
 		close(sock->bsd_socket);
 		efree(sock);
Index: main/network.c
===================================================================
--- main/network.c	(revision 287975)
+++ main/network.c	(working copy)
@@ -314,7 +314,7 @@
 
 	SET_SOCKET_BLOCKING_MODE(sockfd, orig_flags);
 	
-	if ((n = connect(sockfd, addr, addrlen)) < 0) {
+	if ((n = connect(sockfd, addr, addrlen)) != 0) {
 		error = php_socket_errno();
 
 		if (error_code) {
@@ -348,7 +348,7 @@
 		   BSD-derived systems set errno correctly
 		   Solaris returns -1 from getsockopt in case of error
 		   */
-		if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) < 0) {
+		if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) != 0) {
 			ret = -1;
 		}
 	} else {
@@ -375,7 +375,7 @@
 	if (asynchronous) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Asynchronous connect() not supported on this platform");
 	}
-	return connect(sockfd, addr, addrlen);
+	return (connect(sockfd, addr, addrlen) == 0) ? 0 : -1;
 #endif
 }
 /* }}} */
@@ -715,7 +715,7 @@
 
 		clisock = accept(srvsock, (struct sockaddr*)&sa, &sl);
 
-		if (clisock >= 0) {
+		if (clisock != SOCK_ERR) {
 			php_network_populate_name_from_sockaddr((struct sockaddr*)&sa, sl,
 					textaddr, textaddrlen,
 					addr, addrlen
@@ -867,7 +867,7 @@
 					timeout ? &working_timeout : NULL,
 					error_string, error_code);
 
-			if (n != SOCK_CONN_ERR) {
+			if (n != -1) {
 				goto connected;
 			}
 
Index: main/streams/xp_socket.c
===================================================================
--- main/streams/xp_socket.c	(revision 287975)
+++ main/streams/xp_socket.c	(working copy)
@@ -181,6 +181,10 @@
 
 	if (close_handle) {
 
+#ifdef PHP_WIN32
+		if (sock->socket == -1)
+			sock->socket = SOCK_ERR;
+#endif
 		if (sock->socket != SOCK_ERR) {
 #ifdef PHP_WIN32
 			/* prevent more data from coming in */
@@ -312,7 +316,7 @@
 
 			switch (xparam->op) {
 				case STREAM_XPORT_OP_LISTEN:
-					xparam->outputs.returncode = listen(sock->socket, 5);
+					xparam->outputs.returncode = (listen(sock->socket, 5) == 0) ?  0: -1;
 					return PHP_STREAM_OPTION_RETURN_OK;
 
 				case STREAM_XPORT_OP_GET_NAME:
 [2009-09-04 07:59 UTC] svn@php.net
Automatic comment from SVN on behalf of srinatar
Revision: http://svn.php.net/viewvc/?view=revision&revision=288034
Log: - Fixed bug #49447 (php engine need to correctly check for socket API
  return status on windows). (Sriram Natarajan)
 [2009-09-04 09:32 UTC] srinatar@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

fix for this issue has been integrated
 [2009-10-09 14:20 UTC] svn@php.net
Automatic comment from SVN on behalf of pajoye
Revision: http://svn.php.net/viewvc/?view=revision&revision=289416
Log: - Merge: Fixed bug #49447 php engine need to correctly check for socket API
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Fri Apr 18 18:01:58 2014 UTC