[asterisk-commits] elguero: trunk r381894 - in /trunk: ./ res/res_agi.c

Fri Feb 22 13:40:06 CST 2013

Author: elguero
Date: Fri Feb 22 13:40:02 2013
New Revision: 381894

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=381894
Log:
Fix FastAGI To Properly Check For A Connection

When IPv6 support was added to FastAGI, the intent was to have the ability to
check all addresses resolved for a host since we might receive an IPv4 address
and an IPv6 address.  The problem with the current code, is that, since we are
doing O_NONBLOCK, we get EINPROGRESS when calling ast_connect() but are ignoring
this instead of handling it.  We break out of the loop and continue on.  When we
later call ast_poll(), it succeeds but we never check if we have a connection or
not on the socket level.  We then attempt to send data to the host address that
we think is setup and it fails.  We then check the errno and see that we have
"connection refused" and then return with agi failed.

This patch does the following:

* Handles EINPROGRESS by creating the function handle_connection()
  - ast_poll() was moved into this function
  - This function checks the results of the connection on the socket level after
    calling ast_poll()
* Continues to the next address if the above fails to create a connection
* Once all addresses resolved are tried and we still are unable to establish a
  connection, then we return that the FastAGI call failed

(closes issue ASTERISK-21065)
Reported by: Jeremy Kister
Tested by: Jeremy Kister, Michael L. Young
Patches:
  asterisk-21065_poll_correctly_v4.diff Michael L. Young (license 5026)

Review: https://reviewboard.asterisk.org/r/2330/
........

Merged revisions 381893 from http://svn.asterisk.org/svn/asterisk/branches/11

Modified:
    trunk/   (props changed)
    trunk/res/res_agi.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-11-merged' - no diff available.

Modified: trunk/res/res_agi.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_agi.c?view=diff&rev=381894&r1=381893&r2=381894
==============================================================================

--- trunk/res/res_agi.c (original)
+++ trunk/res/res_agi.c Fri Feb 22 13:40:02 2013
@@ -1539,12 +1539,61 @@
 #undef AMI_BUF_SIZE
 }
 
+/*!
+ * \internal
+ * \brief Handle the connection that was started by launch_netscript.
+ *
+ * \param agiurl Url that we are trying to connect to.
+ * \param addr Address that host was resolved to.
+ * \param netsockfd File descriptor of socket.
+ *
+ * \retval 0 when connection is succesful.
+ * \retval 1 when there is an error.
+ */
+static int handle_connection(const char *agiurl, const struct ast_sockaddr addr, const int netsockfd)
+{
+	struct pollfd pfds[1];
+	int res, conresult;
+	socklen_t reslen;
+
+	reslen = sizeof(conresult);
+
+	pfds[0].fd = netsockfd;
+	pfds[0].events = POLLOUT;
+
+	while ((res = ast_poll(pfds, 1, MAX_AGI_CONNECT)) != 1) {
+		if (errno != EINTR) {
+			if (!res) {
+				ast_log(LOG_WARNING, "FastAGI connection to '%s' timed out after MAX_AGI_CONNECT (%d) milliseconds.\n",
+					agiurl, MAX_AGI_CONNECT);
+			} else {
+				ast_log(LOG_WARNING, "Connect to '%s' failed: %s\n", agiurl, strerror(errno));
+			}
+
+			return 1;
+		}
+	}
+
+	if (getsockopt(pfds[0].fd, SOL_SOCKET, SO_ERROR, &conresult, &reslen) < 0) {
+		ast_log(LOG_WARNING, "Connection to %s failed with error: %s\n",
+			ast_sockaddr_stringify(&addr), strerror(errno));
+		return 1;
+	}
+
+	if (conresult) {
+		ast_log(LOG_WARNING, "Connecting to '%s' failed for url '%s': %s\n",
+			ast_sockaddr_stringify(&addr), agiurl, strerror(conresult));
+		return 1;
+	}
+
+	return 0;
+}
+
 /* launch_netscript: The fastagi handler.
 	FastAGI defaults to port 4573 */
 static enum agi_result launch_netscript(char *agiurl, char *argv[], int *fds)
 {
-	int s = 0, flags, res;
-	struct pollfd pfds[1];
+	int s = 0, flags;
 	char *host, *script;
 	int num_addrs = 0, i = 0;
 	struct ast_sockaddr *addrs;
@@ -1586,12 +1635,16 @@
 			continue;
 		}
 
-		if (ast_connect(s, &addrs[i]) && (errno != EINPROGRESS)) {
+		if (ast_connect(s, &addrs[i]) && errno == EINPROGRESS) {
+
+			if (handle_connection(agiurl, addrs[i], s)) {
+				close(s);
+				continue;
+			}
+
+		} else {
 			ast_log(LOG_WARNING, "Connection to %s failed with unexpected error: %s\n",
-				ast_sockaddr_stringify(&addrs[i]),
-				strerror(errno));
-			close(s);
-			continue;
+			ast_sockaddr_stringify(&addrs[i]), strerror(errno));
 		}
 
 		break;
@@ -1602,20 +1655,6 @@
 	if (i == num_addrs) {
 		ast_log(LOG_WARNING, "Couldn't connect to any host.  FastAGI failed.\n");
 		return AGI_RESULT_FAILURE;
-	}
-
-	pfds[0].fd = s;
-	pfds[0].events = POLLOUT;
-	while ((res = ast_poll(pfds, 1, MAX_AGI_CONNECT)) != 1) {
-		if (errno != EINTR) {
-			if (!res) {
-				ast_log(LOG_WARNING, "FastAGI connection to '%s' timed out after MAX_AGI_CONNECT (%d) milliseconds.\n",
-					agiurl, MAX_AGI_CONNECT);
-			} else
-				ast_log(LOG_WARNING, "Connect to '%s' failed: %s\n", agiurl, strerror(errno));
-			close(s);
-			return AGI_RESULT_FAILURE;
-		}
 	}
 
 	if (ast_agi_send(s, NULL, "agi_network: yes\n") < 0) {
@@ -1628,8 +1667,9 @@
 
 	/* If we have a script parameter, relay it to the fastagi server */
 	/* Script parameters take the form of: AGI(agi://my.example.com/?extension=${EXTEN}) */
-	if (!ast_strlen_zero(script))
+	if (!ast_strlen_zero(script)) {
 		ast_agi_send(s, NULL, "agi_network_script: %s\n", script);
+	}
 
 	ast_debug(4, "Wow, connected!\n");
 	fds[0] = s;


