Skip to content

Set reason to an empty string if it is missing (_http_parser.pyx)#3909

Merged
asvetlov merged 1 commit intoaio-libs:masterfrom
WouldYouKindly:fix-parsing-http-reason-phrase
Jul 22, 2019
Merged

Set reason to an empty string if it is missing (_http_parser.pyx)#3909
asvetlov merged 1 commit intoaio-libs:masterfrom
WouldYouKindly:fix-parsing-http-reason-phrase

Conversation

@WouldYouKindly
Copy link
Copy Markdown
Contributor

Resolve #3906

Test that if reason is missing, it is set to an empty string rather than any falsy value.

Make cython parser set it to en empty string if it is missing. self._reason = self._reason or '' is because _on_status_complete is called from two places and when it's called the second time, reason may already be set.

@WouldYouKindly WouldYouKindly requested a review from asvetlov as a code owner July 16, 2019 21:56
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 16, 2019

Codecov Report

Merging #3909 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3909      +/-   ##
==========================================
- Coverage   97.76%   97.73%   -0.03%     
==========================================
  Files          43       43              
  Lines        8620     8620              
  Branches     1380     1380              
==========================================
- Hits         8427     8425       -2     
- Misses         82       83       +1     
- Partials      111      112       +1
Impacted Files Coverage Δ
aiohttp/web_fileresponse.py 96.55% <0%> (-1.15%) ⬇️

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 95ead73...ee4d2d5. Read the comment docs.

Copy link
Copy Markdown
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, please add CHANGES file

@WouldYouKindly
Copy link
Copy Markdown
Contributor Author

@asvetlov done.

Copy link
Copy Markdown
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@asvetlov asvetlov merged commit 8d1f067 into aio-libs:master Jul 22, 2019
asvetlov pushed a commit that referenced this pull request Jul 22, 2019
asvetlov added a commit that referenced this pull request Jul 22, 2019
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.

Consider removing assert about HTTP Reason-Phrase being always present

3 participants