Allocation optimization in _set_rfc1143() is wrong.

[charlesUnixPro]

The comments in _set_rfc1143() indicate that option state slots are allocated in groups of four to reduce realloc() calls, but the code never uses the three extra slots.

The code incorrectly assumes that if the entry is not found, a relloc is needed. The "search for entry" loop should break if the telopt field is 0 (indication end of list), and the realloc should only occur if "i" is less than "q_size".

This error causes excessive calls to realloc and wasted space in telnet->q.

[seanmiddleditch]

Good catch. Would be happy to accept a pull request that fixes it.

[charlesUnixPro]

I did a fix and was reviewing it and realized that the bug may be less trivial then I thought, but I don't understand telnet protocol well enough to know.

I think the code would be indicating that TELNET_TELOPT_BINARY opts had been received (the allocator zeros q, and the search code would indicate a hit on BINARY as it is encoded as 0. I don't know what the effect of this is on the protocol.

[seanmiddleditch]

Hmm. For reference (for myself as much as anyone), https://tools.ietf.org/html/rfc856

Refreshing myself on that code (... it could use better comments, unsurprisingly), the incoming option code would need to be TELNET_TELOPT_BINARY and then the first unused entry would be interpreted as such, as would any option actually used for BINARY already. Basically, the BINARY option and none others would be able to use the chunked allocation optimization, entirely by accident.

Your changes should be safe and not result in a behavioral change.

It should be easy (and a good idea) to add a test case for BINARY mode to the tests/ directory to make sure. Shame on me for not already having one.

[seanmiddleditch]

PR has been merged.