Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Ecto.Query.CastError when :id is not an integer / does not match the field type #69

Open
JamieREvans opened this issue Aug 3, 2017 · 4 comments

Comments

@JamieREvans
Copy link

JamieREvans commented Aug 3, 2017

If I put in "/users/foo", Canary will throw an exception in trying to access the resource as "foo" is clearly not an acceptable where query for id.

     ** (Ecto.Query.CastError) /my_app/deps/ecto/lib/ecto/repo/queryable.ex:331: value `"foo"` in `where` cannot be cast to type :id in query:
     
     from g in MyApp.Account.User,
       where: g.id == ^"foo",
       select: g

This exception should be caught and we should have an :unprocessable_entity handler in the error controller, rather than letting the application implode.

@cpjk
Copy link
Owner

cpjk commented Sep 4, 2017

You might be able to accomplish what you're looking for with https://github.com/cpjk/canary#specifing-database-field

However, if you want a more custom authorization solution, it is fairly simple to do it yourself using custom plugs (and Canada as well if you like the can? user, :action, thing API). 😺

@JamieREvans
Copy link
Author

Sorry for any confusion, @cpjk. What I meant was that if someone using canary with the default id field, which, in Ecto, only supports numbers, if a string is passed in the id field, Canary throws an exception in the plug before we're able to even catch / wrap it. We'd have to build a plug ahead of Canary for all connections to make sure the ID field isn't a string, which seems a little crazy.

I would suggest having Canary catch common exceptions like this, especially if it's just for a simple web app that uses basic Phoenix routing with object ids in the url, since the urls would be very susceptible to having bad IDs.

It is just a shame to see a server crash for something like this.

@cpjk
Copy link
Owner

cpjk commented Sep 4, 2017

Ah, I understand. What do you think would be the appropriate catching behaviour from canary?
Maybe return a 404? Maybe forbidden so as to not give anything away about URL structure.

@JamieREvans
Copy link
Author

I think a 422, unprocessable entity, is the right exception for the error handler to receive. 400 means the HTTP request was faulty, but 422 means that the request cannot be served as requested. It says less than 404 or 403, in my mind.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants