atuttle / Taffy

:candy: The REST Web Service framework for ColdFusion and Lucee
http://taffy.io
Other
226 stars 118 forks source link

[RFC] Better handling for missing required fields #291

Open atuttle opened 9 years ago

atuttle commented 9 years ago

Given the following resource method signature:

function put( required someVar ){}

It would be nice if Taffy better dealt with missing required arguments/fields. Currently, CF throws an exception because we try to invoke the function without including a value for required arguments.

Possible implementations:

{
  "error": "The following required fields were missing from your request: someVar, foo, bar, baz"
}
neokoenig commented 9 years ago

I'd prefer the latter myself

tomchiverton commented 9 years ago

I'd prefer the latter, as long as it can be disabled and/or overridden (maybe I don't want my API to helpfully list all the arguments)

ryanguill commented 9 years ago

I also prefer the latter, but would also like the ability to have a handler method that can be called to override what happens (in case I dont want to use the default 400, also for logging, etc).

Does the same CF error happen with a mismatch of types? Would you also want to pre-check those?

atuttle commented 9 years ago

The same situation does happen for type mismatch. I suppose if we're going to do one we should do the other.

atuttle commented 9 years ago

as long as it can be disabled and/or overridden (maybe I don't want my API to helpfully list all the arguments)

@tomchiverton While my first instinct was to agree, I'm not so sure. Why is disclosing required fields a security risk? I'm inferring it was for security reasons you don't want them listed; please correct me if I'm wrong here. Just sounds like "security by obscurity" to me, which is never a good idea.

coldfumonkeh commented 9 years ago

The latter option seems to be the most preferable and one that I'd like to see in there. Thanks for raising it @atuttle

tomchiverton commented 9 years ago

On Monday 24 Aug 2015 08:51:44 Adam Tuttle wrote:

as long as it can be disabled and/or overridden (maybe I don't want my API to helpfully list all the arguments) @tomchiverton While my first instinct was to agree, I'm not so sure. Why is disclosing required fields a security risk? I'm inferring it was for security reasons you don't want them listed; please correct me if I'm wrong here. Just sounds like "security by obscurity" to me, which is never a good idea.

I might change them, because they were undocumented, and break some user. But it's their own fault at that point I suppose.

Tom Ah, love, could thou and I with fate conspire, To grasp this sorry scheme of things entire, Would not we shatter it to bits, and then, Remould it nearer to the heart's desire.

atuttle commented 9 years ago

If they're required, how can they be undocumented? (How can anyone get anything done with undocumented requirements?) Unless you're referring to an internal api?

tomchiverton commented 9 years ago

I mean to someone who just hits the API, rather than analysing an existing client or reading the docs, yeah

guillaume-boivin commented 9 years ago

It's a good idea. Most (if not all) of the time you will need to validate the arguments against a set of rules for security and/or business logic. I think that a validator would be nice, because type and required is 2 common tasks but there are far more than that (length, white list, etc...)

atuttle commented 9 years ago

I've been entertaining the idea of a plugin system for a while now. Maybe the validator is the first plugin?

On Wed, Sep 2, 2015, 9:31 PM Guillaume Boivin notifications@github.com wrote:

It's a good idea. Most (if not all) of the time you will need to validate the arguments against a set of rules for security and/or business logic. I think that a validator would be nice, because type and required is 2 common tasks but there are far more than that (length, white list, etc...)

— Reply to this email directly or view it on GitHub https://github.com/atuttle/Taffy/issues/291#issuecomment-137296482.

donwalter commented 8 years ago

The workaround we're trying out right now is to make the arguments required="true", along with default="". This passes the CF validation for missing arguments, and then we can handle it in the function with whatever response we want if the value="". We then modified the documentation to hide the default value, so it just shows them as required with no default value. It might not be the most elegant solution... but it appears to be working, to avoid the CF exception and return friendly errors