3
\$\begingroup\$

I come from a PHP object oriented framework background (Laravel, Symfony, Silex...). With that, validation comes in pre-built classes mechanism in a framework that validates for you parameters that you define and validates against. If it fails validation, you don't proceed.

Example from Laravel:

 public function store(Request $request)
    {
        $validator = Validator::make($request->all(), [
            'title' => 'required|unique:posts|max:255',
            'body' => 'required',
        ]);

        if ($validator->fails()) {
            return redirect('post/create')
                        ->withErrors($validator)
                        ->withInput();
        }

        // Store the blog post...
    }

Here, we define that it is required, max 255 for title. It is easy to read. Symfony is a bit more elaborate but still easy to read due to easily predefined class:

public static function loadValidatorMetadata(ClassMetadata $metadata)
{
    $metadata->addPropertyConstraint('firstName', new Assert\NotBlank());
    $metadata->addPropertyConstraint(
        'firstName',
        new Assert\Length(array("min" => 3))
    );
}

I'm starting to dive into node.js with express.js as base to create an API (I'm aware that Restify and Loopback exist). Currently I have one end point, which I would like to validate a hash token to identify the client, and then inspect that the stored key/value pair matches. The storage is from redis, the following is the code:

/**
 * Check if the hash is in redis
 * @function CheckParam
 * @param {object} req - json object with the data that will be inserted in queries
 * @param cb - callback
 * @callback {object} status code
 */
http.checkParam = function CheckParam(req, cb) {

    if (typeof req.body.data !== 'undefined' && req.body.data && typeof req.query.hash !== 'undefined'
        && req.query.hash && typeof req.body.id !== 'undefined' && req.body.id) {
        if (typeof req.body.data !== 'object' && typeof req.query.hash !== 'string' && typeof req.body.id !== 'number') {
            log.error({part: 'save'}, 'Wrong parameters type');
            cb(403);
        } else {
            redis.get(req.query.hash, function (err, data) {
                if (err) {
                    log.error({part: 'save'}, 'Error from redis \n ' + err);
                    cb(403);
                } else if (!data) {
                    log.error({part: 'save'}, 'Token not found \n ' + req.query.hash);
                    cb(403);
                } else {
                    try {
                        var obj = JSON.parse(data);
                        if (obj.member.id == req.body.id) {
                            cb(200);
                        } else {
                            log.error({part: 'save'}, 'Wrong user id ' + req.body.id);
                            cb(403);
                        }

                    } catch (e) {
                        log.error({part: 'save'}, 'Error from redis \n ' + e);
                        cb(403);
                    }
                }
            });
        }
    } else {
        log.error({part: 'save'}, 'Connection refused');
        cb(403);
    }
};

Is there a more elegant way to validate parameters received from client similar to the above syntax from PHP but in node.js. I reverted to basic "if &&" in order to check 2 simple parameter, but as you can see, it is extensive.

\$\endgroup\$
  • 1
    \$\begingroup\$ That could be cleaned up a lot, but that's not the route I'd recommend taking. Just use a prebuilt validation library -- require it, and then you'll be able to write code similar to your laravel code. Here's one option: github.com/ctavan/express-validator \$\endgroup\$ – Jonah Nov 25 '15 at 1:31
  • \$\begingroup\$ @Jonah are you able to reference your answer (the plugin) with a bit of refactoring of the above code so that you can get the answer pts. because that would help. \$\endgroup\$ – azngunit81 Nov 25 '15 at 4:05
3
\$\begingroup\$
if(typeof req.body.data !== 'undefined' && req.body.data ...

// to

if(req.body.data && req.query.hash && typeof req.body.id)

Assuming that false, 0, null and an empty string has no special meaning in this program, you can simply use loose comparison. Anything falsy (null, undefined, false, 0, NaN, empty string) will just fall off.

function foo(bar){
  if(bar !== 'foo'){
    if(bar !== 'bas'){
      if(bar !== 'bam'){
        // ok, bar is what it is
      } else {
        throw new Error('not bar');
      }
    } else {
      throw new Error('not bar');
    }
  } else {
    throw new Error('not bar');
  }
}

// to

function foo(bar){
  // Expanded for emphasis. You can actually combine
  if(bar === 'foo') throw new Error('not bar');
  if(bar === 'bas') throw new Error('not bar');
  if(bar === 'bam') throw new Error('not bar');

  // ok, bar is what it is
}

Another approach to flattening conditionals is to "return early". Usually, when you have nested if statements, you can invert the condition and the code will look flat. But your mileage may vary, depending on your code.

Now for your errors, HTTP response numbers will cover that for you.

  • To start, 4xx errors mean the client did something wrong. On the other hand, 5xx errors means the server did something wrong.

  • A "wrong parameters type" means the client sent the wrong data. That's a 400 (Bad Request) and not a 403 (Forbidden).

  • A database error or a parse error should be a 500 (Internal Server Error).

  • If the token does not exist or has the wrong id, that counts as a 401 (Unauthorized). The difference between 401 and 403 is that a 401 can be lifted if you are authenticated (ie. token exists, validation passes etc.) while 403 is more of a permanent unauthorized access.

With that, and removing the logging, this should be all that's left. Feel free to add back the brackets. I usually omit them if I deal with one-liner bodies.

http.checkParam = function CheckParam(req, cb) {
  if(!req.body.data || !req.query.hash || ! req.body.id) cb(400);
  else redis.get(req.query.hash, function(err, data) {
    if(err) cb(500);
    else if(!data) cb(401);
    else try {
      var obj = JSON.parse(data);
      if (obj.member.id !== req.body.id) cb(401);
      else cb(200);
    } catch(error){
      cb(500);
    }
  });
};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.