Reject NUL bytes in HTTP request lines#832
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #832 +/- ##
==========================================
- Coverage 78.15% 78.05% -0.10%
==========================================
Files 41 41
Lines 4788 4799 +11
Branches 547 548 +1
==========================================
+ Hits 3742 3746 +4
- Misses 906 912 +6
- Partials 140 141 +1 |
There was a problem hiding this comment.
Thanks for this PR. Looks useful but see inline comments. Also in the PR description you did not use the standard template that makes clear what you have and have not addressed.
Your commit also needs a body message and we need a docs/changelog-fragments.d/832.bugfix.rst describing the bug fix in the appropriate towncrier style - see docs/changelog-fragments.d/README.rst.
|
|
||
| def test_null_byte_in_request_line(test_client): | ||
| """Check that NUL bytes in the request line return Bad Request.""" | ||
| c = test_client.get_connection() |
There was a problem hiding this comment.
Let's use descriptive vars rather than single characters. conn would be better here.
|
|
||
| try: | ||
| method, uri, req_protocol = request_line.strip().split(SPACE, 2) | ||
| if b'\x00' in request_line: |
There was a problem hiding this comment.
Better to check for null bytes before doing any parsing. Suggest moving this above request_line.strip().split(SPACE, 2)
2 participants
Summary
Testing
python -m pytest --no-cov -n 0 cheroot/test/test_core.py -k null_byte_in_request_lineNote: the targeted test passed in the workspace, but pytest emitted a cache-permission warning in this sandbox after completion.