Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove HTTP method case-insensitivity from Router #2500

Closed
akrabat opened this issue Sep 16, 2018 · 9 comments
Closed

Remove HTTP method case-insensitivity from Router #2500

akrabat opened this issue Sep 16, 2018 · 9 comments

Comments

@akrabat
Copy link
Member

akrabat commented Sep 16, 2018

As per https://tools.ietf.org/html/rfc7230#section-3.1.1, HTTP methods are case-sensitive.

Therefore we need to remove:

        // According to RFC methods are defined in uppercase (See RFC 7231)
        $methods = array_map("strtoupper", $methods);

From Slim\Router

@akrabat akrabat changed the title Remove method case-sensitivity from Router Remove HTTP method case-sensitivity from Router Sep 16, 2018
@vlakoff
Copy link
Contributor

vlakoff commented Sep 16, 2018

"Be resilient in what you receive, and strict in what you send".

PHP user code and HTTP header sent are not the same "layer" and don't have to be that tied.

I'd bend keeping this strtoupper.

@akrabat
Copy link
Member Author

akrabat commented Sep 17, 2018

However, $app->map('foo', FooHandler::class') has to match to the HTTP method foo, not FOO.

@JeanPaiva
Copy link
Contributor

JeanPaiva commented Sep 25, 2018

@akrabat can I get this one?

@akrabat
Copy link
Member Author

akrabat commented Sep 25, 2018

@JeanPaiva Sure.

@vlakoff
Copy link
Contributor

vlakoff commented Sep 25, 2018

Shouldn't the issue be titled "Remove HTTP method case-insensitivity from Router"?

@akrabat akrabat changed the title Remove HTTP method case-sensitivity from Router Remove HTTP method case-insensitivity from Router Sep 25, 2018
@JeanPaiva
Copy link
Contributor

JeanPaiva commented Sep 25, 2018

Now $router->map(['get'], '/foo', 'foo:bar'); will return Slim\Exception\HttpNotFoundException.
It's correct?
PR #2507

@vlakoff
Copy link
Contributor

vlakoff commented Sep 25, 2018

My comment above had been too fast.

I hadn't understood there is no change for the simple use case: the user will continue to do $app->get().

@vlakoff
Copy link
Contributor

vlakoff commented Sep 25, 2018

@JeanPaiva In theory yes, but it depends on the HTTP server, which may be tolerant on the case. But in practice, yes, they seem to follow the specs, and reject the request.

@akrabat akrabat added this to the 4.0 milestone Oct 2, 2018
@akrabat
Copy link
Member Author

akrabat commented Nov 25, 2018

Fixed by #2507.

@akrabat akrabat closed this as completed Nov 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants