From 1f5ff11a9ae34d972d41214e339baeae1387bc2e Mon Sep 17 00:00:00 2001 From: Eric House Date: Wed, 25 Mar 2020 22:27:01 -0700 Subject: [PATCH 01/25] recover from corrupt chat player index This should never happen, but it did and I want to stop local crashes to investigate. --- .../src/main/java/org/eehouse/android/xw4/ChatDelegate.java | 5 +++-- xwords4/common/server.c | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ChatDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ChatDelegate.java index 4d62cef2a..4597b4b32 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ChatDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ChatDelegate.java @@ -162,8 +162,9 @@ public class ChatDelegate extends DelegateBase { TextView view = (TextView)row.findViewById( R.id.chat_row_text ); view.setText( msg ); view = (TextView)row.findViewById( R.id.chat_row_name ); - view.setText( getString( R.string.chat_sender_fmt, - m_names[playerIndx] ) ); + + String name = playerIndx < m_names.length ? m_names[playerIndx] : ""; + view.setText( getString( R.string.chat_sender_fmt, name ) ); if ( tsSeconds > 0 ) { long now = 1000L * Utils.getCurSeconds(); diff --git a/xwords4/common/server.c b/xwords4/common/server.c index 771d92987..6a7a58180 100644 --- a/xwords4/common/server.c +++ b/xwords4/common/server.c @@ -722,6 +722,7 @@ sendChatTo( ServerCtxt* server, XP_U16 devIndex, const XP_UCHAR* msg, XWStreamCtxt* stream = messageStreamWithHeader( server, devIndex, XWPROTO_CHAT ); stringToStream( stream, msg ); + XP_ASSERT( from < server->vol.gi->nPlayers ); stream_putU8( stream, from ); stream_putU32( stream, timestamp ); stream_destroy( stream ); @@ -765,6 +766,7 @@ receiveChat( ServerCtxt* server, XWStreamCtxt* incoming ) sendChatToClientsExcept( server, sourceClientIndex, msg, from, timestamp ); } + XP_ASSERT( from < server->vol.gi->nPlayers ); util_showChat( server->vol.util, msg, from, timestamp ); XP_FREE( server->mpool, msg ); return XP_TRUE; From 512a8e1afaa2c9bb6d80dcf3123391b0904919b1 Mon Sep 17 00:00:00 2001 From: Eric House Date: Thu, 26 Mar 2020 18:12:37 -0700 Subject: [PATCH 02/25] fix crash when chat message is too long Len byte was limited to 255, but would get clipped (masked with 0xFF) then all the string data would get written. So on receipt, the clipped length was taken to be that of the string data, with the rest of the string to be interpreted as something else. An array index, in this case. --- .../src/main/java/org/eehouse/android/xw4/DBUtils.java | 2 +- xwords4/common/server.c | 2 -- xwords4/common/strutils.c | 8 ++++++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DBUtils.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DBUtils.java index 25f53e470..4d6c1cb56 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DBUtils.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DBUtils.java @@ -1868,7 +1868,7 @@ public class DBUtils { { Assert.assertNotNull( msg ); Assert.assertFalse( -1 == fromPlayer ); - ArrayList valuess = new ArrayList(); + ArrayList valuess = new ArrayList<>(); valuess.add( cvForChat( rowid, msg, fromPlayer, tsSeconds ) ); appendChatHistory( context, valuess ); Log.i( TAG, "appendChatHistory: inserted \"%s\" from player %d", diff --git a/xwords4/common/server.c b/xwords4/common/server.c index 6a7a58180..771d92987 100644 --- a/xwords4/common/server.c +++ b/xwords4/common/server.c @@ -722,7 +722,6 @@ sendChatTo( ServerCtxt* server, XP_U16 devIndex, const XP_UCHAR* msg, XWStreamCtxt* stream = messageStreamWithHeader( server, devIndex, XWPROTO_CHAT ); stringToStream( stream, msg ); - XP_ASSERT( from < server->vol.gi->nPlayers ); stream_putU8( stream, from ); stream_putU32( stream, timestamp ); stream_destroy( stream ); @@ -766,7 +765,6 @@ receiveChat( ServerCtxt* server, XWStreamCtxt* incoming ) sendChatToClientsExcept( server, sourceClientIndex, msg, from, timestamp ); } - XP_ASSERT( from < server->vol.gi->nPlayers ); util_showChat( server->vol.util, msg, from, timestamp ); XP_FREE( server->mpool, msg ); return XP_TRUE; diff --git a/xwords4/common/strutils.c b/xwords4/common/strutils.c index ba7b5c986..82054d8fc 100644 --- a/xwords4/common/strutils.c +++ b/xwords4/common/strutils.c @@ -267,8 +267,12 @@ stringFromStreamHere( XWStreamCtxt* stream, XP_UCHAR* buf, XP_U16 buflen ) void stringToStream( XWStreamCtxt* stream, const XP_UCHAR* str ) { - XP_U16 len = str==NULL? 0: XP_STRLEN( str ); - XP_ASSERT( len < 0xFF ); + XP_U16 len = str == NULL? 0: XP_STRLEN( str ); + if ( len > 0xFF ) { + XP_LOGFF( "truncating string '%s', dropping len from %d to %d", + str, len, 0xFF ); + len = 0xFF; + } stream_putU8( stream, (XP_U8)len ); stream_putBytes( stream, str, len ); } /* putStringToStream */ From 5e5da59cbc78eceba0068d463a1c5c657e5942fe Mon Sep 17 00:00:00 2001 From: Eric House Date: Thu, 26 Mar 2020 18:24:46 -0700 Subject: [PATCH 03/25] hack: limit chat field length to 255 The common code truncates to 255 *bytes*, not chars, so this isn't even right, but at least for ascii users it'll make it less likely that what's typed doesn't all get sent. The right fix is to change the format to not limit sent str len to 255 bytes. See commit 512a8e1af. --- xwords4/android/app/src/main/res/layout/chat.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/xwords4/android/app/src/main/res/layout/chat.xml b/xwords4/android/app/src/main/res/layout/chat.xml index b0ac5d6eb..0a9b0a5d1 100644 --- a/xwords4/android/app/src/main/res/layout/chat.xml +++ b/xwords4/android/app/src/main/res/layout/chat.xml @@ -34,6 +34,7 @@ android:layout_weight="1" android:scrollHorizontally="false" android:hint="@string/chat_hint" + android:maxLength="255" />