Bad HTTP method throws exception

[nickdnk]

I am not sure this is to be considered an issue or if I should handle this myself with try/catch, but it seems odd to me that making a request with an unsupported HTTP method should throw an exception unless I have coded this to happen. Should it not just be answered with some default "Bad method" at least?

I get this in my log files:

PHP Fatal error: Uncaught exception 'InvalidArgumentException' with message 'Unsupported HTTP method "BDMTHD" provided' in ....

if I for instance run an external security audit.

I am no expert on security though, so I thought I'd bring this up with you guys.

[jordanbardsley7]

You mean this? https://github.com/slimphp/Slim/blob/3.x/Slim/DefaultServicesProvider.php#L172 If you look here the method must be one of https://github.com/slimphp/Slim/blob/3.x/Slim/Http/Request.php#L116 which is why you get that exception

[nickdnk]

I did configure my notAllowedHandler. But that seems to only handle those methods recognized by Slim. I cannot control what methods will be used, as those are generated by the client. The error description reads: Unsupported HTTP method "BDMTHD" provided - I cannot prevent this. BDMTHD is obviously short for "Bad Method" which is what the vulnerability scan intentionally uses to see how the server responds. I believe Slim by default should respond with "Bad request" instead of throwing an exception. Edit: Because there is no case where you'd use an unsupported HTTP method anyway.

My notAllowedHandler is configured like this:

$container['notAllowedHandler'] = function ($c) {
    return function ($request, $response) use ($c) {
        /** @var Slim\Views\Twig $c ['view'] */
        return $c['view']->render($response, 'templates/partials/notFound.twig')->withStatus(404);
    };
};

I just display the same notFound.twig as I do when someone requests a resource that does not exist, but again, this only works for valid request methods.

[designermonkey]

@nickdnk is this still an issue for you?

[nickdnk]

Hey

Unless you fixed it and I missed it, yes.

It's not more of an issue than en error log each request. I still think it should display a 404 for a bad route, since technically you're asking for a resource that doesn't exist (and never will). Or perhaps 400. Anyway I think an exception is unnecessary.

Sent from my iPhone

On 1 Oct 2016, at 14.11, John Porter notifications@github.com<mailto:notifications@github.com> wrote:

@nickdnkhttps://github.com/nickdnk is this still an issue for you?

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/slimphp/Slim/issues/1927#issuecomment-250909175, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIBYslv_QQfUBJUZm9-oyzrgIf67W046ks5qvk3qgaJpZM4JD_rl.

[designermonkey]

I've just had a look to re-familiarise myself with the code responsible and I can see a possible issue from my perspective.

I can understand why the code is as it is: HTTP spec allows for a specific set of methods, and the codebase follows specification. If the specification doesn't have those methods, then it is seen as a runtime exception.

There are a couple of solutions I think.

• One is to allow a method to add additional HTTP methods to the Request instance. Something like withCustomMethod.
• The other is to extend the Request object with a custom version, allowing for custom methods.

My preference would be to add the method, so only the container definition would need changing. This would fit well with the composition over extension principle (I think I wrote that right, correct me if wrong).

Let's ask for opinions from @akrabat and @silentworks.

[nickdnk]

Why are we adding custom methods? We have no idea what the method might be, hence my conclusion: If you send an HTTP method that is not part of the HTTP specs, the server should respond with 400 bad request or 404 not found. There is no reason a malformed request from a client should throw an Exception in the server code? I can't follow that logic at all.

If you specifically use a nonstandard method in your app, then I agree that it should be possible to extend allowed methods. But I'm taking about a HTTP methods/verbs that you cannot control.

I have no interest in using the method BADMTHD, but I don't think my server should handle such a request by throwing an exception.

I hope this makes sense. I had a few beers, honestly.

On 1 Oct 2016, at 21.42, John Porter notifications@github.com<mailto:notifications@github.com> wrote:

I've just had a look to re-familiarise myself with the code responsible and I can see a possible issue from my perspective.

I can understand why the code is as it is: HTTP spec allows for a specific set of methods, and the codebase follows specification. If the specification doesn't have those methods, then it is seen as a runtime exception.

There are a couple of solutions I think.

• One is to allow a method to add additional HTTP methods to the Request instance. Something like withCustomMethod.
• The other is to extend the Request object with a custom version, allowing for custom methods.

My preference would be to add the method, so only the container definition would need changing. This would fit well with the composition over extension principle (I think I wrote that right, correct me if wrong).

Let's ask for opinions from @akrabathttps://github.com/akrabat and @silentworkshttps://github.com/silentworks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/slimphp/Slim/issues/1927#issuecomment-250933022, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIBYsm4j9OPVJHCyXg2M-KqTSeAh4HcHks5qvreJgaJpZM4JD_rl.

[akrabat]

This is now being tracked on #2005 and needs to be fixed in Slim 4.