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

Return 400 on invalid major-only HTTP version in Request-Line #219

Merged
merged 2 commits into from
Aug 12, 2019
Merged

Return 400 on invalid major-only HTTP version in Request-Line #219

merged 2 commits into from
Aug 12, 2019

Conversation

alpire
Copy link
Contributor

@alpire alpire commented Aug 9, 2019

❓ What kind of change does this PR introduce?

  • 🐞 bug fix
  • 🐣 feature
  • πŸ“‹ docs update
  • πŸ“‹ tests/coverage improvement
  • πŸ“‹ refactoring
  • πŸ’₯ other

πŸ“‹ What is the related issue number (starting with #)

Fixes #218

❓ What is the current behavior? (You can also link to an open issue here)

cherrypy returns a 500 on some malformed HTTP request due to a missing argument in a format string in server.read_request_line.

❓ What is the new behavior (if this is a feature change)?

The server now returns a 400.

πŸ“‹ Checklist:

  • I think the code is well written
  • I wrote good commit messages
  • I have squashed related commits together after the changes have been approved
  • Unit tests for the changes exist
  • Integration tests for the changes exist (if applicable)
  • I used the same coding conventions as the rest of the project
  • The new code doesn't generate linter offenses
  • Documentation reflects the changes
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences

This change is Reviewable

@@ -273,6 +273,10 @@ def test_content_length_required(test_client):
b'GET / HTTPS/1.1', # invalid proto
HTTP_BAD_REQUEST, b'Malformed Request-Line: bad protocol',
),
(
b'GET / HTTP/1', # invalid version
Copy link
Member

Choose a reason for hiding this comment

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

Ideas for extra PRs:

  • b'GET / HTTP/x'
  • b'GET / HTTP/'
  • b'GET / HTTP/\\0'
  • b'GET / HTTP/β˜ƒ'

@webknjaz webknjaz changed the title Return 400 on invalid HTTP version (fix #218) Return 400 on invalid major-only HTTP version in Request-Line Aug 12, 2019
@codecov
Copy link

codecov bot commented Aug 12, 2019

Codecov Report

Merging #219 into master will decrease coverage by 0.68%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #219      +/-   ##
==========================================
- Coverage   74.97%   74.28%   -0.69%     
==========================================
  Files          23       23              
  Lines        3580     3582       +2     
==========================================
- Hits         2684     2661      -23     
- Misses        896      921      +25

1 similar comment
@codecov
Copy link

codecov bot commented Aug 12, 2019

Codecov Report

Merging #219 into master will decrease coverage by 0.68%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #219      +/-   ##
==========================================
- Coverage   74.97%   74.28%   -0.69%     
==========================================
  Files          23       23              
  Lines        3580     3582       +2     
==========================================
- Hits         2684     2661      -23     
- Misses        896      921      +25

@webknjaz webknjaz merged commit e03c3f5 into cherrypy:master Aug 12, 2019
@webknjaz
Copy link
Member

Thanks @alpire!

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.

cheroot/server.py:968: TypeError: not enough arguments for format string
2 participants