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

Raise RuntimeError is you try to set the Content Length with chunked encoding enable #1941

Merged

Conversation

julien-duponchelle
Copy link
Contributor

What do these changes do?

If you try to set a content length on a response with chunked encoding enable you get a RuntimeError

Are there changes in behavior for the user?

User will get an exception and not will be able to set the content length.

Related issue number

It's after a discussion here: #1933

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new entry to CHANGES.rst
    • Choose any open position to avoid merge conflicts with other PRs.
    • Add a link to the issue you are fixing (if any) using #issue_number format at the end of changelog message. Use Pull Request number if there are no issues for PR or PR covers the issue only partially.

@julien-duponchelle julien-duponchelle force-pushed the runtime_error_content_length branch from d03f9a7 to 07771f7 Compare June 1, 2017 15:26
@fafhrd91
Copy link
Member

fafhrd91 commented Jun 1, 2017

could you add similar check for chunked encoding?

@julien-duponchelle julien-duponchelle force-pushed the runtime_error_content_length branch from 07771f7 to a064de8 Compare June 8, 2017 14:36
@julien-duponchelle
Copy link
Contributor Author

Commit updated in order to add a similar check for enable_chunked_encoding

@codecov-io
Copy link

Codecov Report

Merging #1941 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1941      +/-   ##
==========================================
+ Coverage   97.08%   97.08%   +<.01%     
==========================================
  Files          37       37              
  Lines        7609     7613       +4     
  Branches     1327     1329       +2     
==========================================
+ Hits         7387     7391       +4     
  Misses        100      100              
  Partials      122      122
Impacted Files Coverage Δ
aiohttp/web_response.py 98.63% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b85bcd...a064de8. Read the comment docs.

@asvetlov asvetlov merged commit 57dc8a3 into aio-libs:master Jun 8, 2017
@asvetlov
Copy link
Member

asvetlov commented Jun 8, 2017

Thanks

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants