Is it invalid to write re.send() function inside mydb.collection().find().toArray callback?

Question

I'am setting up a login page for my app. I want to send a file after verifing if the login page is provided with proper username and password.

I have a handler for a post request which checks if the user entered correct username and password.

app.post('/login',function(req,res){
    var data="";
    var flag_isthere=0,wrongpass=0;
    console.log('login-done');
    req.setEncoding('UTF-8')
    req.on('data',function(chunk){
        data+=chunk;
    });
    req.on('end',function()
    {


        MongoClient.connect("mongodb://localhost:27017/userdetails",{useNewUrlParser: true ,useUnifiedTopology: true },function(err,db)
        {

            if(err) throw err;
            var q = JSON.parse(data)
            const mydb=db.db('userdetails')
            var c=mydb.collection('signup').find().toArray(
                function(err,res)
                {
                for(var i=0;i<res.length;i++)
                    if( (res[i].email==q['email']) )    //check if the account exists
                    {
                        flag_isthere=1;
                        if( (res[i].pass != q['pass'] ) )
                            wrongpass=1;
                        break;
                    }

                if(flag_isthere==0)
                {
                    console.log(q['email'], ' is not registered')
                }
                else
                {
                    console.log('Already exists!!!');       
                }

                if( wrongpass==1)
                {
                    console.log('password entered is wrong')
                }

                if(flag_isthere==1 && wrongpass==0)
                {
                    console.log('Congratulations,username and password is correct');
                    res.send( { login:'OK', error:'' } );    //this statement is giving an error in node JS part
                }


            });//var c
        })//mongoclient.connect

    })//req.on 

    res.send({ login:'OK', error:'' });     //this works properly in node JS
    console.log(flag_isthere , wrongpass )  //but here the flag_isthere==0 and wrongpass==0 , so it won't get validated

});

It gives the error as

TypeError: res.send is not a function
    at E:\ITT_project_shiva\loginserver_new.js:112:25
    at result (E:\ITT_project_shiva\node_modules\mongodb\lib\operations\execute_operation.js:75:17)
    at executeCallback (E:\ITT_project_shiva\node_modules\mongodb\lib\operations\execute_operation.js:68:9)
    at handleCallback (E:\ITT_project_shiva\node_modules\mongodb\lib\utils.js:129:55)
    at cursor.close (E:\ITT_project_shiva\node_modules\mongodb\lib\operations\to_array.js:36:13)
    at handleCallback (E:\ITT_project_shiva\node_modules\mongodb\lib\utils.js:129:55)
    at completeClose (E:\ITT_project_shiva\node_modules\mongodb\lib\cursor.js:859:16)
    at Cursor.close (E:\ITT_project_shiva\node_modules\mongodb\lib\cursor.js:878:12)
    at cursor._next (E:\ITT_project_shiva\node_modules\mongodb\lib\operations\to_array.js:35:25)
    at handleCallback (E:\ITT_project_shiva\node_modules\mongodb\lib\core\cursor.js:32:5)
[nodemon] app crashed - waiting for file changes before starting...

How do I send the response to the user after proper validation?

You've shadowed the `res` variable (`......toArray(function(err,res){.....}`). — Titus, Nov 01 '19 at 13:03
In addition, `res.send({ login:'OK', error:'' });` will run way earlier than you think; you need to move all `res.send()` commands *inside* the callback(s). — , Nov 01 '19 at 13:04
And another issue: rather than grabbing all users from the db and running a loop, use `find()` properly to only grab matching documents from the db in the first place. Should be something like `find({email: q.email })` — , Nov 01 '19 at 13:06
Storing passwords in your database is ***extremely*** poor practice. Store high-quality hashes, and compare hashes, not the actual password. — T.J. Crowder, Nov 01 '19 at 13:16

T.J. Crowder · Answer 1 · 2019-11-01T13:16:22.727

It's not that you're doing it from the callback that's the problem. There are two different problems:

You're shadowing res by redefining it in the callback's parameter list
(Once you fix that) You're calling res.send twice:
- Once at the end of your posthandler
- Once within the callback
send implicitly completes the response, so you can only call it once.

In your case, you want to call it from within your callback, once you've determined that none of the records matches.

See *** comments for a rough guideline (but keep reading):

app.post('/login', function(req, res) {
    var data = "";
    var flag_isthere = 0,
        wrongpass = 0;
    console.log('login-done');
    req.setEncoding('UTF-8')
    req.on('data', function(chunk) {
        data += chunk;
    });
    req.on('end', function() {
        MongoClient.connect("mongodb://localhost:27017/userdetails", {
            useNewUrlParser: true,
            useUnifiedTopology: true
        }, function(err, db) {
            if (err) throw err;
            var q = JSON.parse(data)
            const mydb = db.db('userdetails')
            var c = mydb.collection('signup').find().toArray(
                function(err, array) { // *** Renamed `res` to `array
                    for (var i = 0; i < array.length; i++)
                        if ((array[i].email == q['email'])) //check if the account exists
                    {
                        flag_isthere = 1;
                        if ((array[i].pass != q['pass']))
                            wrongpass = 1;
                        break;
                    }

                    if (flag_isthere == 0) {
                        console.log(q['email'], ' is not registered')
                    } else {
                        console.log('Already exists!!!');
                    }

                    // *** Handle result here
                    if (flag_isthere == 1 && wrongpass == 0) {
                        console.log('Congratulations,username and password is correct');
                        res.send({ login: 'OK', error: '' }); //this statement is giving an error in node JS part
                    } else if (wrongpass == 1) {
                        console.log('password entered is wrong')
                        // *** res.send(/*...*/)
                    } else {
                        // Handle the issue that there was no match
                        // *** res.send(/*...*/)
                    }
                }
            ); //var c
        }) //mongoclient.connect
    }) //req.on 

    // *** Don't try to send a response here, you don't know the answer yet
});

but, it seems like you should be able to find just the one user (via findOne? I don't do MongoDB), rather than finding all of them and then looping through the resulting array.

See also the answers to these two questions, which may help you with asynchronous code issues:

A couple of other notes:

I strongly recommend using booleans for flags, not numbers.
NEVER store actual passwords in your database!! Store a strong hash, and then compare hashes.
You might find async/await syntax more convenient to work with. I think recent MongoDB clients support promises (which you need for async/await).

Is it invalid to write re.send() function inside mydb.collection().find().toArray callback?

1 Answers1