[Freeciv-Dev] [patch #5913] json protocol: Handle multiple packets in buffer -case
Update of patch #5913 (project freeciv): Status: Ready For Test = Done Assigned to:None = cazfi Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #9, patch #5913 (project freeciv): Ok. This is a good improvement. Feel free to commit it. ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #8, patch #5913 (project freeciv): I just tested current Freeciv-web on github with this patch I have fixed some other bugs on freeciv-proxy, but I have not pushed the protocol change. It must go in at the same time for both ends of the communication. ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #7, patch #5913 (project freeciv): I just tested current Freeciv-web on github with this patch, and it didn't work, so further changes are necessary. ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #4, patch #5913 (project freeciv): I suggest you add some debugging and look at the actual packets sent between Freeciv-proxy and the Freeciv server before and after the applying patch. The patch includes some changes that were necessary for handling packets sent by the server (like in server-proxy protocol). Now you are saying that they actually break receiving packets from freeciv-web proxy (proxy-server)? Yes. With the patch, packets can't be sent from Freeciv-proxy to the Freeciv server. Sometimes there are error messages in the logs from the Freeciv server that it is unable to parse the incoming packet. Umh, the change is in receiving side only. So how could that affect sending? True. So it doesn't affect sending. ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #6, patch #5913 (project freeciv): Sure! Let's just make sure that Freeciv-web works also after this change. ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #5, patch #5913 (project freeciv): protocol is not symmetric in a sense that server-proxy has different packet structure to what proxy-server has Meaning of header 'length' field differs between these. Proxy sends just the payload size while all the other parties (freeciv raw protocol, freeciv server in case of json protocol) set full packet length. I'll fix that on freeciv-web side. I consider that complaint to be resolved now, and this patch to be ready to be committed if there's there's no further comments in 24h. ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #1, patch #5913 (project freeciv): I just tested this patch by applying it to the latest version of Freeciv-web from github. After applying the patch, no packets could be sent to or from the Freeciv server, so it is not possible start any games when the patch is applied. Please test the patch in a full Freeciv-web installation. In general I support the effort of improving connection reliability of JSON based connections, however, it needs to work with Freeciv-web as well. Also noting here that I think that embedding Libwebsockets is the long-term solution, see bug #23283. ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #2, patch #5913 (project freeciv): I don't fully undestand the patch, but it could be that the patch is correct, but that it requires some changes in Freeciv-proxy. The patch needs more testing. ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
Follow-up Comment #3, patch #5913 (project freeciv): It then seems that freeciv-web protocol is not symmetric in a sense that server-proxy has different packet structure to what proxy-server has. The patch includes some changes that were necessary for handling packets sent by the server (like in server-proxy protocol). Now you are saying that they actually break receiving packets from freeciv-web proxy (proxy-server)? I think this patch can be broken to a couple of separate changes: 1) Shifting the buffer (where previously was a no-op memmove() with the same source and dest + simply marking the buffer empty) 2) json_loadb() buffer length correction, not including raw-protocol header terminating NULL (handling of that NULL is what I suspect most) 3) Change to checking the minimum length of the buffer to be sane. That's actually an overdue change from the time we removed the unused 'type' from the packet header. The whole code-block is now actually obsolete (we've already read the 'length', no point to check if we've got it) but I didn't remove it in this patch as an unrelated change no packets could be sent to or from the Freeciv server Umh, the change is in receiving side only. So how could that affect sending? ___ Reply to this item at: http://gna.org/patch/?5913 ___ 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 #5913] json protocol: Handle multiple packets in buffer -case
URL: http://gna.org/patch/?5913 Summary: json protocol: Handle multiple packets in buffer -case Project: Freeciv Submitted by: cazfi Submitted on: Sat 07 Mar 2015 11:07:58 AM EET Category: freeciv-web Priority: 5 - Normal Status: Ready For Test Privacy: Public Assigned to: None Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: 3.0.0 ___ Details: Correttly shift and keep unhandled data in incoming buffer when handling json -packets. Previously all of the data was scrapped (not *really*, but anyway size of the buffer content was marked 0), breaking the stream when there's multiple packets in queue. It seems that this has not been an big issue with freeciv-web with proxy feeding the server relatively slowly, but trying to get regular server and client to communicate with json-protocol on the same machine never goes longer than just an packet or two without this fix. Not that this alone would yet allow one to get succesfull login to the server. ___ File Attachments: --- Date: Sat 07 Mar 2015 11:07:58 AM EET Name: JsonMultipacket.patch Size: 2kB By: cazfi http://gna.org/patch/download.php?file_id=24012 ___ Reply to this item at: http://gna.org/patch/?5913 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
