1

Here is my code I am doing this in node.js and node-mysql module.

function registerUser(username, password) {
    var state = '';

    if (!username) { //line #4
            state = 'Missing <strong>username</strong>';
            return state;
    }
    if (!password) { //line #8
            state = 'Missing <strong>password</strong>';
            return state;
    }
    function addUser() {
            statement = 'INSERT INTO data (user_name, user_password) VALUES (\'' + username + '\', \'' + password + '\');';
            connection.query(statement, function(err) {
                    if (err) throw err;
                    console.log('User ' + username + ' added to database');
            })
    }

    connection.query('SELECT user_name FROM data WHERE user_name=\'' + username + '\'', function(err, result){
            if (err) throw err;
            if (!result[0]) {
                    addUser();
                    console.log('User ' + username + ' added.');
                    state =  'Success!';
                    return;
            } else {
                    console.log('User ' + username + ' already exists.');
                    state = 'User already exists!';
                    return;
            }
    })
return state; //line #33
}

Upon triggering if statements on line #4 & #8 state variable is being assigned and returned as intended. However code executed in anonymous function in connection.query is not assigning state and return on line #33 is returning nothing.

I am not sure if there is problem with variable scope or with that connection.query is non-blocking and return on line #33 is being executed before anonymous function.

Is there any way to make this work and return also state from anonymous function?

Thank you in advance.

hackal
  • 15
  • 3
  • 8

1 Answers1

2

.query appears to be asynchronous, which means that it probably executes after return state; is executed. What you need to do is provide a callback argument to registerUser:

function register_user(username, password, callback) {        

    if (!username) { //line #4
        callback('Missing <strong>username</strong>');                
    }

    if (!password) { //line #8
        callback('Missing <strong>password</strong>');
    }

    function addUser() {
            statement = 'INSERT INTO data (user_name, user_password) VALUES (\'' + username + '\', \'' + password + '\');';
            connection.query(statement, function(err) {
                    if (err) throw err;
                    console.log('User ' + username + ' added to database');
            })
    }

    connection.query('SELECT user_name FROM data WHERE user_name=\'' + username + '\'', function(err, result){
            if (err) throw err;
            if (!result[0]) {
                    addUser();
                    console.log('User ' + username + ' added.');
                    callback('Success!');

            } else {
                    console.log('User ' + username + ' already exists.');
                    callback('User already exists!');

            }
    })
}

You will end up using callback for both the synchronous and asynchronous responses. registerUser will end up going called in this manner:

registerUser(username, password, function(state) {
    console.log("Message was:", state);
});

A few other things I noticed:

  • SQL Injection problems.
  • It is better not to return HTML from a SQL call; you don't want to inject view-level concerns into the data layer.
Vivin Paliath
  • 94,126
  • 40
  • 223
  • 295
  • 1
    +1 for sql injection and SoC warning – Ingo Bürk Nov 10 '13 at 19:58
  • I am aware SQL injection. This is only local version I play with. Also your example works with `console.log();` but it doesnt seem to work with `return`. I need to use return because I would like to pass state as string to other method. Example: `res.send(registerUser(username, password, function(state){ return state; }));`. Is there any way `state` can be transformed & returned as string? – hackal Nov 10 '13 at 20:50
  • @hackal If you have a function that is performing both synchronous and asynchronous operations, there is no way you can use `return` for both. You will have to deal with a callback. – Vivin Paliath Nov 10 '13 at 20:51
  • @VivinPaliath Thank you for your advice. I called `res.send(state)` in anonymous function. Also to sql injection. I will eventually escape queries and hash passwords. Plus I will generate html using [EJS](http://embeddedjs.com/). And again thanks for tips. – hackal Nov 10 '13 at 21:01