wsgiref.validate is too restrictive regarding HTTP_CONTENT_LENGTH/TYPE

[Forty-Bot]

Bug report

Bug description:

PEP 3333 specifies that certain CGI environment variables must be placed in the environ dictionary.

The environ dictionary is required to contain these CGI environment variables, as defined by the Common Gateway Interface specification

The CONTENT_TYPE and CONTENT_LENGTH environ variables must be set as follows:

CONTENT_TYPE
The contents of any Content-Type fields in the HTTP request. May be empty or absent.
CONTENT_LENGTH
The contents of any Content-Length fields in the HTTP request. May be empty or absent.

Additionally, HTTP headers must be converted to variables as follows:

HTTP_ Variables
Variables corresponding to the client-supplied HTTP request headers (i.e., variables whose names begin with "HTTP_"). The presence or absence of these variables should correspond with the presence or absence of the appropriate HTTP header in the request.

So if the client sends an HTTP Content-Length header, it must be converted into a CONTENT_LENGTH variable, and may be converted into a HTTP_CONTENT_LENGTH variable. However, the CGI specification makes the following additional comment about Content-Length and Content-Type:

The server is not required to create meta-variables for all the header fields that it receives. In particular, it SHOULD remove any header fields carrying authentication information, such as 'Authorization'; or that are available to the script in other variables, such as 'Content-Length' and 'Content-Type'. The server MAY remove header fields that relate solely to client-side communication issues, such as 'Connection'.

In particular note that HTTP_CONTENT_LENGTH should be removed, not must.

---

On the other hand, wsgiref.validate.validator checks

• That HTTP_CONTENT_TYPE and HTTP_CONTENT_LENGTH are not in the environment (these headers should appear as CONTENT_LENGTH and CONTENT_TYPE).

I believe this is too restrictive because WSGI servers are not required to remove these headers. In particular, nginx (link to the FastCGI documentation, but similar behavior is present for SCGI and UWSGI) passes all HTTP headers by default except Status and X-Accel-.... Instead, the validator should check that if HTTP_CONTENT_TYPE is present that CONTENT_TYPE is also present (and similarly for CONTENT_TYPE).

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

[picnixz]

Ok, so sorry for the back and forth but I think it's not a bug. The rationale is that HTTP_CONTENT_TYPE and HTTP_CONTENT_LENGTH don't seem to be part of the "allowed" headers. For the other headers, HTTP_<NAME> variables are such that <NAME> is not a mandatory environment variable so there is no way of having "duplicates". Unfortunately, I don't know the PEP author and I don't think they are active so it's hard to contact them for clarification here.

[picnixz]

@serhiy-storchaka @vadmium Do you have some insights here?

[Forty-Bot]

Ok, so sorry for the back and forth but I think it's not a bug.

The validator raises an exception for valid implementations. That's a bug because the validator should only raise exceptions for invalid implementations.

The rationale is that HTTP_CONTENT_TYPE and HTTP_CONTENT_LENGTH don't seem to be part of the "allowed" headers

All HTTP headers are "allowed". The CGI spec says that certain ones should be removed, but this is not required. PEP 3333 doesn't say anything about individual HTTP headers.

For some additional context, I believe the reason that these headers should be removed by the server is because they may no longer be relevant due to processing performed by the server. For example, the server may decode the request body, which will change the length. The new request body length would be stored in CONTENT_LENGTH, and HTTP_CONTENT_LENGTH would not match the request body. This behavior seems primarily designed to prevent application authors from accidentally using the wrong environment variable. However, it's still optional (should not must).

[picnixz]

All HTTP headers are "allowed". The CGI spec says that certain ones should be removed, but this is not required. PEP 3333 doesn't say anything about individual HTTP headers.

Yes, but the validator is about PEP-3333 and not about the CGI spec specifically so we can be stricter if we want (but this is subject to interpretation of PEP-3333). I think it's also legitimate to assume that PEP-3333 didn't want to have two conflicting variables as I said (that's what I meant about "not allowed").

The new request body length would be stored in CONTENT_LENGTH, and HTTP_CONTENT_LENGTH would not match the request body.

I don't think so. AFAIU, HTTP_* variables exactly match what was given in the request, so I see them as HTTP_CONTENT_LENGTH and CONTENT_LENGTH as being aliased. However, to avoid sync issues, forbidding HTTP_CONTENT_LENGTH and HTTP_CONTENT_TYPE (but that's my interpretation of PEP-3333) seems reasonable. My rationale is that CONTENT_LENGTH is:

The contents of any Content-Type fields in the HTTP request. May be empty or absent.

while HTTP_* means:

Variables corresponding to the client-supplied HTTP request headers

AFAICT, this is exactly the same, just that HTTP_CONTENT_LENGTH is being aliased to CONTENT_LENGTH for brievety reasons. In particular, I disagree that CONTENT_LENGTH and HTTP_CONTENT_LENGTH can be different.

In addition, this is backed by the implementation of WSGIRequestHandler:

        length = self.headers.get('content-length')
        if length:
            env['CONTENT_LENGTH'] = length

        for k, v in self.headers.items():
            k=k.replace('-','_').upper(); v=v.strip()
            if k in env:
                continue                    # skip content length, type,etc.
            if 'HTTP_'+k in env:
                env['HTTP_'+k] += ','+v     # comma-separate multiple headers
            else:
                env['HTTP_'+k] = v
        return env

For XML RPC, it also seems that we only rely on CONTENT_LENGTH in the environment as well:

    def handle_request(self, request_text=None):
        """Handle a single XML-RPC request passed through a CGI post method.

        If no XML data is given then it is read from stdin. The resulting
        XML-RPC response is printed to stdout along with the correct HTTP
        headers.
        """

        if request_text is None and \
            os.environ.get('REQUEST_METHOD', None) == 'GET':
            self.handle_get()
        else:
            # POST data is normally available through stdin
            try:
                length = int(os.environ.get('CONTENT_LENGTH', None))
            except (ValueError, TypeError):
                length = -1
            if request_text is None:
                request_text = sys.stdin.read(length)

            self.handle_xmlrpc(request_text)

---

Now, changing the validition is not breaking backward compatibility, so I'm also not opposed to make the check less strict for the sake of usability and align with existing implementations of CGI specs (outside PEP-3333), but this would still be a new feature.

Did this cause an issue with production code or is it purely theoretical?

[serhiy-storchaka]

Sorry, I am not expert in this area. Maybe ask on https://discuss.python.org/?

PEP 3333 was published 15 years ago, its predecessor, PEP 333, was published 22 years ago, why is this question only raised now?

[Forty-Bot]

Yes, but the validator is about PEP-3333 and not about the CGI spec specifically

Sure, but PEP 3333 doesn't say anything about these headers. In fact, these headers are only mentioned specifically in the CGI spec!

I don't think so. AFAIU, HTTP_* variables exactly match what was given in the request,

Exactly. So if e.g. the request was gzip-encoded and the server gunzips the file the CONTENT_LENGTH would no longer match HTTP_CONTENT_LENGTH:

The CONTENT_LENGTH value must reflect the length of the message-body after the server has removed any transfer-codings or content-codings.

Contrast with Content-Length (from RFC9110):

Content-Length refers specifically to the amount of data enclosed so that it can be used to delimit framing. In other cases, Content-Length indicates the selected representation's current length

which specifies that Content-Length applies to the encoded length.

In addition, this is backed by the implementation of WSGIRequestHandler:

I would expect so, since this is a should. Reference implementations should implement all shoulds, even if other implementations choose not to.

Did this cause an issue with production code or is it purely theoretical?

As far as I know this should only affect the validator. But it's the default settings for nginx.

---

PEP 3333 was published 15 years ago, its predecessor, PEP 333, was published 22 years ago, why is this question only raised now?

Because I ran the validator and noticed that it doesn't match the spec. This only occurs when you upload a file (as otherwise Content-Length/Type are omitted), and wsgiref.simple_server.demo_app doesn't do anything with uploaded files so I suspect it isn't a common test case.

[picnixz]

The validator is for PEP-3333 and interpretation of PEP-3333 is what we seem to disagree on. I see HTTP_CONTENT_LENGTH and CONTENT_LENGTH as representing the same quantity here.

I would expect so, since this is a should

What I meant is that the author of the handler understood the PEP as I did, that is we don't want possible de-synced CONTENT_LENGTH variables. For me, the PEP says that HTTP_CONTENT_LENGTH is meant to match CONTENT_LENGTH.

What about Flask and other frameworks written in Python? do they do something with wsgiref? I said "it shouldn't cause breakage"; but I'm not sure if there is some code elsewhere relying on the fact that some HTTP_* variables are omitted on purpose in the environment.

---

Honestly, I don't think it's worth changing this, unless there is a production compelling reason so I personally won't make a PR for changing the behavior. I would still want to know what other frameworks do, not just nginx, but maybe Apache, and more importantly Flask and Django, and FastWSGI maybe.