search

tempesta-tech / tempesta

All-in-one solution for high performance web content delivery and advanced protection against DDoS and web attacks
https://tempesta-tech.com/
GNU General Public License v2.0
626 stars 103 forks source link

Content-Length header addition #2209

Open const-t opened 3 months ago

const-t commented 3 months ago

We have some not optimal behavior. In case when Content-Length not listed in response headers and upstream server indicates message finishing by closing connection in function tfw_http_resp_terminate() we add Content-Length header. Upd: The first part of this task about not optimal behavior was done in PR2243.

But for http2 protocol we call tfw_h2_msg_cutoff_headers() and after HPack encode headers, it implies we must not add the header in tfw_http_resp_terminate() but add this header during http2 message adjusting.

ai-tmpst commented 2 months ago

It looks we can just skip Content-Length addition for HTTP2 in tfw_http_resp_terminate().

As for addition this header field during http2 message adjusting, we add this field in tfw_h2_resp_encode_headers() only for chunked responses:

    if (test_bit(TFW_HTTP_B_CHUNKED, resp->flags)) {
        if (unlikely(tfw_h2_add_hdr_clen(resp)))
            goto clean;
    }

I'm not sure what behavior is desired. RFC 3119 says nothing about content-length usage. It's a completely optional field. This issue looks independent, and we could discuss it separately.

ai-tmpst commented 2 weeks ago

Also there is PR726 in tempesta-test

ai-tmpst commented 2 weeks ago

In PR2243 we partly removed Content-length header for HTTP2. In the future we need to remove the rest of the places where this header is added. Also we need to correct tests (look for __prepare_chunked_expected_response function in framework/deproxy_auto_parser.py).

The first part of this task about not optimal behavior was done in PR2243.

krizhanovsky commented 1 week ago

https://github.com/tempesta-tech/tempesta/pull/2243 doesn't fully fix the problem