Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.41k stars 377 forks source link

Special character/emoji string message crashes SqlLog logger #1463

Open jujaga opened 7 years ago

jujaga commented 7 years ago

TShock Version 4.3.24, APIv2.1, OTAPI v2.0.0.27

Stack Trace

2017-06-27 11:27:36 - SqlLog: ERROR: SQL Log insert query failed: MySql.Data.MySqlClient.MySqlException (0x80004005): Incorrect string value: '\xF0\x9F\x91\x8F.' for column 'Message' at row 1
   at MySql.Data.MySqlClient.MySqlStream.ReadPacket()
   at MySql.Data.MySqlClient.NativeDriver.GetResult(Int32& affectedRow, Int64& insertedId)
   at MySql.Data.MySqlClient.Driver.GetResult(Int32 statementId, Int32& affectedRows, Int64& insertedId)
   at MySql.Data.MySqlClient.Driver.NextResult(Int32 statementId, Boolean force)
   at MySql.Data.MySqlClient.MySqlDataReader.NextResult()
   at MySql.Data.MySqlClient.MySqlCommand.ExecuteReader(CommandBehavior behavior)
   at MySql.Data.MySqlClient.MySqlCommand.ExecuteNonQuery()
   at TShockAPI.DB.DbExt.Query(IDbConnection olddb, String query, Object[] args)
   at TShockAPI.SqlLog.Write(String message, TraceLevel level)
2017-06-27 11:27:55 - SqlLog: ERROR: SQL Logging disabled due to errors. Reverting to text logging.
2017-06-27 11:27:55 - SqlLog: ERROR: SQL log failed at: 2017-06-27 18:27:36. Message: Utils: INFO: zeff executed: /me [c/ffoooo:jumps, HOOraY!] good for u! 👏.

Reproducibility

Screenshot N/A

bartico6 commented 6 years ago

To fix the issue, create the server log table with a default charset utf8mb4 and the collation corresponding to that (so instead of utf8_bin you'd do utf8mb4_bin).

Reason for this crash is because some emojis are 4-bytes long, and by default utf8 in MySQL is 3-bytes long. I ran into this importing usernames from SQLite to MySQL during server maintenance.

It's literally a one-liner.

QuiCM commented 6 years ago

I put my comment in the wrong place. From #1516:

We should update our table creation method though, so that future versions are compatible with emojis. If @bartico6 could set up a small walkthrough we can provide upgrade steps for existing DBs

hakusaro commented 6 years ago

From Quake in Discord:

since I'm in charge of doing this utf8 -> utf8mb4 stuff people should use binary or case insensitive comparison for mysql if I do utf8mb4_bin like you tried in that commit you later scrapped it'll allow duplicate names with varying letter cases aka "quaKe" "QUAKE" and "quake" can exist alongside each other decision?(

bartico6 commented 6 years ago

I meant to ask if we should enforce binary or case insensitive comparisons? IIRC binary might be faster, and it was already in @hakusaro's PR before it got scrapped - but that will allow duplicate names with varying casing to be used, which you guys mentioned shouldn't be a thing. I'll need a final decision on whether dupe names are okay from the team before I make any sort of importer/walkthrough.