Skip to content

[WIP] Complete chunked request on server eof#5238

Closed
greshilov wants to merge 1 commit intoaio-libs:masterfrom
greshilov:pr-5220
Closed

[WIP] Complete chunked request on server eof#5238
greshilov wants to merge 1 commit intoaio-libs:masterfrom
greshilov:pr-5220

Conversation

@greshilov
Copy link
Copy Markdown
Contributor

@greshilov greshilov commented Nov 14, 2020

What do these changes do?

Possible fix #5220

Code is hacky and questionable, hope maintainers will help.

Are there changes in behavior for the user?

No.

Related issue number

#5220

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 news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

@greshilov greshilov requested a review from asvetlov as a code owner November 14, 2020 23:40
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Nov 14, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 15, 2020

Codecov Report

Merging #5238 (5bcd59d) into master (a2fef0f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5238   +/-   ##
=======================================
  Coverage   97.54%   97.54%           
=======================================
  Files          43       43           
  Lines        8794     8796    +2     
  Branches     1413     1414    +1     
=======================================
+ Hits         8578     8580    +2     
  Misses        101      101           
  Partials      115      115           
Flag Coverage Δ
unit 97.39% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiohttp/client_reqrep.py 97.38% <100.00%> (+<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 a2fef0f...5bcd59d. Read the comment docs.

@luochen1990
Copy link
Copy Markdown

Hello, is this PR ready to be merged?

@Dreamsorcerer
Copy link
Copy Markdown
Member

@greshilov Want to revive this PR?
From what I can tell from asevtlov's comment (#5220 (comment)) this looks reasonable, just looks like we need something on the server side as well.

@thehesiod
Copy link
Copy Markdown
Contributor

@alexmac this may prove interesting

Dreamsorcerer added a commit that referenced this pull request Nov 3, 2023
Fixes #5220.

I believe this is a better fix than #5238. That PR detects that we
didn't finish sending a chunked response and then closes the connection.
This PR ensures that we simply complete the chunked response by sending
the EOF bytes, allowing the connection to remain open and be reused
normally.
@Dreamsorcerer
Copy link
Copy Markdown
Member

Superseded by #7764 .

Dreamsorcerer added a commit that referenced this pull request Nov 3, 2023
Fixes #5220.

I believe this is a better fix than #5238. That PR detects that we
didn't finish sending a chunked response and then closes the connection.
This PR ensures that we simply complete the chunked response by sending
the EOF bytes, allowing the connection to remain open and be reused
normally.

(cherry picked from commit 9c07121)
Dreamsorcerer added a commit that referenced this pull request Nov 3, 2023
Fixes #5220.

I believe this is a better fix than #5238. That PR detects that we
didn't finish sending a chunked response and then closes the connection.
This PR ensures that we simply complete the chunked response by sending
the EOF bytes, allowing the connection to remain open and be reused
normally.

(cherry picked from commit 9c07121)
xiangxli pushed a commit to xiangxli/aiohttp that referenced this pull request Dec 4, 2023
Fixes aio-libs#5220.

I believe this is a better fix than aio-libs#5238. That PR detects that we
didn't finish sending a chunked response and then closes the connection.
This PR ensures that we simply complete the chunked response by sending
the EOF bytes, allowing the connection to remain open and be reused
normally.

(cherry picked from commit 9c07121)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server returns 400: "invalid character in chunk size header" for valid request

5 participants