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

Set Content-Lenght header after modifying content #270

Merged
merged 1 commit into from Dec 2, 2019

Conversation

jaap3
Copy link
Member

@jaap3 jaap3 commented Nov 7, 2019

No description provided.

@coveralls
Copy link

coveralls commented Nov 7, 2019

Coverage Status

Coverage decreased (-0.06%) to 73.828% when pulling ab0020f on set-content-length into 15f7af6 on master.

@richardbarran
Copy link
Member

I haven't tested it, but the code looks good to me. Just one thing: could you update the README please?

@jaap3
Copy link
Member Author

jaap3 commented Nov 11, 2019

What exactly would you like to see updated in the README?

@richardbarran
Copy link
Member

My apologies! I was thinking of updating the CHANGELOG with a short description of your change, but typed README instead.

In some cases the Content-Length header is set before modifying the
content. Since these middlewares add more content to the response this
causes certain clients (e.g. Selenium) to only partially read the
response body.
@jaap3
Copy link
Member Author

jaap3 commented Dec 2, 2019

@richardbarran Sorry for the delay, but finally I got around to updating the CHANGELOG (and tweaked the code a bit as well). For reference, this https://github.com/django/django/blob/2.2.7/django/middleware/common.py#L111 is the code that would cause the Content-Length header to be set in the first place.

@richardbarran
Copy link
Member

Looks good to me!

@richardbarran richardbarran merged commit 33e5e80 into master Dec 2, 2019
@jaap3 jaap3 deleted the set-content-length branch December 2, 2019 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants