[Freeciv-Dev] Server side treaty checking & segfault bug 'fix'.

[Marko Lindqvist <caz@xxxxxxxxx>]

 Adds some checks for treaties to server side.
  - Don't accept tech clauses with illegal tech value at all.
  - Player can't give away techs (s)he doesn't have.
  - Player can't give away capital.
  - Player can't give away cities (s)he doesn't own.

 Also avoids one segfault (bug still exists in less fatal form)



 This patch takes parts from my earlier patch that makes server not to 
sent information about capital unless receiver have embassy. I'll later
redo that patch to apply on top of this one.

 I check most clauses only when player tries to accept treaty instead of
time clause is first time proposed. This may seem like not very
user-friendly way of doing it. However, these checks should almost never
fail unless either side have modified client (which allows setting illegal
clauses in). Advantage of checking clauses only when accepting them is
that in the future it's possible to make meeting not to reveal anything
(s)he shouldn't know to other player.

  - Player can't give away techs (s)he doesn't have.
  - Player can't give away capital.

  These were completely unchecked at server side.

  - Player can't give away cities (s)he doesn't own.

  This particular clause was ignored, but treaty was otherwise processed.
This could lead to situation where you pay for city, but get nothing in
return. Note that this is possible situation even with non-modified
clients. Player may lose city between inserting clause and accepting
treaty. (Just realised that this probably led to segfault at some
point also, so player would have been saved from making bad bargain.)


 To reproduce segfault:

 1. Player A have seen city of player B.
 2. While city is under player A's fog of war, player C conquers it.
 3. Players A and B have meeting.
 4. Player A inserts clause "Player B gives city to player A"

  Segfault is caused by the fact that players know city by different id.
Player A still knows city by id it had when (s)he saw it, Player B knows
right (or at least newer) id.
  I did not introduce any real fix to this yet, since most
(quick) approaches I thought about had clear disadvantages also. I'm going
towork with treaty stuff a bit longer and some good fix probably comes up
quite naturally. This patch just avoids segfault by checking that
client knows city with given id (good thing to do anyway) and if not,
requests user to erase illegal clause.

  What is the reason to change city id while it changes owner? I can see
it leading to problems in many situations especially with fog of war.
Though it won't remove this bug completely, keeping old id would make
situation much easier.


 Caz

--

treaty_checks.diff
Description: Text document