[Yashas]

Quote:

Originally Posted by Kar Why 28th character, why not 31 (32nd)? Everyone knows about that issue. Everyone who uses zcmd more or less knows about it and fixed it themselves using the comment I made here https://github.com/YashasSamaga/I-ZC...8eda081a05d37c You fixed it by making CallLocalFunction fail? Heh. Be careful with that function. I honestly rather NOT call that function if I know its going to fail.

Why not 31? ZCMD variants (YCMD, IZCMD) prefix "cmd_" to every command function that is created so as to distinguish between commands and user created public function. And also to prevent clashes between public function identifiers. (Check the defines in the include to understand better)

Everyone is confused about the recent fix and I apologize for not stating it clearly in the commit description.

What is the issue?
OnPlayerCommandText crashes when a player uses very lengthy commands since the working algorithm tries to copy the command to an array after converting the characters to lowercase and fails since it tries to access an index which is outside the array bounds.

The algorithm stores the command name which was converted to lowercase in an internal array whose size is 32 (since identifiers in PAWN have a length limit of 32). If a player types a lengthy command (length > 28 ; command name + "cmd_" = length of 32 characters), the algorithm tries to copy the command to the array whose size is 32 which isn't sufficient and triggers the AMX's default exception handeling mechanism which in turn ends the OnPlayerCommandText function call (crashes OnPlayerCommandText).

What was the fix that was applied?
Increased an internal array size so that it can accommodate command names of 144 in size (length of the longest client message).

Does this fix the crash?
Yes.

Does this allow lengthy commands to work?
No.

So how is this a fix?
It is a fix since it solves the crash issue and lets OnPlayerCommandPerformed to be executed. The fix never intended to allow commands which have more than 28 characters to work.

Why let the CallLocalFunction fail? Why not simply stop the CallLocalFunction call?
1. No server has commands which consist more than 28 characters since the script wont compile if there were such commands.
2. The server nor the staff team of the server would ask anyone to use a command that doesn't exist. (lengthy commands)
3. The probability that someone uses a command that is very long is almost zero.
4. There are more chances of a player typing an invalid command which does not cross the character limit.

So its not worth adding protection against lengthy commands which are executed every single time for every command sent. (Did you understand now? You can stop reading: You may continue reading - (condtition)?(if true)if false)).

What were my options at the issue?
1. Add a length check before calling the command function.
Disadvantages: Waste of CPU

2. Increase the internal array length

Why I choose fix #2 over fix #1?
The answer is straight forward and simple, I did not want to add more checks to the code since it would waste CPU.

Isn't trying to call a command that doesn't exist (since its length is far more than the limit) using CallLocalFunction waste of CPU given that CallLocalFunction is a very slow function?
Yes, for cases where the command which was typed exceed the 28-character limit.

No for overall performance.

What happens if I had used fix #1?
I would be doing checks for all the commands to avoid invalid CallLocalFunction calls where the command was longer than the 28-character limit (which happens once in a blue moon - I will take a blind guess, maybe 1 in 1000000).

What happens if I had used fix #2?
IZCMD will carry out its usual command processing and will do it extremely fast.

If a command which is longer than 28 characters is sent, IZCMD will still carry out its normal processing and try to call the command function.

The last step of calling the command function will fail.

Is it worth avoiding the CLF call if the command name was very long?
No. Adding a check for that will waste CPU every single time a command is sent irrespective of if it was within the limits or not.

"Compared to the CPU wasted calling CallLocalFunction for an invalid command just once in 100000 trials is WAY WAY WAY WAY WAY less compared to the CPU wasted doing the check for legitimate commands."