From 0b9144ba7c3b78c6f1b7bd1ac84db9529b03e776 Mon Sep 17 00:00:00 2001
From: Alex Karle <alex@karle.co>
Date: Wed, 6 May 2020 23:51:26 -0400
Subject: [PATCH] Euchre::Errors: Add safeguards to ensure keys present in msg

We had a bug during our playthrough tonight that would have been more
obvious if the server had failed loudly at the required key not being
present in the message.

This commit adds those safeguards, with a check for required fields
prior to handling any other requirements or doing any other work.

This should ideally be transparent to end users, and only be used so
thta we don't enter a "cascading bad" state if a client sends a poor
message.
---
 lib/Euchre/Dealer.pm | 31 +++++++++++++++++++++----------
 lib/Euchre/Errors.pm |  3 +++
 lib/Euchre/Host.pm   | 13 +++++++++++++
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/lib/Euchre/Dealer.pm b/lib/Euchre/Dealer.pm
index e8fd9e8..0f2431e 100644
--- a/lib/Euchre/Dealer.pm
+++ b/lib/Euchre/Dealer.pm
@@ -58,22 +58,22 @@ sub handle_msg {
 
     # Crazy magic dispatch of
     #
-    #   action => [ handler, req-phase, phase-err, needs-turn ]
+    #   action => [ handler, req-keys, req-phase, phase-err, needs-turn ]
     #
     # The last three are optional, but are useful to dedupe common
     # assertions (like, needs to be their turn)
     my %dispatch = (
         # Game management endpoints
-        chat         => [\&chat],
-        take_seat    => [\&take_seat],
-        stand_up     => [\&stand_up],
-        start_game   => [\&start_game, 'lobby', START_GAME],
-        restart_game => [\&restart_game, 'end', RESTART_GAME],
+        chat         => [\&chat, ['msg']],
+        take_seat    => [\&take_seat, ['seat']],
+        stand_up     => [\&stand_up, []],
+        start_game   => [\&start_game, [], 'lobby', START_GAME],
+        restart_game => [\&restart_game, [], 'end', RESTART_GAME],
 
         # Gameplay
-        order        => [\&order, 'vote', ORDER, 1],
-        dealer_swap  => [\&dealer_swap, 'dealer_swap', DEALER_SWAP, 1],
-        play_card    => [\&play_card, 'play', PLAY_CARD, 1],
+        order        => [\&order, ['vote', 'loner'], 'vote', ORDER, 1],
+        dealer_swap  => [\&dealer_swap, ['card'], 'dealer_swap', DEALER_SWAP, 1],
+        play_card    => [\&play_card, ['card'], 'play', PLAY_CARD, 1],
     );
 
 
@@ -83,7 +83,18 @@ sub handle_msg {
     }
 
     my $p = $self->players->{$cid};
-    my ($handler, $req_phase, $phase_err, $turn_based) = @{$dispatch{$msg->{action}}};
+    my ($handler, $req_keys, $req_phase, $phase_err, $turn_based) =
+        @{$dispatch{$msg->{action}}};
+
+    # Validate that all required msg keys are present
+    for my $k (@$req_keys) {
+        if (!exists $msg->{$k}) {
+            $p->error(MISSING_PARAM);
+            return;
+        }
+    }
+
+    # Next validate phase, turn, and handle success/failure
     if ($req_phase && ($self->game->phase ne $req_phase)) {
         $p->error($phase_err);
     } elsif ($turn_based && ($p->seat != $self->game->turn)) {
diff --git a/lib/Euchre/Errors.pm b/lib/Euchre/Errors.pm
index de11395..0b0d008 100644
--- a/lib/Euchre/Errors.pm
+++ b/lib/Euchre/Errors.pm
@@ -28,6 +28,7 @@ use constant {
     BAD_PASS        => 19,
     NOT_AT_TABLE    => 20,
     CHANGE_SEAT     => 21,
+    MISSING_PARAM   => 22,
 };
 
 require Exporter;
@@ -56,6 +57,7 @@ our @EXPORT = qw(
     NOT_IN_GAME
     BAD_PASS
     NOT_AT_TABLE
+    MISSING_PARAM
 );
 
 our @ERR_MSGS = ();
@@ -80,6 +82,7 @@ $ERR_MSGS[DONT_HAVE_CARD]  = "You don't have that card!";
 $ERR_MSGS[NOT_IN_GAME]     = "You're not in any game";
 $ERR_MSGS[BAD_PASS]        = "Game exists, password incorrect";
 $ERR_MSGS[NOT_AT_TABLE]    = "Need to be at a table for action";
+$ERR_MSGS[MISSING_PARAM]   = "Server received incomplete message";
 
 sub err_msg {
     my ($errno) = @_;
diff --git a/lib/Euchre/Host.pm b/lib/Euchre/Host.pm
index 51f8cf8..552c2d7 100644
--- a/lib/Euchre/Host.pm
+++ b/lib/Euchre/Host.pm
@@ -114,6 +114,8 @@ sub pong {
 sub join_table {
     my ($p, $msg) = @_;
 
+    require_keys(@_, qw(table player_name)) or return;
+
     my $tid = $msg->{table};
 
     $p->name($msg->{player_name});
@@ -190,4 +192,15 @@ sub stats {
     return $msg;
 }
 
+sub require_keys {
+    my ($p, $msg, @req_keys) = @_;
+    for my $k (@req_keys) {
+        if (!exists $msg->{$k}) {
+            $p->error(MISSING_PARAM);
+            return 0;
+        }
+    }
+    return 1;
+}
+
 1;
--
libgit2 1.1.1