[💡FEATURE REQUEST]: Requests with `application/x-www-form-urlencoded` content-type are not parsed when request method is `DELETE`

[ekisu]

No duplicates 🥲.

• [X] I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

When sending requests with application/x-www-form-urlencoded parameters with the DELETE request method, it seems that the parameters are lost somewhere inside Roadrunner, a simple way to test is to use the following (assuming Roadrunner is listening in the port 9000):

# When sending as POST, the parameters appear properly on the PHP side
$ curl http://localhost:9000 -X POST --data-urlencode 'id=123' -v -o /dev/null
# But when sending with DELETE, they appear as `{}`
$ curl http://localhost:9000 -X DELETE --data-urlencode 'id=123' -v -o /dev/null

I've attached a log showing the Request object that is received inside the PSR7Worker in both cases

Version (rr --version)

2.10.6

Relevant log output

ROOT/vendor/spiral/roadrunner-http/src/PSR7Worker.php (line 107)
########## DEBUG ##########
object(Spiral\RoadRunner\Http\Request) id:0 {
  remoteAddr => '127.0.0.1'
  protocol => 'HTTP/1.1'
  method => 'POST'
  uri => 'http://localhost:9000/'
  headers => [
    'Accept' => [
      (int) 0 => '*/*'
    ],
    'Content-Length' => [
      (int) 0 => '6'
    ],
    'Content-Type' => [
      (int) 0 => 'application/x-www-form-urlencoded'
    ],
    'User-Agent' => [
      (int) 0 => 'curl/7.80.0'
    ]
  ]
  cookies => []
  uploads => []
  attributes => []
  query => []
  body => '{"id":"123"}'
  parsed => true
}
###########################

ROOT/vendor/spiral/roadrunner-http/src/PSR7Worker.php (line 107)
########## DEBUG ##########
object(Spiral\RoadRunner\Http\Request) id:0 {
  remoteAddr => '127.0.0.1'
  protocol => 'HTTP/1.1'
  method => 'DELETE'
  uri => 'http://localhost:9000/'
  headers => [
    'Accept' => [
      (int) 0 => '*/*'
    ],
    'Content-Length' => [
      (int) 0 => '6'
    ],
    'Content-Type' => [
      (int) 0 => 'application/x-www-form-urlencoded'
    ],
    'User-Agent' => [
      (int) 0 => 'curl/7.80.0'
    ]
  ]
  cookies => []
  uploads => []
  attributes => []
  query => []
  body => '{}'
  parsed => true
}
###########################

[rustatian]

👋 Hey. Please, update to the latest version. We support only the latest minor releases.

[ekisu]

Sure, I've updated both the server and the PHP library to the latest minor version (2.10.6), and I managed to reproduce this issue following the same steps

[rustatian]

Got u, thanks for the report. I'm unsure where this happens (PHP side or RR side), because RR doesn't limit the application/x-www-form-urlencoded to the only POST method... Anyway, I'll have a look.

[rustatian]

@ekisu According to the existing HTTP RFC-7231, the DELETE (https://httpwg.org/specs/rfc7231.html#DELETE) method has no defined behavior on parsing body (which you sent with application/x-www-form-urlencoded header).

A payload within a DELETE request message has no defined semantics; sending a payload body on a DELETE request might cause some existing implementations to reject the request.

So, technically, RR behavior is correct.

You may use either POST, PUT, or PATCH methods with the application/x-www-form-urlencoded if you want to be sure that RR will parse the body.

[ekisu]

I see, using PUT or PATCH does work indeed. In this case (using DELETE with the application/x-www-form-urlencoded content-type), shouldn't Roadrunner forward the request body to the PHP application as is? I would be fine with parsing it on the application side, but as seen on the logs, the entire request body seems to be replaced by an empty body

[rustatian]

@ekisu It's actually not replaced, but not parsed at all 😃. Unfortunately, this is not possible to send the entire body with the application/x-www-form-urlencoded, and method which has no defined semantics in your case, according to the specs.

[ekisu]

I've taken a look a bit deeper on what happens on PHP-FPM in this specific case, and here's what I've found:

• when it receives an POST request with the application/x-www-form-urlencoded content-type, it parses the data into the $_POST superglobal
• when it receives a request with the application/x-www-form-urlencoded content-type with any other request method rather than POST, it does not parse the data, sending it into the php://input stream instead

Looking at the source of the http plugin, it seems that Roadrunner always attempts to parse application/x-www-form-urlencoded request bodies if they're not HEAD or OPTIONS, replacing the original body - this means that the PHP application code has no longer any means to retrieve the original contents. This is just a guess, but I imagine that the Go ParseForm method only parses the body if it's a POST/PUT/PATCH request - and, if it's not, it just sets a default, empty value, which would explain this behavior.

Fortunately, in our usecase, we can change this request to something that works under Roadrunner, but I believe that replicating PHP-FPM's behavior would be the right way here, even if it's an implementation-specific behavior

[rustatian]

I'm not a PHP dev, so, sorry, I cannot refer to the PHP-FPM.

application/x-www-form-urlencoded request bodies if they're not HEAD or OPTION

You're right, but this is old behavior, and it should be updated to skip any method except PUT, PATCH, and POST.

This is just a guess, but I imagine that the Go ParseForm method only parses the body if it's a POST/PUT/PATCH request

Yes, ParseForm also follows the defined part of the standard and skips all methods, but PUT,POST,PATCH.

Fortunately, in our usecase, we can change this request to something that works under Roadrunner

That is awesome that you find ways to bypass this limitation.

but I believe that replicating PHP-FPM's behavior would be the right way here, even if it's an implementation-specific behavior

Might be. But, say in truth, we didn't follow PHP-FPM since we have standards. However, that's something PHP users are used to, I guess. I need to discuss this case with our PHP team to figure out the best way to handle this.

--

Since this is not a bug, may I ask you to rename this issue to a feature request? We will also wait for feedback from the community on this feature.

[ekisu]

Sure, I can't change the labels though :thinking:

You're right, but this is old behavior, and it should be updated to skip any method except PUT, PATCH, and POST.

Would it be possible to treat it as a contentStream instead? Not sure if this would abide to the standards, but it would mimic the "send it as a stream" part

[rustatian]

Would it be possible to treat it as a contentStream instead? Not sure if this would abide to the standards, but it would mimic the "send it as a stream" part

I can send a raw body in the RR's body field (we can't stream it so it might be just a binary body). I need to sync with a PHP part of the worker. We should be careful here not to break the existing behavior.

[rustatian]

Hey @ekisu 👋🏻

We discussed this FR with our PHP team, and as promised, I'm here with the results.

Since this is not a standard, we put this ticket in the Low priority. But, if this proposal is popular, we'll introduce a flag for the http plugin to redirect the payload to the PHP worker with minimal processing by RR.

[rustatian]

@ekisu Hey 👋🏻

After some discussion in the nearby ticket, we decided to send the untouched post form values data to the worker even if this is the DELETE method. The feature will be in the v2.11.1. (https://github.com/roadrunner-server/roadrunner/issues/1264#issuecomment-1220644959) and must be activated by the additional http plugin option (TBD).

[ekisu]

Whoa, this is great news! Thanks @rustatian

[rustatian]

Remember that that is valid only for the x-www-form-urlencoded values. Not for the multipart form values. The release is scheduled for Thursday, 25 Aug.