Simple Batch File Homework-CodePudding

So i have this simple homework of doing a batch file that shows 3 options, and an error message for the entry for the option. so far it seems simple for me. I saw a youtube video and according to what i know it suppose to run ok but the problem is that when I run it and choose option 2 it opens also option 1, and when I choose option 3 it doesn't exit the program. And when I choose a parameter that is not specify the error message do not show. I am gonna copy paste my batch file because is easy to understand.

@echo off

echo Choose an option:
echo.
echo 1)Open Disk cleanup

echo 2)Open Disk Defragmenter

echo 3)Exit
echo.

Set /p Op=Write the option:

if %op%==1 (
start %windir%\system32\cleanmgr.exe
) 
else if %op%==2 (
start %windir%\system32\dfrgui.exe
)
else if %op%==3 (
exit
) 
else (
    cls
    echo Error not defined
)   
pause

Is there something that I am missing or wrote wrong please let me know. If you can copy paste and run it in your computer and tell me if it is working fine because something tell me that in the process of learning I try a few times and i think I messed up with something that suppose to run correctly the file since my instructor told me that we have to be careful when using the commands in a batch file.

CodePudding user response：

Your code formatted correctly but is susceptible to command injection because of the usage of the SET /P command.

@echo off

echo Choose an option:
echo.
echo 1)Open Disk cleanup

echo 2)Open Disk Defragmenter

echo 3)Exit
echo.

Set /p Op=Write the option:

if "%op%"=="1" (
    start "" "%windir%\system32\cleanmgr.exe"
) else if "%op%"=="2" (
    start "" "%windir%\system32\dfrgui.exe"
) else if "%op%"=="3" (
    exit /b
) else (
    cls
    echo Error not defined
    pause
)

A best practice solution which handles invalid input options.

@echo off
echo Choose an option:
echo.
echo 1)Open Disk cleanup

echo 2)Open Disk Defragmenter

echo 3)Exit
echo.

choice /C 123 /N /M "Select an Option:"
set "op=%errorlevel%"

if "%op%"=="1" start "" "%windir%\system32\cleanmgr.exe"
if "%op%"=="2" start "" "%windir%\system32\dfrgui.exe"
if "%op%"=="3" exit /b

CodePudding user response：

I did it with this code.

@echo off

echo Choose an option:

echo.

echo 1)Open Disk Cleanup Utility

echo 2)Open Disk Defragmenter

echo 3)Exit

echo.

Set /p op/3=Write the number option:

if %op/3%==1 ( start %windir%\system32\cleanmgr.exe )

if %op/3%==2 ( start %windir%\system32\dfrgui.exe )

if %op/3%==3 ( exit )

else ( cls echo Error not defined )
pause

the only problem that i have is that the error message appears even though option 1 and 2 are selected, it executes the options but it also shows the error message. I only want that the error message to show if any of those parameters aren't choose. I know that else is responding because if I choose option 1 else is executes because is not number 2 or 3.What it comes to my mind if it is possible to define the op variable in a enumeration format like a group set of options.