Re: [PATCH 3/3] telnet: Avoid command evaluation crashes.

[Simon Josefsson]

Erik Auerswald <auerswal@unix-ag.uni-kl.de> writes:

> Hello Simon,
>
> On Sat, Jul 09, 2022 at 02:36:41PM +0200, Simon Josefsson wrote:
>> Erik Auerswald <auerswal@unix-ag.uni-kl.de> writes:
>> > On Fri, Jul 08, 2022 at 08:55:18AM +0200, Erik Auerswald wrote:
>> >> On Fri, Jul 08, 2022 at 12:58:37AM +0200, Simon Josefsson wrote:
>> >> 
>> >> > Thanks for preparing these [...]
>> >> 
>> >> Thanks for applying my patches!
>> >> [...]
>> > The code looks good.  As intended the two genget() changes are not
>> > included, everything else is.  The NEWS entries look good, too.
>> 
>> Thank you!  I don't care strongly about the genget patches,
>
> I might separate the genget() changes out into two patches (they pertain
> to two distinct issues) with individual rationales since they pertain
> to two different possible robustness improvements.
>
>> I mostly have a general concern that we should minimize changes with
>> no externally testable difference.
>
> I agree that the genget() changes lack tests.  I am optimistic that I
> can write tests, probably by checking the error messages.

Great, genget seems like a sufficiently small and easily testable
module, so adding some test cases for it would be nice.  Maybe you can
try commit access with that patch?  What's your savannah username?  I'll
be happy to review a couple of iterations of it first.  While it is
possibly to put self-tests in libtelnet/, I think we should put all of
them in tests/.

While not written down, if it is possible to write the self-test so it
works standalone, third parties can more easily reuse out internal
self-tests.  That is, consider making this possible as well as
integrating it with 'make check':

gcc -o foo -Ilibtelnet libtelnet/genget.c tests/test-genget.c

Maybe you can write test-genget.c so it tests for the corner-cases you
noticed.

> I would suggest to keep our solution

Me too.

/Simon

signature.asc
Description: PGP signature