Never send an unencoded packet to the server (#719)

* never send an unencoded packet to the server

* oops

* Improve packet validation to remove segfaults

* WARNING: commit breaks connecting to servers, need help
start fixing omniwhy caused by single fuckin string packets (AAAAAAAAAAAAAAAAA)

* Fix failed connections to servers (Thanks to @Iuvee for helping me figure this out!)

* Fix demoserver

* who the fuck still uses goto

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* ANOTHER GOTO????

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* braces

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* good bot Update src/packet_distribution.cpp

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Fix demoserver harder

* Improve demo logging

* Fix memory leakage by deleting the packet
Fix useless demoserver wait packet creation when none of that packet is used

Co-authored-by: stonedDiscord <Tukz@gmx.de>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Salanto <62221668+Salanto@users.noreply.github.com>

1 files changed, 83 insertions, 68 deletions

diff --git a/src/packet_distribution.cpp b/src/packet_distribution.cpp
index 0fc5eee1..3555e21b 100644
--- a/src/packet_distribution.cpp
+++ b/src/packet_distribution.cpp
@@ -29,6 +29,8 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
   QStringList f_contents = p_packet->get_contents();
   QString f_packet = p_packet->to_string();
 
+  bool log_to_demo = true;
+
 #ifdef DEBUG_NETWORK
   if (header != "checkconnection")
     qDebug() << "R:" << f_packet;
@@ -57,8 +59,10 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     QString f_hdid;
     f_hdid = get_hdid();
 
-    AOPacket *hi_packet = new AOPacket("HI#" + f_hdid + "#%");
+    QStringList f_contents = {f_hdid};
+    AOPacket *hi_packet = new AOPacket("HI", f_contents);
     send_server_packet(hi_packet);
+    log_to_demo = false;
   }
   else if (header == "ID") {
     if (f_contents.size() < 2)
@@ -70,22 +74,20 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     if (lobby_constructed)
       w_lobby->enable_connect_button();
 
-    send_server_packet(new AOPacket("ID#AO2#" + get_version_string() + "#%"));
+    QStringList f_contents = {"AO2", get_version_string()};
+    send_server_packet(new AOPacket("ID", f_contents));
   }
   else if (header == "CT") {
-    if (f_contents.size() < 2)
+    if (!courtroom_constructed || f_contents.size() < 2) {
       goto end;
+}
 
-    if (courtroom_constructed) {
-      if (f_contents.size() == 3)
-        w_courtroom->append_server_chatmessage(
-            f_contents.at(0), f_contents.at(1), f_contents.at(2));
-      else
-        w_courtroom->append_server_chatmessage(f_contents.at(0),
-                                               f_contents.at(1), "0");
-
-      append_to_demofile(f_packet_encoded);
-    }
+    if (f_contents.size() == 3)
+      w_courtroom->append_server_chatmessage(
+          f_contents.at(0), f_contents.at(1), f_contents.at(2));
+    else
+      w_courtroom->append_server_chatmessage(f_contents.at(0),
+                                             f_contents.at(1), "0");
   }
   else if (header == "FL") {
     yellow_text_supported = false;
@@ -135,9 +137,10 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
       expanded_desk_mods_supported = true;
     if (f_packet.contains("auth_packet", Qt::CaseInsensitive))
       auth_packet_supported = true;
+    log_to_demo = false;
   }
   else if (header == "PN") {
-    if (f_contents.size() < 2)
+    if (!lobby_constructed || f_contents.size() < 2)
       goto end;
 
     w_lobby->set_player_count(f_contents.at(0).toInt(),
@@ -148,13 +151,15 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     }
 
     if (w_lobby->doubleclicked) {
-        send_server_packet(new AOPacket("askchaa#%"));
-        w_lobby->doubleclicked = false;
+      send_server_packet(new AOPacket("askchaa"));
+      w_lobby->doubleclicked = false;
     }
+    log_to_demo = false;
   }
   else if (header == "SI") {
-    if (f_contents.size() != 3)
+    if (!lobby_constructed || f_contents.size() != 3) {
       goto end;
+}
 
     char_list_size = f_contents.at(0).toInt();
     evidence_list_size = f_contents.at(1).toInt();
@@ -195,7 +200,8 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
       }
     }
 
-    w_courtroom->set_window_title(window_title);
+    if (courtroom_constructed)
+      w_courtroom->set_window_title(window_title);
 
     w_lobby->show_loading_overlay();
     w_lobby->set_loading_text(tr("Loading"));
@@ -203,7 +209,7 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
 
     AOPacket *f_packet;
 
-    f_packet = new AOPacket("RC#%");
+    f_packet = new AOPacket("RC");
     send_server_packet(f_packet);
 
     // Remove any characters not accepted in folder names for the server_name
@@ -225,6 +231,7 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     if (is_discord_enabled())
       discord->state_server(server_name.toStdString(),
                             hash.result().toBase64().toStdString());
+    log_to_demo = false;
   }
   else if (header == "CharsCheck") {
     if (!courtroom_constructed)
@@ -236,6 +243,7 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
       else
         w_courtroom->set_taken(n_char, false);
     }
+    log_to_demo = false;
   }
 
   else if (header == "SC") {
@@ -274,10 +282,9 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     }
 
     if (!courtroom_loaded)
-      send_server_packet(new AOPacket("RM#%"));
+      send_server_packet(new AOPacket("RM"));
     else
       w_courtroom->character_loading_finished();
-    append_to_demofile(f_packet_encoded);
   }
   else if (header == "SM") {
     if (!courtroom_constructed || courtroom_loaded)
@@ -326,7 +333,8 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
       w_courtroom->arup_append(0, "Unknown", "Unknown", "Unknown");
     }
 
-    send_server_packet(new AOPacket("RD#%"));
+    send_server_packet(new AOPacket("RD"));
+    log_to_demo = false;
   }
   else if (header == "FM") // Fetch music ONLY
   {
@@ -340,6 +348,7 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     }
 
     w_courtroom->list_music();
+    log_to_demo = false;
   }
   else if (header == "FA") // Fetch areas ONLY
   {
@@ -355,6 +364,7 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     }
 
     w_courtroom->list_areas();
+    log_to_demo = false;
   }
   else if (header == "DONE") {
     if (!courtroom_constructed)
@@ -370,80 +380,68 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     courtroom_loaded = true;
 
     destruct_lobby();
+    log_to_demo = false;
   }
   else if (header == "BN") {
-    if (f_contents.size() < 1)
+    if (!courtroom_constructed || f_contents.isEmpty())
       goto end;
 
-    if (courtroom_constructed) {
-      if (f_contents.size() >= 2) {
-        // We have a pos included in the background packet!
-        w_courtroom->set_side(f_contents.at(1));
-      }
-      w_courtroom->set_background(f_contents.at(0), f_contents.size() >= 2);
-      append_to_demofile(f_packet_encoded);
+    if (f_contents.size() >= 2) {
+      // We have a pos included in the background packet!
+      w_courtroom->set_side(f_contents.at(1));
     }
+    w_courtroom->set_background(f_contents.at(0), f_contents.size() >= 2);
   }
   else if (header == "SP") {
-    if (f_contents.size() < 1)
+    if (!courtroom_constructed || f_contents.isEmpty())
       goto end;
 
-    if (courtroom_constructed) // We were sent a "set position" packet
-    {
-      w_courtroom->set_side(f_contents.at(0));
-      append_to_demofile(f_packet_encoded);
-    }
+    // We were sent a "set position" packet
+    w_courtroom->set_side(f_contents.at(0));
   }
   else if (header == "SD") // Send pos dropdown
   {
-    if (f_contents.size() < 1)
+    if (!courtroom_constructed || f_contents.isEmpty())
       goto end;
 
     w_courtroom->set_pos_dropdown(f_contents.at(0).split("*"));
   }
   // server accepting char request(CC) packet
   else if (header == "PV") {
-    if (f_contents.size() < 3)
+    if (!courtroom_constructed || f_contents.size() < 3)
       goto end;
-
+    // For some reason, args 0 and 1 are not used (from tsu3 they're client ID and a string "CID")
     w_courtroom->enter_courtroom();
-
-    if (courtroom_constructed) {
-      w_courtroom->set_courtroom_size();
-      w_courtroom->update_character(f_contents.at(2).toInt());
-    }
+    w_courtroom->set_courtroom_size();
+    w_courtroom->update_character(f_contents.at(2).toInt());
   }
   else if (header == "MS") {
     if (courtroom_constructed && courtroom_loaded)
     {
       w_courtroom->chatmessage_enqueue(p_packet->get_contents());
-      append_to_demofile(f_packet_encoded);
     }
   }
   else if (header == "MC") {
     if (courtroom_constructed && courtroom_loaded)
     {
       w_courtroom->handle_song(&p_packet->get_contents());
-      append_to_demofile(f_packet_encoded);
     }
   }
   else if (header == "RT") {
-    if (f_contents.size() < 1)
+    if (f_contents.isEmpty())
       goto end;
     if (courtroom_constructed) {
       if (f_contents.size() == 1)
         w_courtroom->handle_wtce(f_contents.at(0), 0);
-      else if (f_contents.size() == 2)
+      else if (f_contents.size() >= 2)
         w_courtroom->handle_wtce(f_contents.at(0), f_contents.at(1).toInt());
-      append_to_demofile(f_packet_encoded);
     }
   }
   else if (header == "HP") {
-    if (courtroom_constructed && f_contents.size() > 1)
+    if (courtroom_constructed && f_contents.size() >= 2)
     {
       w_courtroom->set_hp_bar(f_contents.at(0).toInt(),
                               f_contents.at(1).toInt());
-      append_to_demofile(f_packet_encoded);
     }
   }
   else if (header == "LE") {
@@ -469,11 +467,10 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
       }
 
       w_courtroom->set_evidence_list(f_evi_list);
-      append_to_demofile(f_packet_encoded);
     }
   }
   else if (header == "ARUP") {
-    if (courtroom_constructed) {
+    if (courtroom_constructed && !f_contents.isEmpty()) {
       int arup_type = f_contents.at(0).toInt();
       for (int n_element = 1; n_element < f_contents.size(); n_element++) {
         w_courtroom->arup_modify(arup_type, n_element - 1,
@@ -481,46 +478,58 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
       }
       w_courtroom->list_areas();
     }
+    log_to_demo = false;
   }
   else if (header == "IL") {
-    if (courtroom_constructed && f_contents.size() > 0)
+    if (courtroom_constructed && !f_contents.isEmpty())
       w_courtroom->set_ip_list(f_contents.at(0));
+    log_to_demo = false;
   }
   else if (header == "MU") {
-    if (courtroom_constructed && f_contents.size() > 0)
+    if (courtroom_constructed && !f_contents.isEmpty())
       w_courtroom->set_mute(true, f_contents.at(0).toInt());
+    log_to_demo = false;
   }
   else if (header == "UM") {
-    if (courtroom_constructed && f_contents.size() > 0)
+    if (courtroom_constructed && !f_contents.isEmpty()) {
       w_courtroom->set_mute(false, f_contents.at(0).toInt());
+    log_to_demo = false;
+}
   }
   else if (header == "BB") {
-    if (courtroom_constructed && f_contents.size() >= 1) {
+    if (courtroom_constructed && !f_contents.isEmpty()) {
       call_notice(f_contents.at(0));
     }
+    log_to_demo = false;
   }
   else if (header == "KK") {
-    if (courtroom_constructed && f_contents.size() >= 1) {
+    if (courtroom_constructed && !f_contents.isEmpty()) {
       call_notice(tr("You have been kicked from the server.\nReason: %1")
                       .arg(f_contents.at(0)));
       construct_lobby();
       destruct_courtroom();
     }
+    log_to_demo = false;
   }
   else if (header == "KB") {
-    if (courtroom_constructed && f_contents.size() >= 1) {
+    if (courtroom_constructed && !f_contents.isEmpty()) {
       call_notice(tr("You have been banned from the server.\nReason: %1")
                       .arg(f_contents.at(0)));
       construct_lobby();
       destruct_courtroom();
     }
+    log_to_demo = false;
   }
   else if (header == "BD") {
+    if (f_contents.isEmpty()) {
+      goto end;
+    }
     call_notice(
         tr("You are banned on this server.\nReason: %1").arg(f_contents.at(0)));
+    log_to_demo = false;
   }
   else if (header == "ZZ") {
-    if (courtroom_constructed && f_contents.size() > 0)
+    if (courtroom_constructed && !f_contents.isEmpty())
       w_courtroom->mod_called(f_contents.at(0));
   }
   else if (header == "CASEA") {
@@ -573,7 +582,6 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
       w_courtroom->set_clock_visibility(id, true);
     else if (type == 3)
       w_courtroom->set_clock_visibility(id, false);
-    append_to_demofile(f_packet_encoded);
   }
   else if (header == "CHECK") {
     if (!courtroom_constructed)
@@ -583,10 +591,11 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     qDebug() << "ping:" << ping_time;
     if (ping_time != -1)
       latency = ping_time;
+    log_to_demo = false;
   }
   // Subtheme packet
   else if (header == "ST") {
-    if (!courtroom_constructed)
+    if (!courtroom_constructed || f_contents.isEmpty())
       goto end;
     // Subtheme reserved as argument 0
     subtheme = f_contents.at(0);
@@ -603,8 +612,9 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
   }
   // Auth packet
   else if (header == "AUTH") {
-    if (!courtroom_constructed || !auth_packet_supported || f_contents.size() < 1)
+    if (!courtroom_constructed || !auth_packet_supported || f_contents.isEmpty()) {
       goto end;
+}
     bool ok;
     int authenticated = f_contents.at(0).toInt(&ok);
     if (!ok) {
@@ -612,9 +622,10 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     }
 
     w_courtroom->on_authentication_state_received(authenticated);
+    log_to_demo = false;
   }
   else if (header == "JD") {
-    if (!courtroom_constructed || f_contents.empty()) {
+    if (!courtroom_constructed || f_contents.isEmpty()) {
       goto end;
     }
     bool ok;
@@ -633,7 +644,7 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
 
  //AssetURL Packet
   else if (header == "ASS") {
-    if (f_contents.size() > 1 || f_contents.size() == 0) { // This can never be more than one link.
+    if (f_contents.size() > 1 || f_contents.isEmpty()) { // This can never be more than one link.
       goto end;
     }
     QUrl t_asset_url = QUrl::fromPercentEncoding(f_contents.at(0).toUtf8());
@@ -641,15 +652,19 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
     asset_url = t_asset_url.toString();
   }
 
+  if (log_to_demo) {
+    append_to_demofile(f_packet_encoded);
+  }
+
 end:
 
   delete p_packet;
 }
 
-void AOApplication::send_server_packet(AOPacket *p_packet, bool encoded)
+void AOApplication::send_server_packet(AOPacket *p_packet)
 {
-  if (encoded)
-    p_packet->net_encode();
+  // ***NEVER*** send an unencoded packet.
+  p_packet->net_encode();
 
   QString f_packet = p_packet->to_string();
 #ifdef DEBUG_NETWORK