onInvalidHTTPMethod not firing with SES Interceptor

Description

The SES Interceptor does not check for an onInvalidHTTPMethod() before throwing the standard error. Starting at line 130 in `coldbox/system/interceptors/SES.cfc`:

if( structKeyExists(aRoute,"action") && isStruct(aRoute.action) ){ // Verify HTTP method used is valid, else throw exception and 403 error if( structKeyExists(aRoute.action,HTTPMethod) ){ aRoute.action = aRoute.action[HTTPMethod]; // Send for logging in debug mode if( log.canDebug() ){ log.debug("Matched HTTP Method (#HTTPMethod#) to routed action: #aRoute.action#"); } } else{ getUtil().throwInvalidHTTP(className="SES", detail="The HTTP method used: #HTTPMethod# is not valid for the current executing resource. Valid methods are: #aRoute.action.toString()#", statusText="Invalid HTTP method: #HTTPMethod#", statusCode="405"); } }

Gliffy Diagrams

Activity

Show:

Evagoras Charalambous March 8, 2016 at 12:02 AM

"Having a global onInvalidHTTPHandler() that can manage global invalid HTTP calls" should be the ideal and should take care of all types of content type return formats.

I am not sure how I would then use an invalidHTTPHandler in the addRoute(), but I suppose it couldn't hurt.

Luis Majano March 7, 2016 at 4:03 PM

Ok, so how about making the ticket two parts:

1. Having a global onInvalidHTTPHandler() that can manage global invalid HTTP calls
2. Add an argument to the addRoute() called invalidHTTPHandler that can manage the invalid HTTP calls to that specific route permutation

Thoughts?

Evagoras Charalambous March 5, 2016 at 6:17 PM

We actually have cases where we serve JSON and PDFs under the same REST API endpoint, so makes a valid point. Thanks for thinking ahead on this, that totally escaped me.

Right now, to circumvent this issue, I have added the code change in the SES that I described above, and also added this to my base controller:

function onInvalidHTTPMethod ( event, rc, prc, faultAction, eventArguments ) { // Setup Response prc.response = wirebox.getInstance( "beans.response@v1" ) .setError( true ) .setErrorCode( 405 ) .setStatusCode( 405 ) .setStatusText( "Method Not Allowed" ) .setData( { "Message" = "The requested resource does not support the HTTP method '#event.getHTTPMethod()#'." } ); // Render Error Out event.renderData( type = prc.response.getFormat(), data = prc.response.getDataPacket(), contentType = prc.response.getContentType(), statusCode = prc.response.getStatusCode(), statusText = prc.response.getStatusText(), location = prc.response.getLocation(), isBinary = prc.response.getBinary() ); }

That, of course, results in a "405 Method Not Allowed" when a consumer calls an illegal method (like a DELETE for example), with this body:

{ "Message": "The requested resource does not support the HTTP method 'DELETE'." }

Jon Clausen March 5, 2016 at 5:06 PM
Edited

By setting it as a global, though, it's still not going to solve the core issue. For example, if you have an API handler that needs to always return JSON, but your app also has other handlers that need to render HTML, the global isn't going to solve the issue because it's going to force you to pick one: an HTML response or a JSON response.

Personally, I've now just gone the way of declaring the `onInvalidHTTPMethod` in my API routing method struct and letting the global method handle the HTML responses:

var defaultAPIActions = {"GET":"index","POST":"add","PUT":"onInvalidHTTPMethod","PATCH":"onInvalidHTTPMethod","DELETE":"onInvalidHTTPMethod"}

Ideally, though, you'd want the SES Interceptor to follow the ColdBox conventions. If an `onInvalidHTTPMethod` exists in the handler or the package, it should fire when the HTTPMethod isn't valid.

Evagoras Charalambous March 5, 2016 at 4:55 PM

That looks like it would work.

Fixed
Pinned fields
Click on the next to a field label to start pinning.

Details

Assignee

Reporter

Components

Fix versions

Affects versions

Priority

Sentry

Created October 15, 2015 at 3:18 PM
Updated September 13, 2016 at 10:55 PM
Resolved September 13, 2016 at 10:55 PM