View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0000788LDMud 3.3Networkingpublic2011-10-09 11:282018-01-30 03:59
Reporter_xtian_ Assigned Tozesstra  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Fixed in Version3.3.721 
Summary0000788: long telnet neg messages are cut off
DescriptionIn comm.c::telnet_neg() long telnet negotiations are cut off after 2048 (minus overhead) chars. The assumption here is that telnet negs should simply not be that long.

The assumption is obviously false. For one there are no limits in the standard and then there are important use-cases where longer texts need to be transmitted over telnet subneg channels.

See:
ATCP composer (sending entire texts over telnet negs): http://www.mudstandards.org/ATCP_composer

See:
As an example Mushclient allowing telnet negs to be any length: http://www.gammon.com.au/scripts/showrelnote.php?version=4.48&productid=0

I am marking this as major bug, since it hinders all ldmud-running MUDs to fully adapt ATCP or GMCP features.
TagsNo tags attached.

Activities

zesstra

2011-10-09 20:27

administrator   ~0002062

Well... The limit of 2048 is not just for telnet negs. It is also the size of a users text buffer for incoming data (network receive buffer) and thus the maximum length for every user command.
You might increase them, it is a compile-time constant in comm.h (MAX_TXT).

There are various places where buffers with this length are allocated on the stack. That basically means that we can't easily make them a) runtime-configurable or b) arbitrary length.
But if somebody wants to work on that... (But it might be more reasonable (see below) to define a protocol which can negotiate the length of sub negotiation messages).

Side remark: I am pretty sure, there are a lot telnet implementations which have such limits (I know of some) despite the standard not defining one...

zesstra

2011-10-09 20:31

administrator   ~0002063

BTW: one might argue, that a) the standard did not have in mind to transfer huge chunks of data via the telnet option subnegotation (since it is about negotiating telnet options/behaviour) and b) since the standard does not define a minimum length any length the implementation chooses, fulfills the standard.

zesstra

2011-10-10 13:37

administrator   ~0002064

BTW: Why does the composer package anyway send messages incompatible to the ATCP message specification from http://www.mudstandards.org/ATCP_Message_Format?
"Messages going to the server must be less than 2048 bytes in length."

(The 2048 are IMHO still a curious convention: the underlying network buffers are likely to be a power of 2 in size, but you will have some overhead, i.e. the ATCP messages should be slightly smaller than a power of two.)

_xtian_

2011-10-16 11:45

reporter   ~0002065

I checked and the ATCP link (or specification) you quoted is out of date or they just ignored that constraint later on. Fact is, clients and servers of muds implementing this are both able to send longer texts.

I understand you don't believe in the rationale behind these newer protocols. But ATCP (and GMCP in minor extend) are gaining quite a following and a lot of servers are adding it. It is a step toward enhancing game-experience for players as it allows construction of GUIs and so on ... Personally, I am glad, the community is embracing this and starting to think more about usability of their games. So please don't dismiss ATCP/GMCP related issues referring to other implementations that also don't have those features implemented or speculate about other ways how things may have been more to your liking. These protocols are here, in heavy use (at least in the english speaking community) and we should not lock ourselves out of that.

Having argued, why I think this is important, on a more technical note:
1) the long telnet neg message is not just cut off, as my title suggested, but alltogether discarded. The rest of the long telnet neg is displayed in the normal text stream.

2) a solution that just concats to a LPC string that will be passed to the telnet_neg-hook of the mudlib would be enough, as mudlibs will have to self implement the ATCP stuff themselves anyway.

Gnomi

2011-10-16 13:25

manager   ~0002066

There must be some constraint of the length of commands or telnet negotiations, otherwise you could implement DOS attacks by causing an out-of-memory condition. So what will the new limit be?

_xtian_

2011-10-17 05:49

reporter   ~0002067

I am told IRE muds cut off composer messages at ~20k text.

Gnomi

2011-10-17 09:20

manager   ~0002068

So, would setting MAX_TEXT in comm.h to 20480 solve this problem?

_xtian_

2011-10-26 16:49

reporter   ~0002069

@gnomi: Would probably solve my problem, yes. But I'm not sure if that buffer Define is not used elsewhere.

_xtian_

2011-11-01 16:32

reporter   ~0002074

I'm fieldtesting a MAX_TEXT of 20480 atm. Will keep you updated.

zesstra

2012-12-05 19:29

administrator   ~0002166

BTW: How did these tests work? ;-)

_xtian_

2012-12-06 06:49

reporter   ~0002167

I observed no problems in the past year.
Allow me to suggest making the higher value the new standard.

zesstra

2012-12-06 19:10

administrator   ~0002168

Fix committed in revision f8edc687ea7e6841fa19325be0d287affcd48258 to master branch (see changeset 878 for details). Thank you for reporting!

zesstra

2012-12-06 19:12

administrator   ~0002169

Fix committed in revision 4c5e08509dea53cb14ec02d83b16aa9fa30be434 to master-3.3 branch (see changeset 879 for details). Thank you for reporting!

zesstra

2018-01-29 18:59

administrator   ~0002322

Fix committed in revision 4c5e08509dea53cb14ec02d83b16aa9fa30be434 to master-3.3 branch (see changeset 2202 for details). Thank you for reporting!

zesstra

2018-01-29 21:57

administrator   ~0002373

Fix committed in revision 4c5e08509dea53cb14ec02d83b16aa9fa30be434 to master-3.3 branch (see changeset 3546 for details). Thank you for reporting!

zesstra

2018-01-30 03:59

administrator   ~0002424

Fix committed in revision f8edc687ea7e6841fa19325be0d287affcd48258 to master branch (see changeset 3926 for details). Thank you for reporting!

Issue History

Date Modified Username Field Change
2011-10-09 11:28 _xtian_ New Issue
2011-10-09 20:27 zesstra Note Added: 0002062
2011-10-09 20:31 zesstra Note Added: 0002063
2011-10-09 20:49 zesstra Project LDMud => LDMud 3.5
2011-10-10 13:37 zesstra Note Added: 0002064
2011-10-16 11:45 _xtian_ Note Added: 0002065
2011-10-16 13:25 Gnomi Note Added: 0002066
2011-10-17 05:49 _xtian_ Note Added: 0002067
2011-10-17 09:20 Gnomi Note Added: 0002068
2011-10-26 16:49 _xtian_ Note Added: 0002069
2011-11-01 16:32 _xtian_ Note Added: 0002074
2012-12-05 19:29 zesstra Note Added: 0002166
2012-12-06 06:49 _xtian_ Note Added: 0002167
2012-12-06 19:04 zesstra Assigned To => zesstra
2012-12-06 19:04 zesstra Status new => assigned
2012-12-06 19:10 zesstra Source_changeset_attached => ldmud.git master f8edc687
2012-12-06 19:10 zesstra Note Added: 0002168
2012-12-06 19:10 zesstra Status assigned => resolved
2012-12-06 19:10 zesstra Resolution open => fixed
2012-12-06 19:12 zesstra Source_changeset_attached => ldmud.git master-3.3 4c5e0850
2012-12-06 19:12 zesstra Note Added: 0002169
2015-01-23 20:36 zesstra Project LDMud 3.5 => LDMud 3.3
2015-01-23 20:38 zesstra Fixed in Version => 3.3.721
2018-01-29 18:59 zesstra Source_changeset_attached => ldmud.git master f8edc687
2018-01-29 18:59 zesstra Source_changeset_attached => ldmud.git master-3.3 4c5e0850
2018-01-29 18:59 zesstra Note Added: 0002322
2018-01-29 21:57 zesstra Source_changeset_attached => ldmud.git master f8edc687
2018-01-29 21:57 zesstra Source_changeset_attached => ldmud.git master-3.3 4c5e0850
2018-01-29 21:57 zesstra Note Added: 0002373
2018-01-30 03:59 zesstra Source_changeset_attached => ldmud.git master f8edc687
2018-01-30 03:59 zesstra Note Added: 0002424