FDOS / freecom

FreeDOS Command Shell (command.com)
http://www.freedos.org/
GNU General Public License v2.0
154 stars 39 forks source link

CMDLINE #51

Closed PerditionC closed 3 years ago

PerditionC commented 3 years ago

see bug report (to be filled in later) - verify and correct if not that FreeCOM sets the PSP portion correctly for long command lines that pass the full command line in CMDLINE environment variable

PerditionC commented 3 years ago

From various emails regarding this issue:

freecom sets the length byte of the command tail at PSP:80h to the "true" value of the commandline length. If the command line is 130 chars, this value will be 82h. However, since the PSP has a size of 256 only, just the first 127 bytes are copied to PSP:81h-ffh

If a command line exceeds 126 bytes, the value at [PSP:80h] "should" be 0x7Fh ( and a 0x0D "should" be placed at [PSP:FFh] ) - that's at least what COMMAND.COM of Win95/98 does. The environment variable CMDLINE will then contain the full command line. It's documented in RBIL. see also in RBIL http://www.ctyme.com/intr/rb-4248.htm

[PSP:80h] > 7Eh tells programs to get command line from environment variable CMDLINE

In FreeCOM, MAX_EXTERNAL_COMMANDSIZE is 125 (7Dh)_ because for normal command lines, psp_cmdlineLength is max that, followed by psp_cmdline with trailing \r AND \0 chars.

Now when you have a LONG command line, two areas of the C code become relevant:

shell/init.c is what received the command line when you start a new instance of FreeCOM,

int initialize(void)
{
...
  /* Aquire the command line, there are three possible sources:
    1) DOS command line @PSP:0x80 as pascal string,
    2) extended DOS command line environment variable CMDLINE,
      if peekb(PSP, 0x80) == 127,&
    3) MKS command line @ENV:2, if peekb(ENV, 0) == '~'
        && peekb(ENV, 1) == '='

    Currently implemented is version #1 only
  */
  cmdlen = peekb(_psp, 0x80);
  if(cmdlen < 0 || cmdlen > 126) {
    error_corrupt_command_line();
    cmdlen = 0;
  }
...

The other part is that when you already have FreeCOM running and want to execute some other app, via lib/exec.c

/lib/exec.c

int exec(const char *cmd, char *cmdLine, const unsigned segOfEnv)
{
...
  assert(cmd);
  assert(cmdLine);
  assert(strlen(cmdLine) <= 125);
...

as well as shell/command.c,

shell/command.c

void execute(char *first, char *rest, int lh_lf)
{
...
        if(strlen(rest) > MAX_EXTERNAL_COMMAND_SIZE) {
        char *fullcommandline = malloc( strlen( first ) + strlen( rest ) + 2 );
        error_line_too_long();
        if( fullcommandline == NULL ) return;
        sprintf( fullcommandline, "%s%s", first, rest );
        if( chgEnv( "CMDLINE", fullcommandline ) != 0 ) {
            free( fullcommandline );
            return;
        }
        free( fullcommandline );
        }
...

This displays "Commandline longer than 125 characters." and leaves it up to the user to wonder whether that is a problem. Actually it cannot know which apps support long command lines, so thata probably is OK.

Also note: CMDLINE is only used for com and exe, not for bat.

Unfortunately, the above part is the ONLY place which uses MAX_EXTERNAL_COMMAND_SIZE, while the assert above hardcodes 125 and init.c hardcodes "negative or above 126" and fails to to anything about long command line used when it is started with one itself, by some outside caller.

shell/cswapc.c contains this:

shell/cswapc.c

DoExec(char *command,char *cmdtail)
{
...
        len = strlen(cmdtail);
        if (len >= 127) len = 127; SEE BELOW
        dosCMDTAIL[0] = len;
        dosCMDTAIL[1+ len] = '\r';
...

Because dosCMDTAIL is ((char far*)MK_FP(_psp, 0x80)) this would actually overflow the buffer by 1 byte

Japheth's description implies that 127 has to be used as magic length, together with injecting an early \r into the PSP and not bothering to put an \0 after it.

So proper code without long command line support was, only changing the SEE BELOW line:

if (len > MAX_EXTERNAL_COMMAND_SIZE) len = 126;

But proper code for long command line support would make a case distinction for long versus short lines.

Note that the assembly language parts of FreeCOM do not process long command lines at all, they simply assume that the C parts have already limited the PSP command line in a sensible way and do not care whether or not the environment contains any CMDLINE variable. This probably is OK.

Best case for external programs utilizing CMDLINE:

while parsing the command line is look at the length stored at PSP:80h. If it is less than 126, I use the data stored at PSP:81h up to the first 0Dh (Carriage Return) character. If the length is 126 or more, I look for the CMDLINE environment variable and use it (if it exists). If I can't find a CMDLINE environment variable, I go back and use what is stored at PSP:81h up to the first 0Dh character. I never actually use (or trust) the value stored at PSP:80h to provide the actual length, but simply use it as a flag to tell me whether or not I should bother looking for a CMDLINE environment variable.

PerditionC commented 3 years ago

should now be fixed when exec'ing programs see https://github.com/FDOS/freecom/commit/ba97fbc3c01f748c3ccd55bcdc949ea543402f7f still need to enable FreeCOM to understand long command lines for itself when initializing

PerditionC commented 3 years ago

now also supports being called with long command lines see commit e05fa918e6a9eb97273d8310bc4f833b766e8f98 and reworked commit a773b62fd9d41255be5432c54b1d238eab4ff34a. Maybe go back to original commit? it was easier to understand for a maintenance point of view and may actually use less memory due to less code...

PerditionC commented 3 years ago

as a unrelated note I need to reconfigure my editor, it is really messing up tabs/spaces

dcoshea commented 3 years ago

I hit this in FreeDOS 1.3-rc4 when using devload to load comment.sys (a config.sys equivalent of echo, and yes I'm aware echo can be used in config.sys in FreeDOS) from http://cd.textfiles.com/simtel/simtel/DISK1/DISC2/BOOTUTIL/COMMENT2.ZIP:

D:\FREEDOS\BIN>devload c:\comment.sys 000102030405060708091011121314151617181920
212223242526272829303132333435363738394041424344454647484950515253545abcde
Commandline longer than 125 characters.
DEVLOAD v3.25 - load DOS device drivers - license GNU General Public License 2
(c) 1992-2011 David Woodhouse, Eric Auer (FreeDOS.org)

Loading: C:\COMMENT.SYS
00010203040506070809101112131415161718192021222324252627282930313233343536373839
4041424344454647484950515253545é¯
Driver not loaded
D:\FREEDOS\BIN>

Note that abcde, which is the part of the command line that cannot possibly fit within the 127 character command-line tail in the PSP, got replaced with garbage characters.

More appropriate behaviour is seen with FreeCom 0.85a:

D:\FREEDOS\BIN>devload c:\comment.sys 000102030405060708091011121314151617181920
212223242526272829303132333435363738394041424344454647484950515253545abcde
DEVLOAD v3.25 - load DOS device drivers - license GNU General Public License 2
(c) 1992-2011 David Woodhouse, Eric Auer (FreeDOS.org)

Loading: C:\COMMENT.SYS
00010203040506070809101112131415161718192021222324252627282930313233343536373839
404142434445464748495051525354
Driver not loaded
D:\FREEDOS\BIN>

In this case the last 5 is also missing, presumably to make way for the carriage return character, but importantly no characters were read past the end of the buffer.