[HttpFoundation] Request without content-type or content-length header should result in null values, not empty strings

[priyadi]

Q	A
Branch?	5.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues
License	MIT

An HTTP request without content-type and content length will result in the variables $_SERVER['CONTENT_TYPE'] and $_SERVER['CONTENT_LENGTH'] having an empty string in PHP.

For example, the request:

POST / HTTP/1.1
Host: app.dev.localhost:8000
User-Agent: curl/8.4.0
Accept: /

(from curl invocation: curl -X 'POST' 'http://app.dev.localhost:8000/' -H 'Content-Type:' -H 'Content-length:' -d '' -v> /dev/null)

It will result in the variables $_SERVER['CONTENT_TYPE'] and $_SERVER['CONTENT_LENGTH'] containing an empty string.

In turn, $request->headers->get('Content-type') and $request->headers->get('Content-length') also result in an empty string.

This PR changes so that $request->headers->get('Content-type') and $request->headers->get('Content-length') will reflect the original state of the request header, that they return null (due to the fact the headers did not exist in the request), not an empty string (erroneously implying the client sent an empty content-type and content-length headers).

[OskarStark]

It sounds useful to me, but I am not sure what the spec is here (cc @stof).
Additionally it feels more like a new feature, because applications could rely on the current behavior 🤔

[94noni]

Perhaps add a deprecation layer for previous version and change for next?
Even if it seems not a « normal behavior »

[priyadi]

@OskarStark @94noni

I dug deeper and found that it only happens with php-fpm SAPI. Under apache and cli-server, it exhibits the correct behavior: both CONTENT_TYPE and CONTENT_LENGTH are unset. I did not test the other SAPIs.

The result: https://github.com/priyadi/php-empty-content-type-length-test/blob/main/output.txt

I believe the correct way forward is to treat this as a bug in php-fpm. But we'll still need the check in this PR, otherwise we need a specific PHP version requirement, which is a far larger burden for a really minor bug.

We can assume an empty CONTENT_TYPE and CONTENT_LENGTH means the headers are not in the request, as the current state of this PR. Or, we can add a check for php-fpm SAPI, and assume the other untested SAPIs are behaving correctly.

I leave the decision to the core devs.

[OskarStark]

Ping @symfony/mergers

[dunglas]

According to the CGI spec, not set and empty are equivalent and allowed:

This specification does not distinguish between zero-length (NULL)
values and missing values. For example, a script cannot distinguish
between the two requests http://host/script and http://host/script?
as in both cases the QUERY_STRING meta-variable would be NULL.

 meta-variable-value = "" | 1*<TEXT, CHAR or tokens of value>

An optional meta-variable may be omitted (left unset) if its value is
NULL. Meta-variable values MUST be considered case-sensitive except
as noted otherwise. The representation of the characters in the
meta-variables is system-defined; the server MUST convert values to
that representation.

https://www.rfc-editor.org/rfc/rfc3875#section-4.1

As PHP inherits from this spec, we should take empty values in consideration.

[dunglas]

To me, it's a bug fix as we aren't currently spec-compliant.

[priyadi]

@dunglas Do you think we need to extend the same treatment to all headers? i.e. we unset the header if the value is an empty string.

[dunglas]

We could apply something like array_filter() on the list, yes.

[nicolas-grekas]

Thank you @priyadi.

[nicolas-grekas]

i.e. we unset the header if the value is an empty string.

before doing that, we should verify that there is a SAPI that can report empty strings for headers.