pandastrike / pbx

An experimental reimagining of Patchboard, optimized for modularity and ES6ness.
MIT License
0 stars 1 forks source link

Include validation errors in response #9

Open PandaWhisperer opened 9 years ago

PandaWhisperer commented 9 years ago

Synopsis

When using an API with a schema, a request that fails validation will return "400 Bad Request", but no further information about WHAT part of the validation failed.

As per HTTP 1.1 specs:

Except when responding to a HEAD request, the server SHOULD include an entity containing an explanation of the error situation

In order to increase compliance and usefulness (and aid developers in debugging their clients), PBX should return a JSON payload that explains the validation errors that occurred.

PandaWhisperer commented 9 years ago

Just realized that as with almost everything that aids debugging, it could also present a security risk, as it could aid an attacker "debug" the target system to find potential attack vectors.

Perhaps this should be an optional feature?

automatthew commented 9 years ago

I think we should use status 422 for JSON bodies that are parseable, but that do not pass validation. 400 is for unparseable bodies.

I do not foresee any security vulnerabilities that come from explaining how a document was invalid.

PandaWhisperer commented 9 years ago

Makes sense.

PandaWhisperer commented 9 years ago

So we have the following in context.coffee#L75-L76:

if request.headers["content-type"]?.match(/json/)
    JSON.parse yield context.body

JSON.parse will throw a SyntaxError if its argument is not well-formed JSON. This is currently not handled by PBX, meaning that it's up to the user to handle this case manually when using yield context.data somewhere. If the error is not handled by the user, a 500 Internal Server Error is the response.

automatthew commented 9 years ago

Need to change that then. Malformed request content is not a 5xx class error.

PandaWhisperer commented 9 years ago

Trying to figure out how to make test case for that. My current solution is breaking after one request.