mozilla / api.webmaker.org

Services for Webmaker
https://api.webmaker.org
16 stars 14 forks source link

Should OPTIONS requests do any validation? #160

Open jbuck opened 9 years ago

jbuck commented 9 years ago

@ashleygwilliams was working on implementing shallow routes for /users/{userid} when she brought up the fact that we're not doing any validation of the userid parameter: https://github.com/mozilla/api.webmaker.org/blob/d0122340e6cd018b90195d7a79bac5080e827661/services/api/routes/authenticated.js#L116-L131 . Currently we do not perform parameter validation or lookups in OPTIONS requests. Should we?

jbuck commented 9 years ago

I did some testing with the GitHub API and found that they do not do validation and/or lookups on OPTIONS requests:

curl -i -X OPTIONS https://api.github.com/users/octocat/orgs returns HTTP/1.1 204 No Content curl -i https://api.github.com/users/octocat/orgs returns HTTP/1.1 200 OK vs curl -i -X OPTIONS https://api.github.com/users/octocatnotfound/orgs returns HTTP/1.1 204 No Content curl -i https://api.github.com/users/octocatnotfound/orgs returns HTTP/1.1 404 Not Found

cadecairos commented 9 years ago

It's probably because OPTIONS requests are mainly generated by browsers, and don't modify anything. it'd just be unnecessary overhead for the server to validate

cadecairos commented 9 years ago

It does sound like we can/should modify the options requests to return 204 statuses instead of 200

jbuck commented 9 years ago

@cadecairos I agree that changing the status code to a 204 makes sense but I think the reason we shouldn't do validation on the server in the OPTIONS request is that the browser doesn't send the results of the CORS preflight request to the XHR. It just sets the error status on the XHR and prevents it from working.

cadecairos commented 9 years ago

That's a good point. The UA will inevitably try hitting the same route with GET/POST/PATCH etc and get the right status anyways.