Summary: PACKET_SERVER_JOIN_REPLY changed shape between
2.2.x and 2.3.x
                 Project: Freeciv
            Submitted by: jtn
            Submitted on: Wed Dec 15 22:46:48 2010
                Category: None
                Severity: 3 - Normal
                Priority: 5 - Normal
                  Status: None
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
         Discussion Lock: Any
        Operating System: None
         Planned Release: 



PACKET_SERVER_JOIN_REPLY (defined in common/packets.def) is supposed to stay
the same shape between different versions of Freeciv, as it's part of the
exchange where capabilities are checked, so it must stay the same in order for
capability mismatch between client and server to be adequately communicated.

Unfortunately, pepeto points out
<https://mail.gna.org/public/freeciv-dev/2010-12/msg00000.html> that the type
of the conn_id member, CONNECTION, changed from UINT8 (one octet on the wire)
to SINT16 (two octets, MSB-first) in r16375
<http://svn.gna.org/viewcvs/freeciv?view=rev&revision=16375> in patch #1363.

Based on reading the code at the head of S2_2 and S2_3 and experiment, I
think we sort of get away with it:
* conn_id is the last member of the packet, so its change in size doesn't
shuffle anything else about.
* I think that any server/client pair which disagree about the shape of the
packet will also have incompatible capability strings. Thus, the server will
respond with you_can_join==FALSE, and in this case, the client doesn't look at
conn_id (which the server has tried to set to -1). You just get error messages
about packet length mismatches.
** Old client, new server: server sends conn_id as FF FF; client sees conn_id
as FF (but doesn't look at it); client sees overlong packets (but
check_packet() only complains at log_verbose level, so only visible with "-d
** New client, old server: server sends conn_id as FF (I think); client sees
short packet, so dio_get_sint16() returns 0 for conn_id, but again client
isn't looking at conn_id; check_packet() complains non-fatally about short
packet at log_error level (in terminal)
** In both cases, the message about why the connection has failed (mismatched
capabilities) gets through OK.

Clearly we could put some sort of ugly hack in new clients to suppress the
short-packet error, if we wanted to.

On the other hand, I'm not sure why the CONNECTION type was widened in the
first place; it makes some manipulation of chat packets easier, but it wasn't
obviously done to lift a fundamental limit on the number of connections or
whatever. So, we could presumably change our minds and revert to UINT8 in
2.3.0-beta1 (or even later) with only the above, relatively cosmetic impact
with early 2.3.x servers.


Reply to this item at:


  Message sent via/by Gna!

Freeciv-dev mailing list

Reply via email to