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>

base/themes

@@ -1 +1 @@
-Subproject commit 859f59b5d441fbf6c4d992a1af3ece35f1b0e946
+Subproject commit 413411ae0f421066bb9c605d6757fa2cf588d7d7

include/aoapplication.h

@@ -78,7 +78,7 @@ public:
 
   void server_packet_received(AOPacket *p_packet);
 
-  void send_server_packet(AOPacket *p_packet, bool encoded = true);
+  void send_server_packet(AOPacket *p_packet);
 
   void call_settings_menu();
   void call_announce_menu(Courtroom *court);

include/aopacket.h

@@ -7,8 +7,8 @@
 
 class AOPacket {
 public:
-  AOPacket(QString p_packet_string);
+  AOPacket(QString header) : m_header(header){}
-  AOPacket(QString header, QStringList &p_contents) : m_header(header), m_contents(p_contents){}
+  AOPacket(QString header, QStringList p_contents) : m_header(header), m_contents(p_contents){}
 
   QString get_header() { return m_header; }
   QStringList &get_contents() { return m_contents; }

include/demoserver.h

@@ -23,7 +23,7 @@ public:
     int max_wait = -1;
 
 private:
-    void handle_packet(AOPacket packet);
+    void handle_packet(AOPacket *packet);
     void load_demo(QString filename);
     void reset_state();
 

src/aopacket.cpp

@@ -1,18 +1,15 @@
 #include "aopacket.h"
 
-AOPacket::AOPacket(QString p_packet_string)
-{
-  QStringList packet_contents = p_packet_string.split("#");
-
-  m_header = packet_contents.first();
-  m_contents = packet_contents.mid(1, packet_contents.size()-2); // trims %
-}
-
 QString AOPacket::to_string(bool encoded)
 {
   QStringList contents = m_contents;
-  if (encoded)
+  if (encoded) {
     escape(contents);
+  }
+  // Our packet is just the header by itself
+  if (contents.isEmpty()) {
+    return m_header + "#%";
+  }
   return m_header + "#" + contents.join("#") + "#%";
 }
 

src/charselect.cpp

@@ -178,10 +178,9 @@ void Courtroom::char_clicked(int n_char)
 
   if (n_char != m_cid || n_char == -1) {
     ao_app->send_server_packet(
-        new AOPacket("PW#" + ui_char_password->text() + "#%"));
+        new AOPacket("PW", {ui_char_password->text()}));
     ao_app->send_server_packet(
-        new AOPacket("CC#" + QString::number(ao_app->s_pv) + "#" +
+        new AOPacket("CC", {QString::number(ao_app->s_pv), QString::number(n_char), get_hdid()}));
-                     QString::number(n_char) + "#" + get_hdid() + "#%"));
   }
   if (n_char == m_cid || n_char == -1) {
     update_character(n_char);

src/courtroom.cpp

@@ -4248,19 +4248,18 @@ void Courtroom::on_ooc_return_pressed()
       append_server_chatmessage(tr("CLIENT"),
                                 tr("Case made by %1.").arg(caseauth), "1");
     if (!casedoc.isEmpty()) {
-      QStringList f_contents = {ui_ooc_chat_name->text(), "/doc " + casedoc};
+      ao_app->send_server_packet(new AOPacket("CT", {ui_ooc_chat_name->text(), "/doc " + casedoc}));
-      ao_app->send_server_packet(new AOPacket("CT", f_contents));
+    }
+    if (!casestatus.isEmpty()) {
+      ao_app->send_server_packet(new AOPacket("CT", {ui_ooc_chat_name->text(), "/status " + casestatus}));
     }
-    if (!casestatus.isEmpty())
-      ao_app->send_server_packet(new AOPacket("CT#" + ui_ooc_chat_name->text() +
-                                              "#/status " + casestatus + "#%"));
     if (!cmdoc.isEmpty())
       append_server_chatmessage(
           "CLIENT", tr("Navigate to %1 for the CM doc.").arg(cmdoc), "1");
 
     for (int i = local_evidence_list.size() - 1; i >= 0; i--) {
       ao_app->send_server_packet(
-          new AOPacket("DE#" + QString::number(i) + "#%"));
+          new AOPacket("DE", {QString::number(i)}));
     }
 
     // sort the case_evidence numerically
@@ -4894,7 +4893,7 @@ void Courtroom::on_music_list_double_clicked(QTreeWidgetItem *p_item,
     packet_contents.append(ui_ic_chat_name->text());
   if (ao_app->effects_supported)
     packet_contents.append(QString::number(music_flags));
-  ao_app->send_server_packet(new AOPacket("MC", packet_contents), false);
+  ao_app->send_server_packet(new AOPacket("MC", packet_contents));
 }
 
 void Courtroom::on_music_list_context_menu_requested(const QPoint &pos)
@@ -5017,7 +5016,7 @@ void Courtroom::music_stop(bool no_effects)
     else
       packet_contents.append(QString::number(music_flags));
   }
-  ao_app->send_server_packet(new AOPacket("MC", packet_contents), false);
+  ao_app->send_server_packet(new AOPacket("MC", packet_contents));
 }
 
 void Courtroom::on_area_list_double_clicked(QTreeWidgetItem *p_item, int column)
@@ -5029,7 +5028,7 @@ void Courtroom::on_area_list_double_clicked(QTreeWidgetItem *p_item, int column)
   QStringList packet_contents;
   packet_contents.append(p_area);
   packet_contents.append(QString::number(m_cid));
-  ao_app->send_server_packet(new AOPacket("MC", packet_contents), false);
+  ao_app->send_server_packet(new AOPacket("MC", packet_contents));
 }
 
 void Courtroom::on_hold_it_clicked()
@@ -5210,7 +5209,7 @@ void Courtroom::on_defense_minus_clicked()
 
   if (f_state >= 0)
     ao_app->send_server_packet(
-        new AOPacket("HP#1#" + QString::number(f_state) + "#%"));
+        new AOPacket("HP", {"1", QString::number(f_state)}));
 }
 
 void Courtroom::on_defense_plus_clicked()
@@ -5219,7 +5218,7 @@ void Courtroom::on_defense_plus_clicked()
 
   if (f_state <= 10)
     ao_app->send_server_packet(
-        new AOPacket("HP#1#" + QString::number(f_state) + "#%"));
+        new AOPacket("HP", {"1", QString::number(f_state)}));
 }
 
 void Courtroom::on_prosecution_minus_clicked()
@@ -5228,7 +5227,7 @@ void Courtroom::on_prosecution_minus_clicked()
 
   if (f_state >= 0)
     ao_app->send_server_packet(
-        new AOPacket("HP#2#" + QString::number(f_state) + "#%"));
+        new AOPacket("HP", {"2", QString::number(f_state)}));
 }
 
 void Courtroom::on_prosecution_plus_clicked()
@@ -5237,7 +5236,7 @@ void Courtroom::on_prosecution_plus_clicked()
 
   if (f_state <= 10)
     ao_app->send_server_packet(
-        new AOPacket("HP#2#" + QString::number(f_state) + "#%"));
+        new AOPacket("HP", {"2", QString::number(f_state)}));
 }
 
 void Courtroom::set_text_color_dropdown()
@@ -5373,7 +5372,7 @@ void Courtroom::on_witness_testimony_clicked()
   if (is_muted)
     return;
 
-  ao_app->send_server_packet(new AOPacket("RT#testimony1#%"));
+  ao_app->send_server_packet(new AOPacket("RT", {"testimony1"}));
 
   ui_ic_chat_message->setFocus();
 }
@@ -5383,7 +5382,7 @@ void Courtroom::on_cross_examination_clicked()
   if (is_muted)
     return;
 
-  ao_app->send_server_packet(new AOPacket("RT#testimony2#%"));
+  ao_app->send_server_packet(new AOPacket("RT", {"testimony2"}));
 
   ui_ic_chat_message->setFocus();
 }
@@ -5393,7 +5392,7 @@ void Courtroom::on_not_guilty_clicked()
   if (is_muted)
     return;
 
-  ao_app->send_server_packet(new AOPacket("RT#judgeruling#0#%"));
+  ao_app->send_server_packet(new AOPacket("RT", {"judgeruling", "0"}));
 
   ui_ic_chat_message->setFocus();
 }
@@ -5403,7 +5402,7 @@ void Courtroom::on_guilty_clicked()
   if (is_muted)
     return;
 
-  ao_app->send_server_packet(new AOPacket("RT#judgeruling#1#%"));
+  ao_app->send_server_packet(new AOPacket("RT", {"judgeruling", "1"}));
 
   ui_ic_chat_message->setFocus();
 }
@@ -5482,7 +5481,7 @@ void Courtroom::on_call_mod_clicked()
     ao_app->send_server_packet(new AOPacket("ZZ", mod_reason));
   }
   else {
-    ao_app->send_server_packet(new AOPacket("ZZ#%"));
+    ao_app->send_server_packet(new AOPacket("ZZ"));
   }
 
   ui_ic_chat_message->setFocus();
@@ -5567,7 +5566,7 @@ void Courtroom::ping_server()
   ping_timer.start();
   is_pinging = true;
   ao_app->send_server_packet(
-      new AOPacket("CH#" + QString::number(m_cid) + "#%"));
+      new AOPacket("CH", {QString::number(m_cid)}));
 }
 
 qint64 Courtroom::pong()
@@ -5597,7 +5596,7 @@ void Courtroom::on_casing_clicked()
       ao_app->send_server_packet(new AOPacket("SETCASE", f_packet));
     }
     else
-      ao_app->send_server_packet(new AOPacket("SETCASE#\"\"#0#0#0#0#0#0#%"));
+      ao_app->send_server_packet(new AOPacket("SETCASE", {"","0","0","0","0","0","0"}));
   }
 }
 

src/demoserver.cpp

@@ -80,36 +80,35 @@ void DemoServer::recv_data()
     QString in_data = QString::fromUtf8(client_sock->readAll());
 
     // Copypasted from NetworkManager
-    if (!in_data.endsWith("%")) {
+    const QStringList packet_list = in_data.split("%", Qt::SkipEmptyParts);
-      partial_packet = true;
-      temp_packet += in_data;
-      return;
-    }
-
-    else {
-      if (partial_packet) {
-        in_data = temp_packet + in_data;
-        temp_packet = "";
-        partial_packet = false;
-      }
-    }
-
-    QStringList packet_list =
-#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)
-        in_data.split("%", QString::SplitBehavior(QString::SkipEmptyParts));
-#else
-        in_data.split("%", Qt::SkipEmptyParts);
-#endif
 
     for (const QString &packet : packet_list) {
-        AOPacket ao_packet(packet);
+        QStringList f_contents;
-        handle_packet(ao_packet);
+        // Packet should *always* end with #
+        if (packet.endsWith("#")) {
+          f_contents = packet.chopped(1).split("#");
+        }
+        // But, if it somehow doesn't, we should still be able to handle it
+        else {
+          f_contents = packet.split("#");
+        }
+        // Empty packets are suspicious!
+        if (f_contents.isEmpty()) {
+          qWarning() << "WARNING: Empty packet received from server, skipping...";
+          continue;
+        }
+        // Take the first arg as the command
+        QString command = f_contents.takeFirst();
+        // The rest is contents of the packet
+        AOPacket *f_packet = new AOPacket(command, f_contents);
+        // Ship it to the server!
+        handle_packet(f_packet);
     }
 }
 
-void DemoServer::handle_packet(AOPacket packet)
+void DemoServer::handle_packet(AOPacket *p_packet)
 {
-    packet.net_decode();
+    p_packet->net_decode();
 
     // This code is literally a barebones AO server
     // It is wise to do it this way, because I can
@@ -119,8 +118,8 @@ void DemoServer::handle_packet(AOPacket packet)
     // Also, at some point, I will make akashit
     // into a shared library.
 
-    QString header = packet.get_header();
+    QString header = p_packet->get_header();
-    QStringList contents = packet.get_contents();
+    QStringList contents = p_packet->get_contents();
 
     if (header == "HI") {
         client_sock->write("ID#0#DEMOINTERNAL#0#%");
@@ -273,6 +272,8 @@ void DemoServer::handle_packet(AOPacket packet)
             client_sock->write(packet.toUtf8());
         }
     }
+
+    delete p_packet;
 }
 
 void DemoServer::load_demo(QString filename)
@@ -390,15 +391,28 @@ void DemoServer::playback()
         current_packet = demo_data.dequeue();
     }
     if (!demo_data.isEmpty()) {
-        AOPacket wait_packet = AOPacket(current_packet);
+        QStringList f_contents;
-
+        // Packet should *always* end with #
-        int duration = wait_packet.get_contents().at(0).toInt();
+        if (current_packet.endsWith("#")) {
+          f_contents = current_packet.chopped(1).split("#");
+        }
+        // But, if it somehow doesn't, we should still be able to handle it
+        else {
+          f_contents = current_packet.split("#");
+        }
+        // Take the first arg as the command
+        QString command = f_contents.takeFirst();
+        int duration = 0;
+        if (!f_contents.isEmpty()) {
+          duration = f_contents.at(0).toInt();
+        }
         // Max wait reached
         if (max_wait != -1 && duration + elapsed_time > max_wait) {
+          int prev_duration = duration;
           duration = qMax(0, max_wait - elapsed_time);
           qDebug() << "Max_wait of " << max_wait << " reached. Forcing duration to " << duration << "ms";
           // Skip the difference on the timers
-          emit skip_timers(wait_packet.get_contents().at(0).toInt() - duration);
+          emit skip_timers(prev_duration - duration);
         }
         // Manual user skip, such as with >
         else if (timer->remainingTime() > 0) {

src/evidence.cpp

@@ -436,7 +436,7 @@ void Courtroom::on_evidence_clicked(int p_id)
   if (f_real_id == local_evidence_list.size()) {
     if (current_evidence_global)
       ao_app->send_server_packet(
-          new AOPacket("PE#<name>#<description>#empty.png#%"));
+          new AOPacket("PE", {"<name>", "<description>", "empty.png"}));
     else {
       evi_type f_evi;
       f_evi.name = "<name>";
@@ -567,7 +567,7 @@ void Courtroom::on_evidence_delete_clicked()
   evidence_close();
   if (current_evidence_global)
     ao_app->send_server_packet(
-        new AOPacket("DE#" + QString::number(current_evidence) + "#%"));
+        new AOPacket("DE", {QString::number(current_evidence)}));
   else {
     local_evidence_list.remove(current_evidence);
     private_evidence_list = local_evidence_list;

src/lobby.cpp

@@ -371,7 +371,7 @@ void Lobby::on_connect_released()
 
   AOPacket *f_packet;
 
-  f_packet = new AOPacket("askchaa#%");
+  f_packet = new AOPacket("askchaa");
 
   ao_app->send_server_packet(f_packet);
 }

src/networkmanager.cpp

@@ -240,15 +240,29 @@ void NetworkManager::handle_server_packet(const QString& p_data)
       partial_packet = false;
     }
   }
-#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)
+
-  const QStringList packet_list = in_data.split("%", QString::SkipEmptyParts);
-#else
   const QStringList packet_list = in_data.split("%", Qt::SkipEmptyParts);
-#endif
 
   for (const QString &packet : packet_list) {
-    AOPacket *f_packet = new AOPacket(packet);
+    QStringList f_contents;
-
+    // Packet should *always* end with #
+    if (packet.endsWith("#")) {
+      f_contents = packet.chopped(1).split("#");
+    }
+    // But, if it somehow doesn't, we should still be able to handle it
+    else {
+      f_contents = packet.split("#");
+    }
+    // Empty packets are suspicious!
+    if (f_contents.isEmpty()) {
+      qWarning() << "WARNING: Empty packet received from server, skipping...";
+      continue;
+    }
+    // Take the first arg as the command
+    QString command = f_contents.takeFirst();
+    // The rest is contents of the packet
+    AOPacket *f_packet = new AOPacket(command, f_contents);
+    // Ship it to the server!
     ao_app->server_packet_received(f_packet);
   }
 }

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);
-    }
   }
   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#%"));
+      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,6 +200,7 @@ void AOApplication::server_packet_received(AOPacket *p_packet)
       }
     }
 
+    if (courtroom_constructed)
       w_courtroom->set_window_title(window_title);
 
     w_lobby->show_loading_overlay();
@@ -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);
-    }
   }
   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
+    // We were sent a "set position" packet
-    {
     w_courtroom->set_side(f_contents.at(0));
-      append_to_demofile(f_packet_encoded);
-    }
   }
   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());
   }
-  }
   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,14 +652,18 @@ 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)
+  // ***NEVER*** send an unencoded packet.
   p_packet->net_encode();
 
   QString f_packet = p_packet->to_string();