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 15 Jul 2012 10:50:02 AM UTC  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: pepeto <pepeto>Open/Closed: Closed
Release: trunk r21550Operating System: Any
Planned Release: 2.5.0

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 23 Feb 2013 11:17:03 AM UTC, 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 22 Feb 2013 08:41:20 AM UTC, 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 20 Feb 2013 10:37:51 AM UTC, 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 18 Feb 2013 11:25:07 AM UTC, 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 16 Feb 2013 06:02:16 PM UTC, 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 10 Feb 2013 03:25:41 PM UTC, comment #9:

Fix attached. It needs to apply patch #3690.

(file #17179)

pepeto <pepeto>
Project MemberIn charge of this item.
Fri 08 Feb 2013 04:44:42 PM UTC, 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 08 Feb 2013 04:09:36 PM UTC, 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 08 Feb 2013 02:36:00 PM UTC, 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 08 Feb 2013 02:02:56 PM UTC, comment #5:

See also bug #20486

Marko Lindqvist <cazfi>
Project Administrator
Thu 30 Aug 2012 03:04:11 PM UTC, 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 15 Jul 2012 12:03:32 PM UTC, 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 15 Jul 2012 11:08:17 AM UTC, 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 15 Jul 2012 11:06:02 AM UTC, 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 15 Jul 2012 10:50:02 AM UTC, 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.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 22 Feb 2013 08:41:36 AM UTCpepetoStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Wed 20 Feb 2013 10:37:51 AM UTCpepetoAttached File-=>Added 0001-Make-initial-network-protocol-compatible-with-old-ve.patch, #17276
      Assigned toNone=>pepeto
      Operating SystemNone=>Any
    Mon 18 Feb 2013 11:25:07 AM UTCpepetoAttached File-=>Added 0005-Make-initial-network-protocol-compatible-with-old-ve.patch, #17247
    Sat 16 Feb 2013 06:02:16 PM UTCpepetoAttached File-=>Added 0005-Make-initial-network-protocol-compatible-with-old-ve.patch, #17222
      CategoryNone=>general
    Sun 10 Feb 2013 03:25:41 PM UTCpepetoAttached File-=>Added trunk_network_protocol.diff, #17179
      StatusNone=>Ready For Test
    Sat 21 Jul 2012 10:57:06 AM UTCjtnSummaryInitial 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