enhance the display url for all addresses 0.0.0.0 while using httpd#1
Conversation
|
This is a benchmark review for experiment This pull request was cloned from |
brendanator
left a comment
brendanator
left a comment
There was a problem hiding this comment.
Hey @brendanator - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Hard-coded IP addresses detected. (link)
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| :meta private: | ||
| """ | ||
| # arbitrary private address | ||
| host = "fd31:f903:5ab5:1::1" if family == socket.AF_INET6 else "10.253.155.219" |
There was a problem hiding this comment.
Using arbitrary private IP addresses might not be reachable in all environments. Consider using well-known IP addresses like '8.8.8.8' for IPv4 or '2001:4860:4860::8888' for IPv6.
| host = "fd31:f903:5ab5:1::1" if family == socket.AF_INET6 else "10.253.155.219" | |
| host = "2001:4860:4860::8888" if family == socket.AF_INET6 else "8.8.8.8" |
| except OSError: | ||
| return "::1" if family == socket.AF_INET6 else "127.0.0.1" | ||
|
|
||
| return s.getsockname()[0] # type: ignore |
There was a problem hiding this comment.
It's generally a good practice to avoid ignoring type checks. If it's necessary here, please add a comment explaining why, to maintain code clarity.
| :meta private: | ||
| """ | ||
| # arbitrary private address | ||
| host = "fd31:f903:5ab5:1::1" if family == socket.AF_INET6 else "10.253.155.219" |
There was a problem hiding this comment.
Using hard-coded IP addresses can lead to potential security risks and maintenance issues. Consider using environment variables or configuration files for these values.
3 participants
The original log
## Open your webbrowser at the URL: http://localhost:42525is ambiguous when I already setaddrto0.0.0.0. It looks like the server runing only on localhost.After merge this PR, when addr isn't
0.0.0.0, the log will show asand show as the following when addr is
0.0.0.0