Fix relay devid protocol to deal with case where client submits a

ID_TYPE_RELAY id that's not in the devices table (as has happened when
a device switches relay URLs during testing, but might also happen if
I have to delete an entry from the devices table.)  In that case,
return ID_TYPE_NONE to the client, which will be its clue to delete
its ID_TYPE_RELAY id and submit the platform-specific id again.

Note: android won't compile this revision thanks to util.h change

xwords4/common/comms.c

@@ -1331,10 +1331,14 @@ got_connect_cmd( CommsCtxt* comms, XWStreamCtxt* stream,
 #endif
 
 #ifdef XWFEATURE_DEVID
-    XP_UCHAR devID[MAX_DEVID_LEN + 1];
-    stringFromStreamHere( stream, devID, sizeof(devID) );
-    if ( devID[0] != '\0' ) {
-        util_deviceRegistered( comms->util, devID );
+    DevIDType typ = stream_getU8( stream );
+    XP_UCHAR devID[MAX_DEVID_LEN + 1] = {0};
+    if ( ID_TYPE_NONE != typ ) {
+        stringFromStreamHere( stream, devID, sizeof(devID) );
+    }
+    if ( ID_TYPE_NONE == typ    /* error case */
+         || '\0' != devID[0] ) /* new info case */ {
+        util_deviceRegistered( comms->util, typ, devID );
     }
 #endif
 

xwords4/common/util.h

@@ -154,7 +154,8 @@ typedef struct UtilVtable {
     XP_U32 (*m_util_getCurSeconds)( XW_UtilCtxt* uc );
 #ifdef XWFEATURE_DEVID
     const XP_UCHAR* (*m_util_getDevID)( XW_UtilCtxt* uc, DevIDType* typ );
-    void (*m_util_deviceRegistered)( XW_UtilCtxt* uc, const XP_UCHAR* idRelay );
+    void (*m_util_deviceRegistered)( XW_UtilCtxt* uc, DevIDType typ, 
+                                     const XP_UCHAR* idRelay );
 #endif
     DictionaryCtxt* (*m_util_makeEmptyDict)( XW_UtilCtxt* uc );
 
@@ -284,10 +285,10 @@ struct XW_UtilCtxt {
          (uc)->vtable->m_util_getCurSeconds((uc))
 
 #ifdef XWFEATURE_DEVID
-# define util_getDevID( uc, t )                      \
+# define util_getDevID( uc, t )                     \
          (uc)->vtable->m_util_getDevID((uc),(t))
-# define util_deviceRegistered( uc, id )             \
-         (uc)->vtable->m_util_deviceRegistered( (uc), (id) )
+# define util_deviceRegistered( uc, typ, id )                       \
+         (uc)->vtable->m_util_deviceRegistered( (uc), (typ), (id) )
 #endif
 
 #define util_makeEmptyDict( uc ) \

xwords4/linux/linuxmain.c

@@ -1593,9 +1593,6 @@ main( int argc, char** argv )
     mainParams.allowPeek = XP_TRUE;
     mainParams.showRobotScores = XP_FALSE;
     mainParams.useMmap = XP_TRUE;
-#ifdef XWFEATURE_DEVID
-    mainParams.devID = "";
-#endif
 
     char* envDictPath = getenv( "XW_DICTSPATH" );
     if ( !!envDictPath ) {

xwords4/linux/linuxutl.c

@@ -353,19 +353,24 @@ linux_util_getDevID( XW_UtilCtxt* uc, DevIDType* typ )
     if ( !!cGlobals->params->rDevID ) {
         *typ = ID_TYPE_RELAY;
         result = cGlobals->params->rDevID;
-    } else {
+    } else if ( !!cGlobals->params->devID ) {
         *typ = ID_TYPE_LINUX;
         result = cGlobals->params->devID;
+    } else {
+        *typ = ID_TYPE_NONE;
+        result = NULL;
     }
     return result;
 }
 
 static void
 linux_util_deviceRegistered( XW_UtilCtxt* XP_UNUSED(uc), 
+                             DevIDType XP_UNUSED_DBG(typ), 
                              const XP_UCHAR* idRelay )
 {
     /* Script discon_ok2.sh is grepping for this in logs, so don't change
        it! */
+    XP_ASSERT( ID_TYPE_RELAY == typ ); /* all the linux client deals with */
     XP_LOGF( "%s: new id: %s", __func__, idRelay );
 }
 #endif

xwords4/relay/cref.cpp

@@ -891,13 +891,7 @@ CookieRef::increasePlayerCounts( CRefEvent* evt, bool reconn, HostID* hidp,
         DevIDType devIDType = evt->u.con.devID->m_devIDType;
         // does client support devID
         if ( ID_TYPE_NONE != devIDType ) { 
-            // have we not already converted it?
-            if ( ID_TYPE_RELAY == devIDType ) {
-                devID = (DBMgr::DevIDRelay)strtoul( evt->u.con.devID->m_devIDString.c_str(), 
-                                                    NULL, 16 );
-            } else {
-                devID = DBMgr::Get()->RegisterDevice( evt->u.con.devID );
-            }
+            devID = DBMgr::Get()->RegisterDevice( evt->u.con.devID );
         }
         *devIDp = devID;
     }
@@ -1071,22 +1065,38 @@ CookieRef::sendResponse( const CRefEvent* evt, bool initial,
     bufp += len;
 
     // we always write at least empty string
-    char idbuf[MAX_DEVID_LEN + 1] = {0};
 
     // If client supports devid, and we have one (response case), write it as
     // 8-byte hex string plus a length byte -- but only if we didn't already
     // receive it.
-    if ( !!devID && ID_TYPE_RELAY < evt->u.con.devID->m_devIDType ) { 
-        len = snprintf( idbuf, sizeof(idbuf), "%.8X", *devID );
-        assert( len < sizeof(idbuf) );
-    }
 
-    len = strlen( idbuf );
-    assert( len <= MAX_DEVID_LEN );
-    *bufp++ = (char)len;
-    if ( 0 < len ) {
-        memcpy( bufp, idbuf, len );
-        bufp += len;
+    // there are three possibilities: it sent us a platform-specific ID and we
+    // need to return the relay version; or it sent us a valid relay version;
+    // or it sent us an invalid one (for whatever reason, e.g. we've wiped the
+    // devices table entry for a problematic GCM id to force reregistration.)
+    // In the first case, we return the new relay version.  In the second, we
+    // return that the type is ID_TYPE_RELAY but don't bother with the version
+    // string; and in the third, we return ID_TYPE_NONE.
+
+    if ( DBMgr::DEVID_NONE == *devID ) { // first case
+        *bufp++ = ID_TYPE_NONE;
+    } else {
+        *bufp++ = ID_TYPE_RELAY;
+
+        // Write an empty string if the client passed the ID to us, or the id
+        // if it's new to the client.
+        char idbuf[MAX_DEVID_LEN + 1];
+        if ( !!ID_TYPE_RELAY < evt->u.con.devID->m_devIDType ) { 
+            len = snprintf( idbuf, sizeof(idbuf), "%.8X", *devID );
+            assert( len < sizeof(idbuf) );
+        } else {
+            len = 0;
+        }
+        *bufp++ = (char)len;
+        if ( 0 < len ) {
+            memcpy( bufp, idbuf, len );
+            bufp += len;
+        }
     }
 
     send_with_length( socket, buf, bufp - buf, true );

xwords4/relay/dbmgr.cpp

@@ -253,13 +253,16 @@ DBMgr::DevIDRelay
 DBMgr::RegisterDevice( const DevID* host )
 {
     DBMgr::DevIDRelay devID;
-    assert( host->m_devIDType != ID_TYPE_RELAY );
+    assert( host->m_devIDType != ID_TYPE_NONE );
     int ii;
     bool success;
 
     // if it's already present, just return
     devID = getDevID( host );
-    if ( DEVID_NONE == devID ) {
+
+    // If it's not present *and* of type ID_TYPE_RELAY, we can do nothing.
+    // Fail.
+    if ( DEVID_NONE == devID && ID_TYPE_RELAY < host->m_devIDType ) {
         // loop until we're successful inserting the unique key.  Ship with this
         // coming from random, but test with increasing values initially to make
         // sure duplicates are detected.
@@ -641,16 +644,23 @@ DBMgr::getDevID( const DevID* devID )
 {
     DBMgr::DevIDRelay rDevID = DEVID_NONE;
     DevIDType devIDType = devID->m_devIDType;
+    char query[512] = {0};
     assert( ID_TYPE_NONE < devIDType );
     const char* asStr = devID->m_devIDString.c_str();
     if ( ID_TYPE_RELAY == devIDType ) {
-        rDevID = strtoul( asStr, NULL, 16 );
+        // confirm it's there
+        DBMgr::DevIDRelay cur = strtoul( asStr, NULL, 16 );
+        if ( DEVID_NONE != cur ) {
+            const char* fmt = "SELECT id FROM " DEVICES_TABLE " WHERE id=%d";
+            snprintf( query, sizeof(query), fmt, cur );
+        }
     } else {
         const char* fmt = "SELECT id FROM " DEVICES_TABLE " WHERE devtype=%d and devid = '%s'";
-        char query[512];
         snprintf( query, sizeof(query), fmt, devIDType, asStr );
-        logf( XW_LOGINFO, "%s: query: %s", __func__, query );
+    }
 
+    if ( '\0' != query[0] ) {
+        logf( XW_LOGINFO, "%s: query: %s", __func__, query );
         PGresult* result = PQexec( getThreadConn(), query );
         assert( 1 >= PQntuples( result ) );
         if ( 1 == PQntuples( result ) ) {