[Freeciv-Dev] [bug #21300] Network compression can send packet which receiver interprets incorrectly
URL: http://gna.org/bugs/?21300 Summary: Network compression can send packet which receiver interprets incorrectly Project: Freeciv Submitted by: jtn Submitted on: Sun Nov 24 14:41:49 2013 Category: None Severity: 3 - Normal Priority: 5 - Normal Status: In Progress Assigned to: jtn Originator Email: Open/Closed: Open Release: 2.3.4,2.4.0,2.5.0,2.6.0 Discussion Lock: Any Operating System: Any Planned Release: 2.3.5,2.4.1,2.5.0,2.6.0 ___ Details: If, by bad luck, an outgoing compressed lump of data ends up being *exactly* 49148 octets, then the network compression (USE_COMPRESSION) decides to send it with normal rather than jumbo encoding due to its test of JUMBO_BORDER. Unfortunately, the encoded length ends up being 0x, which the receiver interprets as being a jumbo packet (JUMBO_SIZE). It then tries to interpret the next four octets (which are in fact the start of the compressed data) as a length field. You'd have to be quite unlucky to hit this, but if you did then the network connection would likely never recover. With things as they stand this is most likely to show up when a client connects. I believe I've reproduced this by hacking, but it's too tangled up with other changes to present here. The client eventually barfed with can't grow buffer. Fix should be trivial: in conn_compression_flush() on the sender, change compressed_size = JUMBO_BORDER to . No change is needed on the receiver. ___ Reply to this item at: http://gna.org/bugs/?21300 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21300] Network compression can send packet which receiver interprets incorrectly
Follow-up Comment #1, bug #21300 (project freeciv): A related problem is that compressed data of size 49149 or 49150 would I think overflow the packet length field, causing a Trying to put %d into 16 bits warning on recent branches, and different comedy on the receiver (on any branch). This is due to the jumbo decision not taking into account the length field. ___ Reply to this item at: http://gna.org/bugs/?21300 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21301] Possible network stall with short packets at start of connection
URL:
http://gna.org/bugs/?21301
Summary: Possible network stall with short packets at start
of connection
Project: Freeciv
Submitted by: jtn
Submitted on: Sun Nov 24 15:51:14 2013
Category: None
Severity: 3 - Normal
Priority: 5 - Normal
Status: In Progress
Assigned to: jtn
Originator Email:
Open/Closed: Open
Release:
Discussion Lock: Any
Operating System: Any
Planned Release: 2.5.0,2.6.0
___
Details:
Since patch #2789 (16-bit packet type), get_packet_from_connection() has
insisted on at least four bytes (length + type) before proceeding.
if (pc-buffer-ndata 2+2) {
/* length and type not read */
return NULL;
}
However, as of bug #19943, 3-byte packets (those with no payload) are still
valid at the start of a connection. So, this could in theory cause a stall, if
there's any case where the sender sends a short packet and waits for a
response.
I've not seen or heard of such a stall in practice, but we may as well fix it.
(Probably becomes more important with patch #4274.)
___
Reply to this item at:
http://gna.org/bugs/?21301
___
Message sent via/by Gna!
http://gna.org/
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4313] Diagnose too many calls to conn_compression_thaw()
URL: http://gna.org/patch/?4313 Summary: Diagnose too many calls to conn_compression_thaw() Project: Freeciv Submitted by: jtn Submitted on: Sun Nov 24 15:52:39 2013 Category: None Priority: 5 - Normal Status: In Progress Privacy: Public Assigned to: jtn Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: 2.5.0,2.6.0 ___ Details: conn_compression_freeze()/thaw() follow the usual reference counting pattern, but _thaw() lacks the usual sanity check that the count hasn't gone negative. ___ Reply to this item at: http://gna.org/patch/?4313 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4314] Network compression for big map updates
URL: http://gna.org/patch/?4314 Summary: Network compression for big map updates Project: Freeciv Submitted by: jtn Submitted on: Sun Nov 24 15:56:42 2013 Category: None Priority: 5 - Normal Status: In Progress Privacy: Public Assigned to: jtn Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: 2.4.2,2.5.0,2.6.0 ___ Details: Following on from discussion in bug #18532, it seems like a good idea to enable compression when sending big updates to the map, such as when revealing it at game end. buffer_shared_vision() seems like a good place to do this (it already enables network buffering). This catches all sorts of other situations which are likely to benefit from compression, such as giving maps and shared vision by diplomacy. Is there any reason why enabling network compression for these periods would be a bad idea? It worked fine in my testing. ___ Reply to this item at: http://gna.org/patch/?4314 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4314] Network compression for big map updates
Follow-up Comment #1, patch #4314 (project freeciv): (I do intend to commit this to S2_4, but after 2.4.1 so that we get time to test it.) ___ Reply to this item at: http://gna.org/patch/?4314 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4315] Avoid sending compressed packet that would be bigger than uncompressed one
URL: http://gna.org/patch/?4315 Summary: Avoid sending compressed packet that would be bigger than uncompressed one Project: Freeciv Submitted by: jtn Submitted on: Sun Nov 24 15:58:52 2013 Category: None Priority: 3 - Low Status: In Progress Privacy: Public Assigned to: jtn Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: 2.5.0,2.6.0 ___ Details: Since I've been crawling all over the network compression code, may as well fix the issue I noted at the end of bug #21297, even though it's harmless and unlikely. ___ Reply to this item at: http://gna.org/patch/?4315 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4316] Add assertions for invariants and assumptions of network compression code
URL: http://gna.org/patch/?4316 Summary: Add assertions for invariants and assumptions of network compression code Project: Freeciv Submitted by: jtn Submitted on: Sun Nov 24 16:00:33 2013 Category: None Priority: 5 - Normal Status: In Progress Privacy: Public Assigned to: jtn Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: 2.5.0,2.6.0 ___ Details: Since I've been trying to understand how this code works, add some documentation through assertions (static and runtime). Note, this needs the fix for bug #21300 (it's how I spotted that issue). ___ Reply to this item at: http://gna.org/patch/?4316 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4313] Diagnose too many calls to conn_compression_thaw()
Update of patch #4313 (project freeciv): Planned Release: 2.5.0,2.6.0 = 2.4.2,2.5.0,2.6.0 ___ Reply to this item at: http://gna.org/patch/?4313 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21297] Network compression could cause network buffer overflow and cut connection
Update of bug #21297 (project freeciv): Status:None = In Progress Assigned to:None = jtn Planned Release: = 2.4.2,2.5.0,2.6.0 ___ Reply to this item at: http://gna.org/bugs/?21297 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21297] Network compression could cause network buffer overflow and cut connection
Follow-up Comment #3, bug #21297 (project freeciv): I was able to trigger this on Ubuntu with FREECIV_COMPRESSION_LEVEL=0, even when taking extra care to not emit more than MAX_LEN_BUFFER. Reducing the queue max to MAX_LEN_BUFFER/2 or even /4 doesn't seriously reduce the level of compression achieved. I propose /2. ___ Reply to this item at: http://gna.org/bugs/?21297 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21300] Network compression can send packet which receiver interprets incorrectly
Update of bug #21300 (project freeciv): Status: In Progress = Ready For Test ___ Additional Item Attachment: File name: trunk-S2_5-S2_4-S2_3-net-compress-jumbo.patch Size:1 KB ___ Reply to this item at: http://gna.org/bugs/?21300 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4313] Diagnose too many calls to conn_compression_thaw()
Update of patch #4313 (project freeciv): Status: In Progress = Ready For Test ___ Additional Item Attachment: File name: trunk-S2_5-S2_4-net-compress-thaw-assertion.patch Size:0 KB ___ Reply to this item at: http://gna.org/patch/?4313 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21297] Network compression could cause network buffer overflow and cut connection
Update of bug #21297 (project freeciv): Status: In Progress = Ready For Test ___ Follow-up Comment #4: (Also noticed in passing that conn_compression_flush() can send a compressed packet that's slightly bigger than the uncompressed one would be, if it goes to jumbo encoding. Hardly matters, but could fix in passing.) This is now patch #4315. (file #19374) ___ Additional Item Attachment: File name: trunk-S2_5-S2_4-net-compress-overflow.patch Size:2 KB ___ Reply to this item at: http://gna.org/bugs/?21297 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4314] Network compression for big map updates
Update of patch #4314 (project freeciv): Status: In Progress = Ready For Test ___ Follow-up Comment #2: As noted elsewhere, a big map revealed in its entirely is reduced to 20% of the previous network traffic. User now sees it coming through in big lumps rather than individual tiles. (file #19375) ___ Additional Item Attachment: File name: trunk-S2_5-S2_4-net-compress-map-updates.patch Size:1 KB ___ Reply to this item at: http://gna.org/patch/?4314 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21301] Possible network stall with short packets at start of connection
Update of bug #21301 (project freeciv): Status: In Progress = Ready For Test ___ Additional Item Attachment: File name: trunk-S2_5-net-accept-small-packets.patch Size:0 KB ___ Reply to this item at: http://gna.org/bugs/?21301 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4315] Avoid sending compressed packet that would be bigger than uncompressed one
Update of patch #4315 (project freeciv): Status: In Progress = Ready For Test ___ Reply to this item at: http://gna.org/patch/?4315 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #4316] Add assertions for invariants and assumptions of network compression code
Update of patch #4316 (project freeciv): Status: In Progress = Ready For Test ___ Additional Item Attachment: File name: trunk-S2_5-net-compress-asserts.patch Size:2 KB ___ Reply to this item at: http://gna.org/patch/?4316 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #18532] Cannot gracefully /end a game on a big map
Follow-up Comment #9, bug #18532 (project freeciv): when the map is revealed [...] there's no attempt to enable network buffering or compression for the large amount of data that is likely to be sent. Perhaps adding some would help. OK, there is buffering via buffer_shared_vision(). Compression is now enabled by patch #4314. ___ Reply to this item at: http://gna.org/bugs/?18532 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21272] freeciv-gtk2.exe caused an Access Violation while looking inside city
Follow-up Comment #3, bug #21272 (project freeciv): Just had my client crash for a fourth time with this bug. I was looking into a team mates city again. I was able to look into that city both before the crash and again after I logged back on. Nothing had changed to the city in the meantime. This is a longturn game, so yes the savegame is not possible. ___ Reply to this item at: http://gna.org/bugs/?21272 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
