[ModifyChunkResponsePlugin] Generates invalid Content-Length and connection error

[Telemattic]

Follow the steps in the example usage of ModifyChunkResponsePlugin, but fetch http://httpbin.org/get instead.
Found while exploring a repro for what I think is a related case to #1028

Steps to reproduce the behavior:

1. Run proxy.py as proxy --hostname 127.0.0.1 --plugins proxy.plugin.ModifyChunkResponsePlugin
2. Verify using curl -v -x localhost:8899 http://httpbin.org/get
3. curl prints out the request and response headers, followed by modify\nchunk\nresponse\nplugin\n
4. wait about 10 seconds
5. curl says:

• transfer closed with 226 bytes remaining to read
• Closing connection 0
  curl: (18) transfer closed with 226 bytes remaining to read

Expected behavior
At step 4 curl should have exited cleanly, without waiting for 10 seconds.

I believe what happened is the content-length was not corrected when modify was replace by ModifyChunkResponsePlugin, consequently the client is left waiting for data that is never going to come, and eventually the connection is closed by the proxy.

Version information

• OS: ubuntu
• Browser: curl
• Device: linux box
• proxy.py Version 2.3.1

[abhinavsingh]

You are correct, in prior versions we didn't really bothered to fix the content-length. But this should work in v2.4.0 series. Can you try out one of the RC releases? Now we also have an integration test for ModifyChunkResponsePlugin so it should be working fine.

Thanks for reporting this issue.

[Telemattic]

I grabbed 2.4.0rc7 (I grabbed an enlistment from github and did a checkout of the commit with the 2.4.0rc7 tag) and the issue still repros there, the content-length remains 255 in the response with the modified body.

I followed the top-level README instructions for labeled "From command line using repo source". I added a print statement in the plugin to make sure the right plugin was getting picked up (I'm a little vague on how the plugins will get resolved from my enlistment vs. my normal python environment) and that seems to be working. I checked the version of the proxy as well:

python3 -m proxy --version
2.4.0rc7

Here's a patch that fixes the plugin to generate valid responses.

patch.txt

[abhinavsingh]

I grabbed 2.4.0rc7 (I grabbed an enlistment from github and did a checkout of the commit with the 2.4.0rc7 tag) and the issue still repros there, the content-length remains 255 in the response with the modified body.

Remember this is a chunk response plugin, why are even seeing a Content-Length header at all? Are you trying to access a non-chunked-response resource here? Further, we do have integration tests from Python 3.6-3.10 across all major OS. So, I am surprised this is broken for you. How are integration tests working then?

Below is my findings:

1. make ca-certificates

2. python -m proxy --port 9000 --plugins proxy.plugin.CacheResponsesPlugin,proxy.plugin.ModifyChunkResponsePlugin --ca-key-file ca-key.pem --ca-cert-file ca-cert.pem --ca-signing-key ca-signing-key.pem --enable-web-server --num-acceptors 1

3. Curl request

curl -v -x localhost:9000 --cacert ca-cert.pem https://httpbin.org/stream/5                 ─╯
*   Trying ::1:9000...
* connect to ::1 port 9000 failed: Connection refused
*   Trying 127.0.0.1:9000...
* Connected to localhost (127.0.0.1) port 9000 (#0)
* allocate connect buffer!
* Establish HTTP proxy tunnel to httpbin.org:443
> CONNECT httpbin.org:443 HTTP/1.1
> Host: httpbin.org:443
> User-Agent: curl/7.77.0
> Proxy-Connection: Keep-Alive
> 
< HTTP/1.1 200 Connection established
< 
* Proxy replied 200 to CONNECT request
* CONNECT phase completed!
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*  CAfile: ca-cert.pem
*  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: CN=httpbin.org
*  start date: Jan 22 19:10:47 2022 GMT
*  expire date: Jan 22 19:10:47 2024 GMT
*  subjectAltName: host "httpbin.org" matched cert's "httpbin.org"
*  issuer: CN=localhost
*  SSL certificate verify ok.
> GET /stream/5 HTTP/1.1
> Host: httpbin.org
> User-Agent: curl/7.77.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Sat, 22 Jan 2022 19:10:57 GMT
< Content-Type: application/json
< Transfer-Encoding: chunked
< Connection: keep-alive
< Server: gunicorn/19.9.0
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Credentials: true
< 
modify
chunk
response
plugin
* Connection #0 to host localhost left intact

[abhinavsingh]

I looked at the patch and followed the code flow. Even if for some reason you are getting a Content-Length header, real fix should land here

proxy.py/proxy/common/utils.py

Lines 107 to 115 in 54e74a9

	for k, _ in headers.items():
	if k.lower() == b'content-length':
	has_content_length = True
	if k.lower() == b'transfer-encoding':
	has_transfer_encoding = True
	if body is not None and \
	not has_transfer_encoding and \
	not has_content_length:
	headers[b'Content-Length'] = bytes_(len(body))

.build_http_response() is ideally supposed to also update the Content-Length header value. But it doesn't if one already exists. Ignoring the fact that developer may have modified the response content. Hence, when you deleted Content-Length header, it started to work magically. But this is an unrelated issue.

Real issue is Content-Length header itself. Related RFC spec here https://greenbytes.de/tech/webdav/rfc2616.html#rfc.section.4.4

[abhinavsingh]

Opened

• build_http_response must always update Content-Length header #1041

to separately track the Content-Length scenario that you helped discover. Thank you