0

So I'm running into an issue with ExpressJS and can't seem to find documentation to solve the issue.

Tech:

  • body-parser: 1.17.0
  • express 4.15.0
  • multer: 1.3.0
  • MongoDB
  • Postman

View is currently 3 fields:

  • name (required)
  • tagline (required)
  • image (optional)

What I'm trying to do is Error Handle on the Image before writing anything into the database. The image can only be of mime type image/jpeg or image/png to prevent HTML being uploaded with malicious JS inside.

I believe the issue seems to be that I'm not properly triggering an error while running through the image check conditionals and sending multiple responses which is setting off the Error: Can't set headers after they are sent.

drinks.routes.js

var express = require('express');
var router = express.Router();
var jwt = require('jsonwebtoken');
var multer  = require('multer');
var passport = require('passport');
var config = require('../config/main');
var upload = multer({ dest: 'uploads/images' })

var Drink = require('../models/drinks.model.js');

router.use(function(req, res, next){
  next();
});

...

.post(passport.authenticate('jwt', { session: false }), upload.single('image'), function(err, req, res, next){
  var drink = req.body;
  var drinkImage = req.file;
  if(typeof drinkImage !== "undefined"){
    console.log('image was uploaded');
    if(drinkImage.mimetype !== "image/jpeg" || drinkImage.mimetype !== "image/png" ){
      console.log('Image was not a JPEG or PNG', drinkImage.mimetype);
      res.status(500).send({ error: "Your image was incorrect"});  // >>>>>>>>>>>>>>> The error seems to be coming from here. Unsure of how to properly raise a flag to tell the response to the client. Have tried res.send(), the res.status().send(), res.json(), currently working with next() method to keep going on but not sure how to define err if that is the case
    }
    console.log('image correct mimetype');
  } else {
    drinkImage = {}; // Setting this as an empty object so it doesn't throw an error with the model which is looking for `image: drinkImage.name`
  }
  Drink.createDrink(drink, drinkImage, function(err, drink, drinkImage){
    if(err){
      console.log('Error adding Drink', err);
      res.send(err);
    }
    res.status(200).json(drink)
  });
});

Topic Research

wsfuller
  • 1,810
  • 8
  • 31
  • 49
  • I'd say the problem is in the last couple of lines. If `err` is *truthy*, you are attempting to send the error via `res.send(err)` **and** set the status to 200 and respond with some JSON via `res.status(200).json(drink)`. You should probably put that last part in an `else` block – Phil May 29 '17 at 23:46
  • Looking further, you're simply lacking exit clauses when handling error conditions. Nothing in your code stops processing after you attempt to respond with an error status. Calling `res.send` does not stop the code from continuing – Phil May 29 '17 at 23:48
  • Sorry a bit confused on how to approach this reading over your first comment. Where that confusion comes in is there needs to be error handing on the image type & if there is an error creating the object in the database. By giving the freedom of having to optionally upload an image I'm not very clear on how to make that work. The `createDrink` I would think would have it's own error handling writing to the DB and the image type is different from that As for the 2nd comment have tried using `res.end()` to stop the process but still have some same results. – wsfuller May 30 '17 at 00:03
  • Read it again. `res.send` does **not** stop the process. Try `return res.send(...` – Phil May 30 '17 at 00:05
  • Understood that `res.send` does not stop the process that's why I tried `res.end`. The `return` was the missing piece. – wsfuller May 30 '17 at 00:57

1 Answers1

0

This issue is due to Javascript's asynchronous nature.

The code should not execute Drink.createDrink() after responding error 500.

if (drinkImage.mimetype !== "image/jpeg" || drinkImage.mimetype !== "image/png" ) {
  console.log('Image was not a JPEG or PNG', drinkImage.mimetype);
  res.status(500).send({ error: "Your image was incorrect"});
  return; // THIS IS VERY IMPORTANT!
}
haotang
  • 5,520
  • 35
  • 46
  • Running into another issue. Seems like `drinkImage.mimetype !== "image/jpeg" || drinkImage.mimetype !== "image/png"` Will cause the code to fail every time. However `drinkImage.mimetype !== "image/jpeg"` works as expected or `drinkImage.mimetype !== "image/png"`. Is there another way to write the OR operator there? – wsfuller May 30 '17 at 00:47
  • Must be getting too frustrated. Should have been &&. Thanks for the help, the `return` was the solution. `drinkImage.mimetype !== "image/jpeg" && drinkImage.mimetype !== "image/png"` – wsfuller May 30 '17 at 00:53
  • Glad that I could help :) – haotang May 30 '17 at 06:48