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.

Sign up for GitHub

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

Jump to bottom

Fix response `content-length` when reading from HAR files #6548

Merged

mhils merged 8 commits into mitmproxy:main from zanieb:zb/har-content-length

Dec 12, 2023

Contributor

zanieb commented Dec 12, 2023 •

edited

Loading

Description

Closes #6547

Responses in flows constructed from HAR files were using the Response.make utility which resulted in the injection of content-length headers. When a content-length header existed already, this could cause failures during replay. See the linked issue or 91bc983 for a concrete example.

Checklist

I have updated tests where applicable.
I have added an entry to the CHANGELOG.

zanieb and others added 4 commits

December 12, 2023 10:33


          Fix response content-length when reading from HAR files

4f66e37


          Update test fixtures

27d8efc


          [autofix.ci] apply automated fixes


          Add changelog entry

zanieb commented

View reviewed changes

mitmproxy/http.py Show resolved Hide resolved


          Remove unused code

e70f023

zanieb commented

View reviewed changes

mitmproxy/net/http/headers.py Show resolved Hide resolved

zanieb commented

View reviewed changes

test/mitmproxy/data/har_files/charles.json Show resolved Hide resolved

zanieb commented

View reviewed changes

test/mitmproxy/data/har_files/chrome.json Show resolved Hide resolved

zanieb commented

View reviewed changes

test/mitmproxy/data/har_files/firefox.json Show resolved Hide resolved


          Add regression test

91bc983

zanieb commented

View reviewed changes

test/mitmproxy/data/har_files/head-content-length.json Show resolved Hide resolved


          Revert temporary changes to show HAR file error

93fb4ed

We should expose this in the future, it is painful to debug when the message gets eaten

mhils approved these changes

View reviewed changes

Member

mhils left a comment

This was a breeze to review with all the comments, thanks a bunch! 🍰 😃

Member

mhils commented Dec 12, 2023

We have a CI job that enforces that mitmproxy/net/http/headers.py has 100% coverage from test/mitmproxy/net/http/test_headers.py. Once that is resolved this is good to go! :)


          Add test coverage for infer_content_encoding directly

3ab9b2e

mhils enabled auto-merge (squash)

December 12, 2023 17:20

Contributor Author

zanieb commented Dec 12, 2023

Thanks for the quick responses!

mhils merged commit 1fcd033 into mitmproxy:main

19 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet