Fix SI address and initialistion bugs

The SI95 address translation between an address struct and
human readable string had a function call with reversed
parameter constants.

The SI initialisation could excite an NPE if the v6 "listen
all" string (::) was given instead of 0.0.0.0.

Issue-ID: RIC-327

Signed-off-by: E. Scott Daniels <daniels@research.att.com>
Change-Id: Ifa673aca7ceaa7e47fbd9158340f0f825ac0fae8

diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt
index a8b20b7..c3244be 100644 (file)
--- a/CHANGES_CORE.txt
+++ b/CHANGES_CORE.txt
@@ -5,6 +5,10 @@
 # API and build change  and fix summaries. Doc correctsions
 # and/or changes are not mentioned here; see the commit messages.
 
 # API and build change  and fix summaries. Doc correctsions
 # and/or changes are not mentioned here; see the commit messages.
 

+2020 April 6; version 3.6.6
+       Correct bug in SI95 address conversion module (RIC-327)
+       Correct bug in SI initialisation module
+

 2020 April 2; version 3.6.5
        Correct potential nil pointer use when examining interfaces for
        use as a listen target (RIC-307)
 2020 April 2; version 3.6.5
        Correct potential nil pointer use when examining interfaces for
        use as a listen target (RIC-307)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e2392c1..b54c676 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -40,7 +40,7 @@ cmake_minimum_required( VERSION 3.5 )
 
 set( major_version "3" )               # should be automatically populated from git tag later, but until CI process sets a tag we use this
 set( minor_version "6" )
 
 set( major_version "3" )               # should be automatically populated from git tag later, but until CI process sets a tag we use this
 set( minor_version "6" )

-set( patch_level "5" )
+set( patch_level "6" )

 
 set( install_root "${CMAKE_INSTALL_PREFIX}" )
 set( install_inc "include/rmr" )
 
 set( install_root "${CMAKE_INSTALL_PREFIX}" )
 set( install_inc "include/rmr" )

diff --git a/src/rmr/si/src/rmr_si.c b/src/rmr/si/src/rmr_si.c
index 8606241..17014a6 100644 (file)
--- a/src/rmr/si/src/rmr_si.c
+++ b/src/rmr/si/src/rmr_si.c
@@ -674,7 +674,7 @@ static void* init(  char* uproto_port, int max_msg_size, int flags ) {
                ctx->my_ip = get_default_ip( ctx->ip_list );    // and (guess) at what should be the default to put into messages as src
                if( ctx->my_ip == NULL ) {
                        rmr_vlog( RMR_VL_WARN, "rmr_init: default ip address could not be sussed out, using name\n" );
                ctx->my_ip = get_default_ip( ctx->ip_list );    // and (guess) at what should be the default to put into messages as src
                if( ctx->my_ip == NULL ) {
                        rmr_vlog( RMR_VL_WARN, "rmr_init: default ip address could not be sussed out, using name\n" );

-                       strcpy( ctx->my_ip, ctx->my_name );                     // if we cannot suss it out, use the name rather than a nil pointer
+                       ctx->my_ip = strdup( ctx->my_name );            // if we cannot suss it out, use the name rather than a nil pointer

                }
        }
        if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, " default ip address: %s\n", ctx->my_ip );
                }
        }
        if( DEBUG ) rmr_vlog( RMR_VL_DEBUG, " default ip address: %s\n", ctx->my_ip );

diff --git a/src/rmr/si/src/si95/siaddress.c b/src/rmr/si/src/si95/siaddress.c
index 8761b66..e77ab4f 100644 (file)
--- a/src/rmr/si/src/si95/siaddress.c
+++ b/src/rmr/si/src/si95/siaddress.c
@@ -86,7 +86,7 @@ extern int SIgenaddr( char *target, int proto, int family, int socktype, struct
                if( *dstr == '[' ) {                            // strip [ and ] from v6 and point pstring if port there
                        dstr++;
                        pstr = strchr( dstr, ']' );
                if( *dstr == '[' ) {                            // strip [ and ] from v6 and point pstring if port there
                        dstr++;
                        pstr = strchr( dstr, ']' );

-                       if( *pstr != ']' ) {
+                       if( !pstr || *pstr != ']' ) {

                                free( fptr );
                                return -1;
                        }
                                free( fptr );
                                return -1;
                        }


@@ -109,15 +109,16 @@ extern int SIgenaddr( char *target, int proto, int family, int socktype, struct
        memset( &hint, 0, sizeof( hint  ) );
        hint.ai_family = family;                        //  AF_INET AF_INET6...  let this be 0 to select best based on addr 
        hint.ai_socktype = socktype;            //  SOCK_DGRAM SOCK_STREAM 
        memset( &hint, 0, sizeof( hint  ) );
        hint.ai_family = family;                        //  AF_INET AF_INET6...  let this be 0 to select best based on addr 
        hint.ai_socktype = socktype;            //  SOCK_DGRAM SOCK_STREAM 

-       hint.ai_protocol = proto;                       //  IPPORTO_TCP IPPROTO_UDP 
+       hint.ai_protocol = proto;                       //  IPPROTO_TCP IPPROTO_UDP 

        hint.ai_flags = ga_flags;
 
        if( DEBUG ) 
        hint.ai_flags = ga_flags;
 
        if( DEBUG ) 

-               rmr_vlog( RMR_VL_DEBUG, "siaddress: calling getaddrinfo flags=%x proto=%d family=%d target=%s host=%s port=%s\n", 
-                               ga_flags, proto, family, target, dstr, pstr );
+               rmr_vlog( RMR_VL_DEBUG, "siaddress: calling getaddrinfo flags=%x sockty=%d proto=%d family=%d target=%s host=%s port=%s\n",
+                               ga_flags, socktype, proto, family, target, dstr, pstr );

 
        if( (error = getaddrinfo( dstr, pstr, &hint, &list )) ) {
 
        if( (error = getaddrinfo( dstr, pstr, &hint, &list )) ) {

-               fprintf( stderr, "error from getaddrinfo: target=%s host=%s port=%s(port): error=(%d) %s\n", target, dstr, pstr, error, gai_strerror( error ) );
+               fprintf( stderr, "sigenaddr: error from getaddrinfo: target=%s host=%s port=%s(port): error=(%d) %s\n", 
+                       target, dstr, pstr, error, gai_strerror( error ) );

        } else {
                *rap = (struct sockaddr *) malloc(  list->ai_addrlen );         //  alloc a buffer and give address to caller 
                memcpy( *rap, list->ai_addr, list->ai_addrlen  );
        } else {
                *rap = (struct sockaddr *) malloc(  list->ai_addrlen );         //  alloc a buffer and give address to caller 
                memcpy( *rap, list->ai_addr, list->ai_addrlen  );


@@ -167,11 +168,12 @@ extern int SIaddress( void *src, void **dest, int type ) {
                        rlen = strlen( *dest );
                        break;
 
                        rlen = strlen( *dest );
                        break;
 

-               case AC_TOADDR6:                        //  from hostname;port string to address for send etc 
-                       return SIgenaddr( src, PF_INET6, IPPROTO_TCP, SOCK_STREAM, (struct sockaddr **) dest );
+               case AC_TOADDR6:                        //  from hostname:port string to address for send etc 
+                       return SIgenaddr( src, IPPROTO_TCP, AF_INET6, SOCK_STREAM, (struct sockaddr **) dest );

 
                case AC_TOADDR:                         //  from dotted decimal to address struct ip4 
 
                case AC_TOADDR:                         //  from dotted decimal to address struct ip4 

-                       return SIgenaddr( src, PF_INET, IPPROTO_TCP, SOCK_STREAM, (struct sockaddr **) dest );
+                       //return SIgenaddr( src, AF_INET, IPPROTO_TCP, SOCK_STREAM, (struct sockaddr **) dest );
+                       return SIgenaddr( src, IPPROTO_TCP, AF_INET, SOCK_STREAM, (struct sockaddr **) dest );

        }
 
        return rlen;
        }
 
        return rlen;

diff --git a/test/si95_test.c b/test/si95_test.c
index d5bcc3a..26b1b21 100644 (file)
--- a/test/si95_test.c
+++ b/test/si95_test.c
@@ -36,6 +36,12 @@
 #include <pthread.h>
 #include <ctype.h>
 
 #include <pthread.h>
 #include <ctype.h>
 

+#include <netdb.h>             // these four needed for si address tests
+#include <stdio.h>
+#include <ctype.h>
+#include <netinet/in.h>
+
+

 
 #include <unistd.h>
 #include <stdio.h>
 
 #include <unistd.h>
 #include <stdio.h>


@@ -67,14 +73,14 @@
 #include <rmr_logging.h>
 #include <logging.c>
 
 #include <rmr_logging.h>
 #include <logging.c>
 

-//#include <si95/siaddress.c>
+#include <si95/siaddress.c>

 //#include <si95/sialloc.c>
 //#include <si95/sibldpoll.c>
 //#include <si95/sicbreg.c>
 //#include <si95/sicbstat.c>
 //#include <si95/siclose.c>
 //#include <si95/sialloc.c>
 //#include <si95/sibldpoll.c>
 //#include <si95/sicbreg.c>
 //#include <si95/sicbstat.c>
 //#include <si95/siclose.c>

-//#include <si95/siconnect.c>
-//#include <si95/siestablish.c>
+#include <si95/siconnect.c>
+#include <si95/siestablish.c>

 //#include <si95/sigetadd.c>
 //#include <si95/sigetname.c>
 #include <si95/siinit.c>
 //#include <si95/sigetadd.c>
 //#include <si95/sigetname.c>
 #include <si95/siinit.c>


@@ -90,6 +96,11 @@
 #include <si95/sitrash.c>
 //#include <si95/siwait.c>
 
 #include <si95/sitrash.c>
 //#include <si95/siwait.c>
 

+// ---------------------------------------------------------------------
+
+void*  si_ctx = NULL;                  // a global context might be useful
+
+// ---------------------------------------------------------------------

 
 /*
        Memory allocation/free related tests
 
 /*
        Memory allocation/free related tests


@@ -124,7 +135,6 @@ static int memory( ) {
        return errors;
 }
 
        return errors;
 }
 

-void*  si_ctx = NULL;                  // a global context might be useful

 
 /*
        Test initialisation related things
 
 /*
        Test initialisation related things


@@ -141,6 +151,58 @@ static int init() {
        return errors;
 }
 
        return errors;
 }
 

+static int cleanup() {
+       int errors = 0;
+       
+       if( ! si_ctx ) {
+               return 0;
+       }
+
+       SIconnect( si_ctx, "localhost:43086" );         // ensure context has a tp block to free on shutdown
+       SIshutdown( si_ctx );
+
+       return errors;
+}
+
+/*
+       Address related tests.
+*/
+static int addr() {
+       int errors = 0;
+       int l;
+       struct sockaddr* addr;
+       char buf1[4096];
+       char buf2[4096];
+       char* dest;
+
+       addr = (struct sockaddr *) malloc( sizeof( struct sockaddr ) );
+/*
+       l = SIgenaddr( "    [ff02::4]:4567", PF_INET6, IPPROTO_TCP, SOCK_STREAM, &addr );
+
+       SIgenaddr( "    [ff02::4]:4567", PF_INET6, IPPROTO_TCP, SOCK_STREAM, &addr );
+*/
+
+       dest = NULL;
+       snprintf( buf1, sizeof( buf1 ), "   [ff02::5:4001" );           // invalid address, drive leading space eater too
+       l = SIaddress( buf1, &dest, AC_TOADDR6 );
+       errors += fail_if_true( l > 0, "to addr6 with bad addr convdersion returned valid len" );
+
+       snprintf( buf1, sizeof( buf1 ), "[ff02::5]:4002" );             // v6 might not be supported so failure is OK here
+       l=SIaddress( buf1, &dest, AC_TOADDR6 );
+       errors += fail_if_true( l < 1, "to addr convdersion failed" );
+
+       snprintf( buf1, sizeof( buf1 ), "localhost:43086" );
+       l = SIaddress( buf1, (void **) &dest, AC_TOADDR );
+       errors += fail_if_true( l < 1, "to addr convdersion failed" );
+
+       snprintf( buf1, sizeof( buf1 ), "localhost:4004" );
+       l = SIaddress( buf1, &dest, AC_TODOT );
+       errors += fail_if_true( l < 1, "to dot convdersion failed" );
+
+       return errors;
+
+}
+

 /*
        Drive tests...
 */
 /*
        Drive tests...
 */


@@ -153,7 +215,11 @@ int main() {
 
        errors += init();
        errors += memory();
 
        errors += init();
        errors += memory();

+       errors += addr();
+fprintf( stderr, ">>> cleaning\n" );
+       errors += cleanup();

 
 

+       fprintf( stderr, "<INFO> testing finished\n" );

        if( errors == 0 ) {
                fprintf( stderr, "<PASS> all tests were OK\n\n" );
        } else {
        if( errors == 0 ) {
                fprintf( stderr, "<PASS> all tests were OK\n\n" );
        } else {

diff --git a/test/wormhole_static_test.c b/test/wormhole_static_test.c
index 25d2cca..9d5fd58 100644 (file)
--- a/test/wormhole_static_test.c
+++ b/test/wormhole_static_test.c
@@ -54,6 +54,7 @@ static int worm_test( ) {
        void* p;
 
        rmr_mbuf_t*     mbuf;           // mbuf to send to peer
        void* p;
 
        rmr_mbuf_t*     mbuf;           // mbuf to send to peer

+       rmr_mbuf_t*     resp;           // response message from a wormhole call

        int             whid = -1;
        int             last_whid;
 
        int             whid = -1;
        int             last_whid;
 


@@ -63,6 +64,20 @@ static int worm_test( ) {
 
        gen_rt( ctx );
 
 
        gen_rt( ctx );
 

+       // ---- must run these checks before wormhole(s) are opened --------------------------
+       mbuf = rmr_alloc_msg( ctx, 2048 );                      // get an muf to pass round, no need to init for first test
+       resp = rmr_wh_call( ctx, 0, mbuf, 1, 10 );
+       errors += fail_if_nil( resp, "wormhole call given valid context and msg, before wormholes open  didn't return message pointer" );
+       if( resp ) {
+       errors += fail_if_equal( resp->state, RMR_OK, "wormhole call given valid context and msg, before wormholes open returned ok state" );
+       }
+
+       rmr_wh_close( NULL, 0 );                        // drive for coverage; nothing to vet
+       rmr_wh_close( ctx, 0 );
+       wh_init( NULL );
+
+       // ----- end must run before opening wormholes ---------------------------------------
+

        whid = rmr_wh_open( NULL, NULL );
        errors += fail_not_equal( whid, -1, "call to wh_open with invalid values did not return bad whid" );
 
        whid = rmr_wh_open( NULL, NULL );
        errors += fail_not_equal( whid, -1, "call to wh_open with invalid values did not return bad whid" );
 


@@ -76,6 +91,9 @@ static int worm_test( ) {
        whid = rmr_wh_open( ctx, "localhost:89219" );
        errors += fail_if_equal( whid, -1, "call to wh_open with valid target failed" );
 
        whid = rmr_wh_open( ctx, "localhost:89219" );
        errors += fail_if_equal( whid, -1, "call to wh_open with valid target failed" );
 

+       i = wh_init( ctx );                                     // already initialised, ensure no harm
+       errors += fail_not_equal( i, 1, "second call to wh_init() didn't return true" );
+

        rmr_wh_close( ctx, 4 );                                 // test for coverage only; [5] should have nil pointer
        rmr_wh_close( ctx, 50 );                                        // test for coverage only; more than allocated reference
 
        rmr_wh_close( ctx, 4 );                                 // test for coverage only; [5] should have nil pointer
        rmr_wh_close( ctx, 50 );                                        // test for coverage only; more than allocated reference
 


@@ -153,6 +171,28 @@ static int worm_test( ) {
                whid = -1;
        }
 
                whid = -1;
        }
 

+       // ----- wormhole call --------------------------------------------------
+       mbuf = rmr_alloc_msg( ctx, 2048 );                              // ensure we have a buffer to pass
+       resp = rmr_wh_call( NULL, 0, NULL, 1, 10 );     // ensure no msg when msg is nil
+       errors += fail_not_nil( resp, "wormhole call given nil context and msg didn't return nil message" );
+
+       resp = rmr_wh_call( NULL, 0, mbuf, 1, 10 );      // ensure invalid state when context is bad
+       errors += fail_if_nil( resp, "wormhole call given nil context and valid msg didn't return message pointer" );
+       if( resp != NULL ) {
+               errors += fail_if_equal( resp->state, RMR_OK, "wormhole call given nil context and valid message returned OK state" );
+       }
+
+       resp = rmr_wh_call( ctx, 2000, mbuf, 1, 10 );        // invalid wormhole id
+       errors += fail_if_nil( resp, "wormhole call given bad whid returned no msg" );
+       if( resp ) {
+               errors += fail_if_equal( resp->state, RMR_OK, "wormhole call given bad whid returned good state" );
+       }   
+
+       whid = rmr_wh_open( ctx, "localhost:9219" );    // ensure one is open
+       resp = rmr_wh_call( ctx, whid, mbuf, 1, 1 );
+       errors += fail_if_nil( resp, "wormhole call given valid info returned nil response" );
+
+

        // -----------------------------------------------------------------------
        // WARNING:  these tests destroy the context, so they MUST be last
        if( mbuf ) {                    // only if we got an mbuf
        // -----------------------------------------------------------------------
        // WARNING:  these tests destroy the context, so they MUST be last
        if( mbuf ) {                    // only if we got an mbuf


@@ -163,6 +203,7 @@ static int worm_test( ) {
                errors += fail_not_equal( mbuf->state, RMR_ERR_NOHDR, "send with bad header did now set msg state correctly" );
 
                errno = 0;
                errors += fail_not_equal( mbuf->state, RMR_ERR_NOHDR, "send with bad header did now set msg state correctly" );
 
                errno = 0;

+               wh_nuke( NULL );                                // coverage only

                wh_nuke( ctx );
                ctx->wormholes = NULL;
                mbuf = rmr_wh_send_msg( ctx, 4, mbuf );         // coverage test on mbuf header check
                wh_nuke( ctx );
                ctx->wormholes = NULL;
                mbuf = rmr_wh_send_msg( ctx, 4, mbuf );         // coverage test on mbuf header check