one_argument

Posted by Greven on Tue 16 Dec 2003 09:53 PM — 5 posts, 18,640 views.

Greven Canada Tue 16 Dec 2003 09:53 PM #0

Anyone ever notice this:

char *one_argument( char *argument, char *arg_first )
{
    char cEnd;
    sh_int count;

    count = 0;

    while ( isspace(*argument) )
	argument++;

    cEnd = ' ';
    if ( *argument == '\'' || *argument == '"' )
	cEnd = *argument++;

    while ( *argument != '\0' || ++count >= 255 )
    {
	if ( *argument == cEnd )
	{
	    argument++;
	    break;
	}
	*arg_first = LOWER(*argument);
	arg_first++;
	argument++;
    }
    *arg_first = '\0';

    while ( isspace(*argument) )
	argument++;

    return argument;
}

We jus recently noticed this when in a writing buffer, if your input extends past 80 characters, and you enter all CAPS, then it will lower the entire line, since it runs through it using one_argument. Maybe this was somewhere else, thought I would let people know.

David Haley USA Tue 16 Dec 2003 10:33 PM #1

That's interesting to know, that the one_argument function converts its result to lower case. Probably to have case-insensitive argument checking and the like, although this is a very good example of behavior with unpleasant side effects because it was implemented with redudancy. You'll notice, for example, that almost all argument-checking functions (e.g. mset) compare the argument case-insensitively. So it's redundant to first convert to lower-case, and then check without caring about case; and furthermore, converting it to lower case can have bad effects somewhere else.

I'm also unsure of the following line:

while ( *argument != '\0' || ++count >= 255 )

While the argument is not null or the count is greater than 255... that's a little silly, since the count starts out at 0, and therefore won't be greater than 255... this actually will *break* if your string is greater than 255 in length, because *argument will eventually be null, but since your count is bigger than 255, it'll continue... forever!

It should be:

while ( *argument != '\0' || ++count <= 255 )

This will, however, limit your argument size to 255 characters. That seems to have been their intention, though...

Nick Gammon Australia Forum Administrator Fri 19 Dec 2003 06:28 AM #2

I was about to say the same thing about the 255 but Ksilyan beat me to it.

The interesting thing is that a quick scan of where one_argument is used, it is usually passed a char x [MAX_INPUT_LENGTH] as an argument, and MAX_INPUT_LENGTH is 1024. Seems strange to limit it to 255 when you generally allow for 1024.

Except, of course that the test as written does the opposite.

Here could be a good test - go to a SMAUG mud and simply type a command with a 260 character argument. If the MUD goes dead you know they haven't picked that one up.

Hmmm - maybe I shouldn't suggest that? Anyway a quick test reveals it triggers the error message "Line too long". This kicks in for a 254 or more character line, so the likelihood of someone typing in a 260-character argument is pretty low. Seems a bit unnecessary to allow for 1024 character arguments all through the code too.

Gregar Fri 19 Dec 2003 06:18 PM #3

I believe the intent for this function was to prevent someone from running thru your game screaming in all caps. This prevents the occurance by forcing ALL-CAPS to all lower, effectively silencing the twits intent to disrupt.

Samson USA Fri 19 Dec 2003 06:57 PM #4

The case smashing of one_argument has been known all along - just look at the do_password function where they comment about it doing just that and having to basically rewrite it so it won't smash.

As for the crash, my code allows input of up to half of MAX_INPUT_LENGTH, which is currently set to 1024, so I can enter lines of up to 512. I just tried to crash my game by entering a ridiculously long ( about 560 or so ) command to the do_look function, using the first word as a valid target, then again with one that isn't. No crashes. It did as expected. The first time it showed me what I looked at, the second it just told me it wasn't there.

If there's a specific command this can be tested with, then maybe it's a fault with just that one command? Anyway, I long ago made my one_argument keep the case of commands passed to it. It doesn't appear to have triggered anything bad as a result. Not even spamming 15 yr olds :P