bugFreeciv - Bugs: bug #19943, Initial protocol between trunk and...

 
 
Show feedback again

bug #19943: Initial protocol between trunk and S2_4-or-earlier broken

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sun Jul 15 10:50:02 2012  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: pepeto <pepeto>Open/Closed: Closed
Release: trunk r21550Operating System: Any
Planned Release: 2.5.0Contains string changes: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sat Feb 23 11:17:03 2013, comment #14:

Thank you for doing this. It was bothering me (but not enough to actually do anything). It works fine in my (simple) testing.

Jacob Nevins <jtn>
Project Administrator
Fri Feb 22 08:41:20 2013, SVN revision 22421:

Keep login network protocol compatible with former versions.

Packets headers contain 3 bytes in the initial protocol (2 for the length,
1 for the type), and 4 bytes after the SERVER_JOIN_REPLY packet has been
sent or received (2 for the length, 2 for the type).

These values are defined in "common/packets.c", respectively in functions
packet_header_init() and packet_header_set().

Label packets in "common/packets.def" which are used in the initial protocol
(their number must be in range 0-255). Update comments and documentation.

Do not send ping when accepting a new connection, because we would be unable
to handle packets with different header sizes asynchronously.

See gna bug #19943

(Browse SVN revision 22421)

pepeto <pepeto>
Project MemberIn charge of this item.
Wed Feb 20 10:37:51 2013, comment #12:

New version of the patch:

  • also label PACKET_CONNECT_MSG.

Would someone wish to test over patch #3690?

(file #17276)

pepeto <pepeto>
Project MemberIn charge of this item.
Mon Feb 18 11:25:07 2013, comment #11:

New version of the patch attached:

  • move all the process into packets.c, using post-send and post-recv routines ;
  • more comments.

(file #17247)

pepeto <pepeto>
Project MemberIn charge of this item.
Sat Feb 16 18:02:16 2013, comment #10:

New version of the patch attached:

  • some variables renamed ;
  • also modify doc/HACKING ;
  • ensure to don't receive the PACKET_SERVER_JOIN_REQ twice ;
  • the PACKET_SERVER_JOIN_REQ must be sent in appropriate time (using ping timeout) after connection is accepted, to avoid mute connections.

(file #17222)

pepeto <pepeto>
Project MemberIn charge of this item.
Sun Feb 10 15:25:41 2013, comment #9:

Fix attached. It needs to apply patch #3690.

(file #17179)

pepeto <pepeto>
Project MemberIn charge of this item.
Fri Feb 8 16:44:42 2013, comment #8:

> Also note that it's much more likely that user of recent, but
> not latest, trunk checkout try to connect to latest server. So
> you shouldn't be breaking compatibility with current 16bit
> versions to get compatibility with 1.5 years old 8bit versions.


trunk isn't a stable version, there is no release neither. So there is no reason to keep trunk 1.5 years old compatible with next code.

pepeto <pepeto>
Project MemberIn charge of this item.
Fri Feb 8 16:09:36 2013, comment #7:

Just make sure that either the support can be later dropped without compatibility break at that point, or that code is not too complicated. I don't want to maintain infinitely complicated code for minimal gain - giving user better error message in the rare case they they try to connect pre-2.5 client to newer server. Capabilities are not matching in any case, so the connection will be closed anyway.
Also note that it's much more likely that user of recent, but not latest, trunk checkout try to connect to latest server. So you shouldn't be breaking compatibility with current 16bit versions to get compatibility with 1.5 years old 8bit versions.

Marko Lindqvist <cazfi>
Project Administrator
Fri Feb 8 14:36:00 2013, comment #6:

I will work on it, when I will finish with the get_dio_xxx() stuff.

My idea is simple : packets can be sent and received with 8 bits for the type before the connection is totally completed (pconn->established == TRUE), and 16 bits after that...

pepeto <pepeto>
Project MemberIn charge of this item.
Fri Feb 8 14:02:56 2013, comment #5:

See also bug #20486

Marko Lindqvist <cazfi>
Project Administrator
Thu Aug 30 15:04:11 2012, comment #4:

Just for grins, i booted my old PowerMac into Debian and built freeciv: i got the 'client.conn.client.request_id_of_currently_handled_packet == 0' assertion for fresh installs of TRUNK and 2.4.0-beta1. The version string of the server seemed to match that of the client in both cases!

david@debianG5:~$ uname -a
Linux debianG5 2.6.32-5-powerpc64 #1 SMP Sun May 6 05:10:56 UTC 2012 ppc64 GNU/Linux
david@debianG5:~$ gcc --version
gcc (Debian 4.4.5-8) 4.4.5

David Lowe <doctorjlowe>
Sun Jul 15 12:03:32 2012, comment #3:

> Do we want to do something like have the initial exchange use
> 8-bit fields? Or will we live with the mutual unintelligibility?


I'd have clean cut now to 16bit fields. That's why the change was timed to beginning of entire release cycle.

Changing this now would break compatibility with TRUNK between S2_4 branching and now.

Best, if one that requires some work, way to do this would be to make TRUNK to somehow work with both 8bit and 16bit fields in initial packets, so it would be compatible with both past and future versions, and only later (when 2.4 is so old that practically nobody uses those versions with 8bit fields) move fully to 16bit fields.

Marko Lindqvist <cazfi>
Project Administrator
Sun Jul 15 11:08:17 2012, comment #2:

Ah yes, patch #2789.

Do we want to do something like have the initial exchange use 8-bit fields? Or will we live with the mutual unintelligibility?

Jacob Nevins <jtn>
Project Administrator
Sun Jul 15 11:06:02 2012, comment #1:

> Perhaps one of the aspects of the protocol that should never
> change, has?


One of the first patches that went in after S2_4 branching was 8bit -> 16bit packet type fields.

Marko Lindqvist <cazfi>
Project Administrator
Sun Jul 15 10:50:02 2012, original submission:

The behaviour when a new (trunk r21550, "2.5") server/client talks to an older (S2_4 or earlier) counterpart seems different, and less useful, to how this situation was handled in the past. I haven't looked into it much. Perhaps one of the aspects of the protocol that should never change, has?

Here's the behaviour I see:

trunk client, S2_4 server: server says:

client says:

S2_4 client, trunk server: server says:

(Interesting -- my username is "jtn", not "tn".)

client says:

Here's what I expect to happen: S2_3 client to S2_4 server: server says:

and client says:

Jacob Nevins <jtn>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by pepeto (Posted a comment)
  • -unavailable- added by doctorjlowe (Posted a comment)
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Error: not logged in

     

     

    Follow 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri Feb 22 08:41:36 2013pepetoStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Wed Feb 20 10:37:51 2013pepetoAttached File-=>Added 0001-Make-initial-network-protocol-compatible-with-old-ve.patch, #17276
      Assigned toNone=>pepeto
      Operating SystemNone=>Any
    Mon Feb 18 11:25:07 2013pepetoAttached File-=>Added 0005-Make-initial-network-protocol-compatible-with-old-ve.patch, #17247
    Sat Feb 16 18:02:16 2013pepetoAttached File-=>Added 0005-Make-initial-network-protocol-compatible-with-old-ve.patch, #17222
      CategoryNone=>general
    Sun Feb 10 15:25:41 2013pepetoAttached File-=>Added trunk_network_protocol.diff, #17179
      StatusNone=>Ready For Test
    Sat Jul 21 10:57:06 2012jtnSummaryInitial protocol between trunk and S2_4-or-earlier broken?=>Initial protocol between trunk and S2_4-or-earlier broken
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup