Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content-Length header set to 0 in HEAD requests #659

Closed
johanpoirier opened this issue Jun 29, 2015 · 5 comments
Closed

Content-Length header set to 0 in HEAD requests #659

johanpoirier opened this issue Jun 29, 2015 · 5 comments
Labels
Milestone

Comments

@johanpoirier
Copy link

@johanpoirier johanpoirier commented Jun 29, 2015

Hi,

I g-have a problem with HEAD requests : mitmproxy override Content-Length header and set it to 0.

Without mitmproxy:

HTTP/1.1 200 OK
Accept-Ranges: bytes
Cache-Control: max-age=86400
Connection: keep-alive
Content-Length: 1488324
Content-Type: application/epub+zip
Date: Mon, 29 Jun 2015 09:20:54 GMT
ETag: "559103d3-16b5c4"
Expires: Tue, 30 Jun 2015 09:20:54 GMT
Last-Modified: Mon, 29 Jun 2015 08:37:39 GMT
Server: nginx

With mitmproxy:

Accept-Ranges: bytes
Cache-Control: max-age=86400
Connection: keep-alive
Content-Length: 0
Content-Type: application/epub+zip
Date: Mon, 29 Jun 2015 09:48:54 GMT
Etag: "559103d3-16b5c4"
Expires: Tue, 30 Jun 2015 09:48:54 GMT
Last-Modified: Mon, 29 Jun 2015 08:37:39 GMT
Server: nginx

I looked in the code and I saw that the Content-Length is computed from the length of the body of the request. Or HEAD requests have no body but can have a Content-Length > 0.

I don't know how to fix the bug but if anyone has a solution, I would be happy.

@mhils
Copy link
Member

@mhils mhils commented Jun 29, 2015

Thanks for the report - I think we don't consider yet that there are responses whose actual content length is not equal to Content-Length.

https://tools.ietf.org/html/rfc7230#section-3.3

Responses to the HEAD request method (Section 4.3.2 of [RFC7231]) never include a message body because the associated response header fields (e.g., Transfer-Encoding, Content-Length, etc.), if present, indicate only what their values would have been if the request method had been GET

We currently always set the content-length to the correct value at http.py#L803-L806. I'm afraid we need to get rid of that, which should be done very carefully as quite a few features depend on that.

@mhils mhils added the kind/bug label Jun 29, 2015
@mhils
Copy link
Member

@mhils mhils commented Sep 17, 2015

This should be fixed on master now, thanks!

@mhils mhils closed this Sep 17, 2015
@mhils mhils added this to the 0.14 milestone Sep 17, 2015
@jvillacorta
Copy link
Contributor

@jvillacorta jvillacorta commented Dec 10, 2015

On 0.15 I am still getting 0 as Content-Lenght on HEAD requests.

Without Proxy:

curl -i -X HEAD http://vid.applovin.com/1428106347_x.mp4
HTTP/1.1 200 OK
X-Trans-Id: tx4577952af96b4ea88804c-00562d1eb2ord1
Last-Modified: Sat, 04 Apr 2015 00:12:29 GMT
ETag: 2940aaa3e549ad144d762bb184b3f557
Content-Length: 3223986
Accept-Ranges: bytes
X-Timestamp: 1428106348.10108
Content-Type: video/mp4
Cache-Control: public, max-age=604800
Expires: Thu, 17 Dec 2015 18:30:29 GMT
Date: Thu, 10 Dec 2015 18:30:29 GMT

With Proxy:

X-Trans-Id: tx4577952af96b4ea88804c-00562d1eb2ord1
Last-Modified: Sat, 04 Apr 2015 00:12:29 GMT
ETag: 2940aaa3e549ad144d762bb184b3f557
content-length: 0
Accept-Ranges: bytes
X-Timestamp: 1428106348.10108
Content-Type: video/mp4
Cache-Control: public, max-age=604800
Expires: Thu, 17 Dec 2015 18:32:47 GMT
Date: Thu, 10 Dec 2015 18:32:47 GMT

@ecbaldwin
Copy link

@ecbaldwin ecbaldwin commented Jan 1, 2016

Looks like you "currently always set the content-length to the correct value" again but now its at message.py#L69-L70. I have no idea what the right way to fix it is but for my testing, I don't need those two lines and so I removed them.

@mhils mhils closed this in 11215e4 Jan 2, 2016
@mhils
Copy link
Member

@mhils mhils commented Jan 2, 2016

Fixed this one and added a regression test. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.