yii\filters\VerbFilter violates HTTP 1.1 spec

[PowerGamer1]

https://tools.ietf.org/html/rfc7230#section-3.1.1

The method token indicates the request method to be performed on the
target resource. The request method is case-sensitive.

https://tools.ietf.org/html/rfc7231#section-4

+---------+-------------------------------------------------+-------+
| Method | Description | Sec. |
+---------+-------------------------------------------------+-------+
| GET | Transfer a current representation of the target | 4.3.1 |
| | resource. | |
| HEAD | Same as GET, but only transfer the status line | 4.3.2 |
| | and header section. | |
| POST | Perform resource-specific processing on the | 4.3.3 |
| | request payload. | |
| PUT | Replace all current representations of the | 4.3.4 |
| | target resource with the request payload. | |
| DELETE | Remove all current representations of the | 4.3.5 |
| | target resource. | |
| CONNECT | Establish a tunnel to the server identified by | 4.3.6 |
| | the target resource. | |
| OPTIONS | Describe the communication options for the | 4.3.7 |
| | target resource. | |
| TRACE | Perform a message loop-back test along the path | 4.3.8 |
| | to the target resource. | |
+---------+-------------------------------------------------+-------+

yii\filters\VerbFilter class violates HTTP 1.1 spec by assuming that HTTP method names are case-insensitive as a result preventing the use of distinct and valid custom methods such as 'get', 'Get' (distinct from standard method 'GET'), etc.

[samdark]

I'm against the change. It breaks backwards compatibility and it's unlikely anyone would use Get method. Also it will be extremely hard to debug if one would use get.

[PowerGamer1]

So you basically promote writing code that breaks important industry-wide standards?? Are you now trying to follow Microsoft that did the same with their old versions of Internet Explorer several years ago? Don't you remember how "nice" it was to work with such browser that did things differently to the web standards?

It breaks backwards compatibility

If you so worried about breaking BC add the change to upgrade.md. Anyone who actually used VerbFilter in their code would immediately understand what to change and the change is very easy. Anyone who must keep BC no matter what would not upgrade to new Yii versions at all. Thing is, keeping compliance with an industry standard like HTTP is worth breaking BC and more.

it's unlikely anyone would use Get method

It is much more likely someone would want to use a custom and perfectly standard compliant method name like Combine - and currently such name would not work in Yii2 at all !

Also it will be extremely hard to debug if one would use get

I don't see how this one would be "extremely hard" to debug - you run a test REST request and look at the HTTP headers returned by the server - a mismatch in case of method names should not be hard to see. I can say from my own experience that there were instances of BC breaking NOT described in upgrade.md much less obvious that this one.

[PowerGamer1]

Oh and by the way, people who used only standard HTTP method names and actually followed the standard would not have any BC breaks in their code at all ! And as a bonus after the fix those people who violated the standard would be forced to change their code to be compliant with the standard too.

[samdark]

@yiisoft/core-developers need opinions.

[cebe]

whats the use case for case-sensitive HTTP methods?

[PowerGamer1]

whats the use case for case-sensitive HTTP methods?

For example, someone may want to use lowercased names for custom methods to easily distinguish them from the names of the standard HTTP method names (which are all uppercased). For example, lets say I develop a REST service which does not use standard HTTP method names and the standard semantics attached to them all together. And if I want to, say, use a method named with a word "put" but with a COMPLETELY different semantics from HTTP method PUT I will have to name it put or Put to be compliant with HTTP spec. Other custom methods I will also name in similar letter casing (Combine, Swap, etc).

But really, the only thing that matters is the precise and unambiguous REQUIREMENT of the standard:

The request method is case-sensitive

The HTTP standard specifies how two pieces of software developed by completely different parties have to interact with each other. The Yii2 framework represents a server side piece and as such HAS to be COMPLIANT. There hardly can be any other requirement more important than that.

HTTP standard for server-side developers should be like a Bible for Christians, I really don't understand how could you even consider the possibility of violating that specification?

[cebe]

I am not saying that Yii should not follow the standard, but as it currently does not, changing it will break a lot of applications and therefore we need to weigth the practial application of non-uppercase HTTP methods vs. breaking existing applications. I currently do not see the need to break anything in 2.0.x. We can adjust it in 2.1 to be compatible.

[PowerGamer1]

changing it will break a lot of applications

This is just your assumption and not a fact. As I said before, any application that was following the standard (by using uppercase names of standard HTTP methods) will not have any BC breaks.

breaking existing applications

Such applications SHOULD be broken for exactly the same reason - the applications not following the standard ideally should not exist at all. By not implementing the fix as soon as possible you not only bear responsibility for creating another non-standard compliant piece of software (Yii2 framework itself) but also responsibility for proliferation of hundreds of other non-standard compliant programs built by others with the use of Yii2 framework.

We can adjust it in 2.1 to be compatible.

Better than nothing I suppose. Personally I would seriously consider looking for alternative PHP frameworks built by people for whom compliance with industry standards is a topmost priority.

[PowerGamer1]

Oh and by the way at the very least fix the standard-violating COMMENTS with lowercase method names in VerbFilter.php that encourage other people to violate the standard and have absolutely zero danger of BC breaking.

[samdark]

This is just your assumption and not a fact.

I've reviewed about 10 apps this year and all of them used lowercase method names.

Such applications SHOULD be broken for exactly the same reason - the applications not following the standard ideally should not exist at all.

These will be suddenly broken after minor update and implications could be really serious including security issues.

Overall, I'm OK requiring uppercase methods in 2.1 but doubt it's a good change for 2.0.

[samdark]

Fixing comments is fine. Done 062e1c7

[PowerGamer1]

I've reviewed about 10 apps this year and all of them used lowercase method names.

And in every app you reviewed your response to the authors should have been "What are you doing violating the spec? Fix this immediately!".

These will be suddenly broken after minor update

That's the way of life - BC breaks happen even when we do not anticipate them. But in this particular case they will happen for the very right reason. Also, as I already said, they will be easy to fix.

including security issues.

Can't really think of any possible situation where such a change will lead to security issues.

Anyway, it does not look like I am going to sway your mind so do as you will. You are the ones to live with the reputation of yet another spec-disrespecting developers after all.

[samdark]

And in every app you reviewed your response to the authors should have been "What are you doing violating the spec? Fix this immediately!".

That's not the point. The point is that many apps will be broken for sure. Following the spec if fine but breaking many existing apps in minor version intentionally for the sake of formal compliance and nothing else is not.

That's the way of life - BC breaks happen even when we do not anticipate them.

In this case we know exactly that apps will be broken. The right thing to do in this case is to use 2.1 tag to release it where intentional breaks are totally fine. Of course, we sometimes introducing intentional breaks in 2.0 but these are for things concerning security mostly which could not be avoided.

Can't really think of any possible situation where such a change will lead to security issues.

Right. Not in this case.

[rob006]

Symfony also doesn't care about method case: https://github.com/symfony/http-foundation/blob/8c09d528eb7434a1cbc98444e28606999048fbf3/Request.php#L1206-L1221

[PowerGamer1]

breaking many existing apps in minor version intentionally for the sake of formal compliance and nothing else is not.

I bet Microsoft was thinking along those lines too, when they didn't want to fix their old IE to be compliant with the web standards only because "hey, so many sites now working with IE will break after the fix". Did this bring much good to the industry and Microsoft specifically in the end?

Symfony also doesn't care about method case: https://github.com/symfony/http-foundation/blob/8c09d528eb7434a1cbc98444e28606999048fbf3/Request.php#L1206-L1221

Good catch, I wonder what their reaction would be if someone reports them the fact that they are violating HTTP spec too?

[rob006]

I bet Microsoft was thinking along those lines too /.../

🤦 You're really overreacting here. All proper requests with standard HTTP method are handling properly.
And if you want to have put and PUT methods that works differently, you're just asking for trouble - it is good that Yii does not allow you to do that.

Good catch, I wonder what their reaction would be if someone reports them the fact that they are violating HTTP spec too?

There is only one way to find out. 😈

[bizley]

Please share the link to the Symfony issue with your report, I'm curious how this will be handled. Also remember to report similar to the Laravel.

[klimov-paul]

Request method names should be case-sensitive indeed. You may check this using following steps:

1. Create a simpe PHP script as following:

<?php
print_r($_POST);

2. Run following CURL commands from cli:

# POST reqcognised data output:
curl -X POST http://example.com/test.php --data "param1=value1&param2=value2"
# POST not reqcognised, no data output:
curl -X post http://example.com/test.php --data "param1=value1&param2=value2"

However, making such change causes a BC break as developer likely configure methods for VerbFilter in lowercase:

'actions' => [
     'index'  => ['get'],
     'create' => ['get', 'post'],
],

The obvious solution for the issue would be added an additional configuration field VerbFilter::$caseSensitive, which will determine whether method should be compared in case-sensitive mode. However it does not concern VerbFilter only, as yii\web\Request already uses strtoupper() while retrieving method name:
https://github.com/yiisoft/yii2/blob/master/framework/web/Request.php#L238
https://github.com/yiisoft/yii2/blob/master/framework/web/Request.php#L242
https://github.com/yiisoft/yii2/blob/master/framework/web/Request.php#L246

Thus same option needed for Request as well

[samdark]

I don't like introducing options here. Best thing is to fix that in 2.1.

[PowerGamer1]

Request method names should be case-sensitive indeed. You may check this using following steps:

That's how you write web software - you make it compliant with the standard.

The obvious solution for the issue would be added an additional configuration field

In case you decide to go this route, this field must have a default value of "verbs are case-sensitive" with a comment like "DON'T TOUCH unless you need it for BC - WILL BE REMOVED in the future". Then again, my opinion remains the same - this must be fixed ASAP regardless of BC break to force both old and new users of Yii2 to follow the standard.

[jblac]

I do agree with @PowerGamer1 on this one, while those 10 applications @samdark reviewed used lower case, how many do not? It's a problem I see all the time with Yii2 applications, yii2 doesn't follow standards ( PSR or otherwise ) so why should the users of the framework?

[klimov-paul]

Resolved by commit 89c14b7

[PowerGamer1]

@klimov-paul
Why haven't you also fixed Request.php - those places you mentioned above (#14458 (comment))? Without it the fix is incomplete.

[klimov-paul]

Because it is alerady done by #14701