Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up Error Handler #19

Closed
arb opened this issue Sep 5, 2016 · 3 comments
Closed

Clean up Error Handler #19

arb opened this issue Sep 5, 2016 · 3 comments
Milestone

Comments

@arb
Copy link
Owner

arb commented Sep 5, 2016

The current error handler spits back the entire Joi validation message which could be a pretty bad information leak as it contains the entire object.

The hapi response looks something like this:

{
  "statusCode": 400,
  "error": "Bad Request",
  "message": "child \"name\" fails because [\"name\" is not allowed to be empty]",
  "validation": {
    "source": "payload",
    "keys": [
      "name"
    ]
  }
}
@arb arb added the bug label Sep 5, 2016
@gonenduk
Copy link
Contributor

gonenduk commented Sep 5, 2016

I use Celebrate (Joi) to validate the endpoints input and Boom (made by the same person who made Joi) to report errors. With boom, the error information is not the error output. It has a special output.payload sub object which you send back to the client, while the rest of the info is for your own use (like writing to log files). I make sure to throw only Boom errors in my code, however Joi and system errors are not.. so my API error handler look like:

(err, req, res, next) => {
    const errPayload = Boom.wrap(err, err.isJoi ? 400 : 500).output.payload;
    if (errPayload.statusCode == 500) {
      logger.error(err.stack);
    }
    res.status(errPayload.statusCode).json(errPayload);
  }

In the code, I convert all errors to Boom errors (only if they are not ones already) and add the right status. Then I return the error output payload.

@arb
Copy link
Owner Author

arb commented Sep 6, 2016

Yeah that's a fine way to do it. In your case, using your own error handler would probably be the way to go as I'm not planning on introducing boom into this module. This bug is for specifically Celebrate.errors() which leaks the entire validated object back to the client.

I'll probably just end up copying what hapi does when it formats joi errors.

@arb arb added enhancement and removed bug labels Sep 7, 2016
@arb
Copy link
Owner Author

arb commented Sep 7, 2016

After reviewing this, I've removed the bug label. Worse case scenario is the client gets back the sent object which they've already got. So no big deal.

@arb arb modified the milestone: 3.0.0 Sep 8, 2016
@arb arb closed this as completed in 15b2ee2 Sep 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants