[Freeciv-Dev] (PR#13763) improve client state robustness

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13763 >

Various parts of the client will fail if it receives certain packets 
while detached (/detach).  In particular, if there's no player then 
game.player_ptr will either buffer overrun (a security flaw that should 
be fixed in 2.0 also) or, if set to NULL as it should be, will cause a 
crash pretty quickly.

This patch fixes some of the worst offenders.

-jason

? common/diptreaty.o.eFGlh4
Index: client/civclient.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/civclient.c,v
retrieving revision 1.229
diff -p -u -r1.229 civclient.c
--- client/civclient.c  1 Aug 2005 06:48:28 -0000       1.229
+++ client/civclient.c  24 Aug 2005 17:46:02 -0000
@@ -710,7 +710,8 @@ double real_timer_callback(void)
 **************************************************************************/
 bool can_client_issue_orders(void)
 {
-  return (!client_is_observer()
+  return (game.player_ptr
+         && !client_is_observer()
          && get_client_state() == CLIENT_GAME_RUNNING_STATE);
 }
 
@@ -740,6 +741,7 @@ bool can_intel_with_player(const struct 
 **************************************************************************/
 bool can_client_change_view(void)
 {
-  return (get_client_state() == CLIENT_GAME_RUNNING_STATE
-         || get_client_state() == CLIENT_GAME_OVER_STATE);
+  return (game.player_ptr
+         && (get_client_state() == CLIENT_GAME_RUNNING_STATE
+             || get_client_state() == CLIENT_GAME_OVER_STATE));
 }
Index: client/control.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.c,v
retrieving revision 1.183
diff -p -u -r1.183 control.c
--- client/control.c    21 Jul 2005 08:07:18 -0000      1.183
+++ client/control.c    24 Aug 2005 17:46:02 -0000
@@ -227,6 +227,9 @@ at the end of the goto, then they are st
 **************************************************************************/
 void update_unit_focus(void)
 {
+  if (!can_client_change_view()) {
+    return;
+  }
   if (!punit_focus
       || (punit_focus->activity != ACTIVITY_IDLE
          && !unit_has_orders(punit_focus)
@@ -258,6 +261,8 @@ void advance_unit_focus(void)
   struct unit *punit_old_focus = punit_focus;
   struct unit *candidate = find_best_focus_candidate(FALSE);
 
+  assert(can_client_change_view());
+
   set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
   if (!can_client_change_view()) {
     return;
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.242
diff -p -u -r1.242 mapview_common.c
--- client/mapview_common.c     1 Aug 2005 23:09:35 -0000       1.242
+++ client/mapview_common.c     24 Aug 2005 17:46:03 -0000
@@ -1967,8 +1967,10 @@ static void queue_add_callback(void)
 **************************************************************************/
 void queue_mapview_update(enum update_type update)
 {
-  needed_updates |= update;
-  queue_add_callback();
+  if (can_client_change_view()) {
+    needed_updates |= update;
+    queue_add_callback();
+  }
 }
 
 /**************************************************************************
@@ -1981,11 +1983,13 @@ void queue_mapview_update(enum update_ty
 void queue_mapview_tile_update(struct tile *ptile,
                               enum tile_update_type type)
 {
-  if (!tile_updates[type]) {
-    tile_updates[type] = tile_list_new();
+  if (can_client_change_view()) {
+    if (!tile_updates[type]) {
+      tile_updates[type] = tile_list_new();
+    }
+    tile_list_append(tile_updates[type], ptile);
+    queue_add_callback();
   }
-  tile_list_append(tile_updates[type], ptile);
-  queue_add_callback();
 }
 
 /**************************************************************************
Index: client/messagewin_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/messagewin_common.c,v
retrieving revision 1.23
diff -p -u -r1.23 messagewin_common.c
--- client/messagewin_common.c  5 May 2005 18:32:46 -0000       1.23
+++ client/messagewin_common.c  24 Aug 2005 17:46:03 -0000
@@ -26,9 +26,10 @@
 #include "citydlg_g.h"
 #include "mapview_g.h"
 #include "messagewin_g.h"
-#include "options.h"
 
+#include "civclient.h"
 #include "messagewin_common.h"
+#include "options.h"
 
 static int frozen_level = 0;
 static bool change = FALSE;
@@ -70,7 +71,7 @@ void meswin_force_thaw(void)
 **************************************************************************/
 void update_meswin_dialog(void)
 {
-  if (frozen_level > 0 || !change) {
+  if (frozen_level > 0 || !change || !can_client_change_view()) {
     return;
   }
 
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.542
diff -p -u -r1.542 packhand.c
--- client/packhand.c   18 Aug 2005 06:44:27 -0000      1.542
+++ client/packhand.c   24 Aug 2005 17:46:04 -0000
@@ -812,7 +812,7 @@ void handle_start_phase(int phase)
 {
   game.info.phase = phase;
 
-  if (is_player_phase(game.player_ptr, phase)) {
+  if (game.player_ptr && is_player_phase(game.player_ptr, phase)) {
     /* HACK: this is updated by the player packet too; we update it here
      * so the turn done button state will be set properly. */
     game.player_ptr->phase_done = FALSE;
@@ -1344,9 +1344,7 @@ void handle_game_info(struct packet_game
 
   game.government_when_anarchy
     = get_government(game.info.government_when_anarchy_id);
-  if (!can_client_change_view()) {
-    game.player_ptr = &game.players[game.info.player_idx];
-  }
+  game.player_ptr = get_player(game.info.player_idx);
   if (get_client_state() == CLIENT_PRE_GAME_STATE) {
     popdown_races_dialog();
   }
@@ -1559,10 +1557,12 @@ void handle_player_info(struct packet_pl
 
   sz_strlcpy(pplayer->username, pinfo->username);
 
-  /* Just about any changes above require an update to the intelligence
-   * dialog. */
-  update_intel_dialog(pplayer);
-  update_conn_list_dialog();
+  if (can_client_change_view()) {
+    /* Just about any changes above require an update to the intelligence
+     * dialog. */
+    update_intel_dialog(pplayer);
+    update_conn_list_dialog();
+  }
 }
 
 /**************************************************************************
Index: client/text.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/text.c,v
retrieving revision 1.48
diff -p -u -r1.48 text.c
--- client/text.c       22 Aug 2005 17:57:20 -0000      1.48
+++ client/text.c       24 Aug 2005 17:46:04 -0000
@@ -506,16 +506,20 @@ const char *get_info_label_text(void)
 
   astr_clear(&str);
 
-  astr_add_line(&str, _("Population: %s"),
-               population_to_text(civ_population(game.player_ptr)));
+  if (game.player_ptr) {
+    astr_add_line(&str, _("Population: %s"),
+                 population_to_text(civ_population(game.player_ptr)));
+  }
   astr_add_line(&str, _("Year: %s (T%d)"),
                textyear(game.info.year), game.info.turn);
-  astr_add_line(&str, _("Gold: %d (%+d)"), game.player_ptr->economic.gold,
-               player_get_expected_income(game.player_ptr));
-  astr_add_line(&str, _("Tax: %d Lux: %d Sci: %d"),
-               game.player_ptr->economic.tax,
-               game.player_ptr->economic.luxury,
-               game.player_ptr->economic.science);
+  if (game.player_ptr) {
+    astr_add_line(&str, _("Gold: %d (%+d)"), game.player_ptr->economic.gold,
+                 player_get_expected_income(game.player_ptr));
+    astr_add_line(&str, _("Tax: %d Lux: %d Sci: %d"),
+                 game.player_ptr->economic.tax,
+                 game.player_ptr->economic.luxury,
+                 game.player_ptr->economic.science);
+  }
   if (!game.info.simultaneous_phases) {
     astr_add_line(&str, _("Moving: %s"), get_player(game.info.phase)->name);
   }
@@ -643,16 +647,18 @@ const char *get_bulb_tooltip(void)
 
   astr_add_line(&str, _("Shows your progress in "
                        "researching the current technology."));
-  if (get_player_research(game.player_ptr)->researching == A_UNSET) {
-    astr_add_line(&str, _("no research target."));
-  } else {
-    /* TRANS: <tech>: <amount>/<total bulbs> */
-    struct player_research *research = get_player_research(game.player_ptr);
+  if (game.player_ptr) {
+    if (get_player_research(game.player_ptr)->researching == A_UNSET) {
+      astr_add_line(&str, _("no research target."));
+    } else {
+      /* TRANS: <tech>: <amount>/<total bulbs> */
+      struct player_research *research = get_player_research(game.player_ptr);
 
-    astr_add_line(&str, _("%s: %d/%d."),
-                 get_tech_name(game.player_ptr, research->researching),
-                 research->bulbs_researched,
-                 total_bulbs_required(game.player_ptr));
+      astr_add_line(&str, _("%s: %d/%d."),
+                   get_tech_name(game.player_ptr, research->researching),
+                   research->bulbs_researched,
+                   total_bulbs_required(game.player_ptr));
+    }
   }
   return str.str;
 }
@@ -705,8 +711,10 @@ const char *get_government_tooltip(void)
 
   astr_clear(&str);
 
-  astr_add(&str, _("Shows your current government:\n%s."),
-      get_government_name(game.player_ptr->government));
+  astr_add_line(&str, _("Shows your current government:"));
+  if (game.player_ptr) {
+    astr_add_line(&str, get_government_name(game.player_ptr->government));
+  }
   return str.str;
 }
 
Index: client/gui-gtk-2.0/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/mapview.c,v
retrieving revision 1.176
diff -p -u -r1.176 mapview.c
--- client/gui-gtk-2.0/mapview.c        22 May 2005 18:12:53 -0000      1.176
+++ client/gui-gtk-2.0/mapview.c        24 Aug 2005 17:46:04 -0000
@@ -99,12 +99,15 @@ void update_timeout_label(void)
 **************************************************************************/
 void update_info_label( void )
 {
-  int  d;
   GtkWidget *label;
 
   label = gtk_frame_get_label_widget(GTK_FRAME(main_frame_civ_name));
-  gtk_label_set_text(GTK_LABEL(label),
-                    get_nation_name(game.player_ptr->nation));
+  if (game.player_ptr) {
+    gtk_label_set_text(GTK_LABEL(label),
+                      get_nation_name(game.player_ptr->nation));
+  } else {
+    gtk_label_set_text(GTK_LABEL(label), "-");
+  }
 
   gtk_label_set_text(GTK_LABEL(main_label_info), get_info_label_text());
 
@@ -113,27 +116,30 @@ void update_info_label( void )
                      client_cooling_sprite(),
                      client_government_sprite());
 
-  d=0;
-  for (; d < game.player_ptr->economic.luxury /10; d++) {
-    struct sprite *sprite = get_tax_sprite(tileset, O_LUXURY);
+  if (game.player_ptr) {
+    int d = 0;
 
-    gtk_image_set_from_pixbuf(GTK_IMAGE(econ_label[d]),
-                             sprite_get_pixbuf(sprite));
-  }
+    for (; d < game.player_ptr->economic.luxury /10; d++) {
+      struct sprite *sprite = get_tax_sprite(tileset, O_LUXURY);
+
+      gtk_image_set_from_pixbuf(GTK_IMAGE(econ_label[d]),
+                               sprite_get_pixbuf(sprite));
+    }
  
-  for (; d < (game.player_ptr->economic.science
-            + game.player_ptr->economic.luxury) / 10; d++) {
-    struct sprite *sprite = get_tax_sprite(tileset, O_SCIENCE);
+    for (; d < (game.player_ptr->economic.science
+               + game.player_ptr->economic.luxury) / 10; d++) {
+      struct sprite *sprite = get_tax_sprite(tileset, O_SCIENCE);
 
-    gtk_image_set_from_pixbuf(GTK_IMAGE(econ_label[d]),
-                             sprite_get_pixbuf(sprite));
-  }
+      gtk_image_set_from_pixbuf(GTK_IMAGE(econ_label[d]),
+                               sprite_get_pixbuf(sprite));
+    }
  
-  for (; d < 10; d++) {
-    struct sprite *sprite = get_tax_sprite(tileset, O_GOLD);
+    for (; d < 10; d++) {
+      struct sprite *sprite = get_tax_sprite(tileset, O_GOLD);
 
-    gtk_image_set_from_pixbuf(GTK_IMAGE(econ_label[d]),
-                             sprite_get_pixbuf(sprite));
+      gtk_image_set_from_pixbuf(GTK_IMAGE(econ_label[d]),
+                               sprite_get_pixbuf(sprite));
+    }
   }
  
   update_timeout_label();