This code does not run but only exits. I am not sure where I have made a mistake

Question

I am hoping you can help me. So, I have been trying to run this command. But everytime I do so..it exits out. Then, I found this error.."batch file do( was unexpected at this time" I am not sure where I made a mistake. Thanks in advance for all the replies.

Here is my code:

@echo off
for /L %%n in (1,1,3) do (
    set uname=mama 
    set pword=mu

    :xy
    set /p user ="Enter Username"
    set /p pass="Enter Pass"
    if %uname%==%user% ( echo 
        username is valid ) ELSE (
        echo username not found
        goto xy
    )
    IF %pword%==%pass% ( echo sucess 
    ) ELSE (
        echo invalid password
        goto xy
    )
    pause
    if %%n EQU 3 (echo Run Again)
)

no labels and no `goto`s allowed within a code block. Change your logic. Technically: [delayed expansion](https://stackoverflow.com/questions/30282784/variables-are-not-behaving-as-expected/30284028#30284028) in combination with [empty variable with if](https://stackoverflow.com/questions/27489804/set-p-empty-answer-crash/27489844#27489844) — Stephan, Feb 09 '20 at 18:15

Io-oI · Answer 1 · 2020-02-09T18:49:38.407

This is elaborated based in @Stephan comments:

Replace the for loop with a _cnt counter to limit attempts by if.

@echo off && setlocal enabledelayedexpansion 

set "uname=mama" && set "pword=mu"

:xy

set /a "_cnt+=1+0" && set /p "user=Enter Username: "

if not !_cnt! EQU 3 (

   if "!uname!" == "!user!" (echo/ Username is valid^!! 
       ) else ( echo/ Username not found^!! & goto :xy )

   set /p "pass=Enter Pass: " 

   if "!pword!" == "!pass!" (echo/ Sucess^!! 
       ) else ( echo/ Invalid password^!! & goto :xy )

   ) else ( echo/ Run Again^!! && %__APPDIR__%timeout -1 & endlocal &  goto :EOF )

rem./ do more tasks here... after use endlocal ... && endlocal & goto :EOFF

Compo · Answer 2 · 2020-02-10T02:00:29.670

Looking at your code it appeared to me that using Call would solve both your nested label and delayed expansion issues, and probably make your code easier for you to manage.

@Echo Off
Set "uname=mama"
Set "pword=mu"
Set "user="
Set "pass="

:Inputs
For /L %%G In (1,1,3) Do (
    Call :xy
    If Not ErrorLevel 1 GoTo Main
)
"%__AppDir__%choice.exe" /M "Would you like to try again"
If Not ErrorLevel 2 GoTo Inputs
Exit /B 1

:Main
Rem Your code below here for users with successful inputs.
Echo Welcome %user%.
Timeout /T 3 /NoBreak > NUL
Rem Your code ends above here.
Exit /B 0

:xy
ClS
Set /P "user=Enter Username> %user%"
If /I Not "%uname%" == "%user%" (
    Echo Username not found.
    Set "user="
    Timeout /T 2 /NoBreak > NUL
    Exit /B 1
)
Echo Username is valid.
Set /P "pass=Enter Pass> "
IF "%pword%" == "%pass%" Exit /B 0
Echo Invalid password.
Set "pass="
Timeout /T 2 /NoBreak > NUL
Exit /B 1

You may notice that I've used the recommended syntax for defining variables with the Set command. It ensures that the content is protected and does not have things such as hidden trailing spaces. If you take a look at the code in your question, you will note that the value assigned to the uname variable is actually mama<space>. This would obviously cause issues when mama is locked out of the rest of the script!

You also notice had an unwanted newline instead of a space between echo and username is valid.

Edit

The code above was re-worked to allow for up to three attempts at entering both values correctly, although the try again message seems a little redundant then.

that forces the user to enter valid credentials three times before executing the payload. — Stephan, Feb 09 '20 at 20:39
To be honest with you @Stephan, I didn't even consider that the intention was to allow the end user only three attempts at getting both entries correct. I apologise for my misunderstanding and have re-worked my code to account for that. — Compo, Feb 10 '20 at 00:42

score 0 · Answer 3 · answered Feb 09 '20 at 21:14

You had a few errors in your code.

labels are not allowed in code blocks and may break them. // Labels removed.
goto unconditionally breaks the code block // left one goto to intentionally leave the loop and removed the rest
you need delayed expansion to use a variable changed within the same code block (technically not needed for uname and pword, as I defined them outside the loop) // enabled and used delayed expansion
You had a space in set uname=mama<space> // fixed by recommended syntax set "var=value"
(security issue) for security reasons, you shouldn't tell if it was the username or the password (or both), which was wrong. // fixed (as a bonus, it simplifies the code)

With the above errors fixed and reduced code complexity, it could look like:

@echo off
setlocal enabledelayedexpansion
set "uname=mama"
set "pword=mu"
for /L %%n in (1,1,3) do (
    set /p "user=Enter Username: "
    set /p "pass=Enter Password: "
    if "!uname!-!pword!" == "!user!-!pass!" goto :payload
    echo Invalid username or password.
    pause
)
echo sorry, run again.
goto :eof

:payload
echo successfully logged in.
REM rest of your code

(speaking of security: having the credentials in clear text in the (readable) code isn't secure by any standard, but it's hard to overcome with pure batch)

This code does not run but only exits. I am not sure where I have made a mistake

3 Answers3