A couple of bug reports mention problems with the get_host_and_port
function, so I've rewritten it to use memmove rather than strcpy (so
that we can safely use the same string as both the source and the
destination), and also to not unnecessarily destroy the address string
passed in.
I've also written some crude unit tests to verify that this function
works correctly now; running "make unittest" will create a sipp binary
that just runs some unit tests, complains about them if they fail, and
exits. (I expect this function to only be used by developers.)
Currently the only unit tests are of get_host_and_port, passing in:
* IPv6 address, not bracketed
* IPv6 address, bracketed, no port
* IPv6 address, bracketed, with port
* IPv4 address, no port
* IPv4 address, with port
* Hostname, no port
* Hostname, with port
There's certainly scope to test more and to improve the way we do this
testing (e.g. creating a separate sipp_ut binary), but I decided not to
let the perfect be the enemy of the good.
I'll leave this up for review for at least a week or two before checking
it in.
Best,
Rob
Index: sipp.cpp
===================================================================
--- sipp.cpp (revision 600)
+++ sipp.cpp (working copy)
@@ -622,7 +622,7 @@
return ip_addr;
}
-void get_host_and_port(char * addr, char * host, int * port)
+void get_host_and_port(const char * addr, char * host, int * port)
{
/* Separate the port number (if any) from the host name.
* Thing is, the separator is a colon (':'). The colon may also exist
@@ -630,32 +630,53 @@
* RFC 2732). If that's the case, then we need to skip past the IPv6
* address, which should be contained within square brackets ('[',']').
*/
- char *p;
- p = strchr( addr, '[' ); /* Look for '['. */
- if( p != NULL ) { /* If found, look for ']'. */
- p = strchr( p, ']' );
+ const char *cp;
+ char *p3;
+ char *p4;
+ int len;
+ cp = strchr( addr, '[' ); /* Look for '['. */
+ if( cp != NULL ) { /* If found, look for ']'. */
+ cp = strchr( cp, ']' );
}
- if( p == NULL ) { /* If '['..']' not found, */
- p = addr; /* scan the whole string. */
+ if( cp == NULL ) { /* If '['..']' not found, */
+ /* addr is not a []-enclosed IPv6 address, but might still be IPv6 (without
+ * a port), or IPv4 or a hostname (with or without a port) */
+ len = strlen(addr) + 1;
+ memmove(host, addr, len);
+ p3 = strchr( host, ':' );
+ if (p3 == NULL) {
+ /* No colon - just set the port to 0 */
+ *port = 0;
+ } else {
+ p4 = strchr( p3 + 1, ':' );
+ if (p4 != NULL) {
+ /* Found a second colon in addr - so this is an IPv6 address
+ * without a port. Set the port to 0 */
+ *port = 0;
+ } else {
+ /* IPv4 address or hostname with a colon in it - convert the colon to
+ * a NUL terminator, and set the value after it as the port */
+ *p3 = '\0';
+ *port = atol(p3 + 1);
+ }
+ }
+
} else { /* If '['..']' found, */
- char *p1; /* extract the remote_host */
+ const char *p1; /* extract the remote_host */
char *p2;
p1 = strchr( addr, '[' );
- p2 = strchr( addr, ']' );
+ len = strlen(p1) + 1;
+ memmove(host, p1 + 1, len);
+ p2 = strchr( host, ']' );
*p2 = '\0';
- strcpy(host, p1 + 1);
- *p2 = ']';
+ /* Check for a port specified after the ] */
+ p2 = strchr( p2 + 1, ':' );
+ if (p2 != NULL) {
+ *port = atol(p2 + 1);
+ } else {
+ *port = 0;
+ }
}
- /* Starting at <p>, which is either the start of the host substring
- * or the end of the IPv6 address, find the last colon character.
- */
- p = strchr( p, ':' );
- if( NULL != p ) {
- *p = '\0';
- *port = atol(p + 1);
- } else {
- *port = 0;
- }
}
static unsigned char tolower_table[256];
@@ -4113,7 +4134,43 @@
return sipp_do_connect_socket(socket);
}
+#ifdef _UNITTEST
+int main()
+{
+ /* Unit testing function */
+ char ipv6_addr_brackets[] = "[fe80::92a4:deff:fe74:7af5]";
+ char ipv6_addr_port[] = "[fe80::92a4:deff:fe74:7af5]:999";
+ char ipv6_addr[] = "fe80::92a4:deff:fe74:7af5";
+ char ipv4_addr_port[] = "127.0.0.1:999";
+ char ipv4_addr[] = "127.0.0.1";
+ char hostname_port[] = "sipp.sf.net:999";
+ char hostname[] = "sipp.sf.net";
+ int port_result = -1;
+ char host_result[255];
+ char orig_addr[255];
+
+#define TEST_GET_HOST_AND_PORT(VAR, EXPECTED_HOST, EXPECTED_PORT) {\
+ strcpy(host_result,""); \
+ strcpy(orig_addr,VAR); \
+ get_host_and_port(VAR, host_result, &port_result); \
+ if ((strcmp(host_result, EXPECTED_HOST) != 0) || (port_result != EXPECTED_PORT)) \
+ {fprintf(stderr, "get_host_and_port fails for address %s - results are %s and %d, expected %s and %d\n", orig_addr, host_result, port_result, EXPECTED_HOST, EXPECTED_PORT);};\
+}
+
+ TEST_GET_HOST_AND_PORT(ipv6_addr, "fe80::92a4:deff:fe74:7af5", 0)
+ TEST_GET_HOST_AND_PORT(ipv6_addr_brackets, "fe80::92a4:deff:fe74:7af5", 0)
+ TEST_GET_HOST_AND_PORT(ipv6_addr_port, "fe80::92a4:deff:fe74:7af5", 999)
+ TEST_GET_HOST_AND_PORT(ipv4_addr, "127.0.0.1", 0)
+ TEST_GET_HOST_AND_PORT(ipv4_addr_port, "127.0.0.1", 999)
+ TEST_GET_HOST_AND_PORT(hostname, "sipp.sf.net", 0)
+ TEST_GET_HOST_AND_PORT(hostname_port, "sipp.sf.net", 999)
+
+ return 0;
+}
+
+#else
+
/* Main */
int main(int argc, char *argv[])
{
@@ -5058,6 +5115,8 @@
}
+#endif
+
void sipp_usleep(unsigned long usec) {
if (usec >= 1000000) {
sleep(usec / 1000000);
Index: Makefile
===================================================================
--- Makefile (revision 600)
+++ Makefile (working copy)
@@ -105,7 +105,7 @@
CFLAGS_SunOS=${DEBUG_FLAGS} -D__SUNOS
CFLAGS_Cygwin=-D__CYGWIN -Dsocklen_t=int
CFLAGS_Darwin=-D__DARWIN
-CFLAGS=$(CFLAGS_$(SYSTEM)) $(VERINFO) $(TLS) $(PCAPPLAY) $(EXTRACFLAGS)
+CFLAGS=$(CFLAGS_$(SYSTEM)) $(VERINFO) $(TLS) $(PCAPPLAY) $(EXTRACFLAGS) $(UNITTEST)
#C++ Compiler Flags
CPPFLAGS_hpux=-AA -mt -D__HPUX -D_INCLUDE_LONGLONG -DNOMACROS +W829
@@ -115,7 +115,7 @@
CPPFLAGS_SunOS=${DEBUG_FLAGS} -D__SUNOS
CPPFLAGS_Cygwin=-D__CYGWIN -Dsocklen_t=int
CPPFLAGS_Darwin=-D__DARWIN
-CPPFLAGS=$(CPPFLAGS_$(SYSTEM)) $(VERINFO) $(TLS) $(PCAPPLAY) $(EXTRACPPFLAGS)
+CPPFLAGS=$(CPPFLAGS_$(SYSTEM)) $(VERINFO) $(TLS) $(PCAPPLAY) $(EXTRACPPFLAGS) $(UNITTEST)
#Linker mapping
CCLINK_hpux=aCC
@@ -163,6 +163,9 @@
all:
$(MAKE) OSNAME=`uname|sed -e "s/CYGWIN.*/CYGWIN/"` MODELNAME=`uname -m|sed "s/Power Macintosh/ppc/"` $(OUTPUT)
+unittest:
+ $(MAKE) UNITTEST="-D_UNITTEST" OSNAME=`uname|sed -e "s/CYGWIN.*/CYGWIN/"` MODELNAME=`uname -m|sed "s/Power Macintosh/ppc/"` $(OUTPUT)
+
# Building with TLS and authentication
ossl:
$(MAKE) OSNAME=`uname|sed -e "s/CYGWIN.*/CYGWIN/"` MODELNAME=`uname -m|sed "s/Power Macintosh/ppc/"` OBJ_TLS="auth.o sslinit.o sslthreadsafe.o milenage.o rijndael.o" TLS_LIBS="-lssl -lcrypto" TLS="-D_USE_OPENSSL -DOPENSSL_NO_KRB5" $(OUTPUT)
------------------------------------------------------------------------------
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
_______________________________________________
Sipp-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/sipp-users
