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

Remove return statement that is never reached on get_trusted_client_host() method #1349

Closed
wants to merge 1 commit into from

Conversation

Kludex
Copy link
Member

@Kludex Kludex commented Feb 1, 2022

Before this PR:

+ venv/bin/coverage report --show-missing --skip-covered --fail-under=97
Name                                              Stmts   Miss  Cover   Missing
-------------------------------------------------------------------------------
tests/protocols/test_http.py                        473      1    99%   134
tests/protocols/test_websocket.py                   451      1    99%   622
tests/test_config.py                                260      1    99%   12
uvicorn/_handlers/http.py                            22      3    86%   67-76, 81
uvicorn/config.py                                   304     17    94%   16, 25-29, 141-142, 326-327, 331, 361, 517-519, 545-546, 552-554
uvicorn/middleware/proxy_headers.py                  33      1    97%   44
uvicorn/protocols/http/flow_control.py               34      6    82%   25, 38-40, 44-45
uvicorn/protocols/http/h11_impl.py                  304     14    95%   124-125, 225-226, 228, 243-244, 264-265, 317, 323, 421, 424, 439
uvicorn/protocols/http/httptools_impl.py            324     12    96%   125-126, 140, 155-156, 160, 318, 324, 422, 425, 457, 459
uvicorn/protocols/utils.py                           37      2    95%   14-17
uvicorn/protocols/websockets/websockets_impl.py     166      2    99%   105-106
uvicorn/protocols/websockets/wsproto_impl.py        222     27    88%   27, 77, 90, 100, 113, 115, 125, 131, 134-137, 169-180, 218-225
uvicorn/server.py                                   186     55    70%   20-25, 31-32, 90-92, 113-129, 133-139, 143-152, 164-167, 186-187, 193, 206, 249-251, 255, 267, 278-281, 297, 311-314
uvicorn/subprocess.py                                21      4    81%   69-76
uvicorn/supervisors/basereload.py                    51      7    86%   43-48, 81, 90
-------------------------------------------------------------------------------
TOTAL                                              4744    153    97%

This PR removes the following line:

uvicorn/middleware/proxy_headers.py                  33      1    97%   44

See comments about this on #591 (comment)

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

This doesn't look correct to me. The method surely can fall through that for statement?
(It might be that we don't have coverage for that case, but that's a different issue)

@Kludex
Copy link
Member Author

Kludex commented Feb 1, 2022

image

@tomchristie
Copy link
Member

So it can fall through to None if this is misconfigured.

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.

2 participants