Fixed the issue:Supports specifying port ranges#6281
Fixed the issue:Supports specifying port ranges#6281xianglongfei-8888 wants to merge 1 commit intoavocado-framework:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the flexibility of the Avocado framework's status server by enabling the specification of port ranges for both listening and connection URIs. This change allows the system to automatically identify and utilize an available port within a defined range, which is particularly useful in dynamic or constrained network environments. The implementation also ensures consistent configuration by synchronizing the resolved port across relevant settings, streamlining the setup process for users. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature by adding support for specifying port ranges for the status server. However, the implementation of the port range resolution logic is vulnerable to a Denial of Service (DoS) due to unhandled OverflowError exceptions when port numbers are outside the valid range (0-65535), potentially crashing the Avocado runner process via malformed command-line arguments. Additionally, a high-severity issue exists with IPv6 support in the new port-finding logic, currently limited to IPv4, and a minor refactoring is suggested to remove a duplicated logging statement.
avocado/core/status/server.py
Outdated
| sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
| try: | ||
| sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) | ||
| sock.bind((host, port)) |
There was a problem hiding this comment.
The current implementation for finding an open port is hardcoded to IPv4 (socket.AF_INET) and does not correctly handle IPv6 addresses (e.g., [::1]). The bind call will fail for an IPv6 literal with brackets, and the address family is incorrect.
To properly support both IPv4 and IPv6, you should use socket.getaddrinfo to determine the correct address family and prepare the host string for the bind call.
avocado/core/status/server.py
Outdated
| start = int(start_s) | ||
| end = int(end_s) | ||
| if start > end: | ||
| raise ValueError( | ||
| f"Invalid port range (start > end) in status server URI: {uri}" | ||
| ) | ||
|
|
||
| last_exc = None |
There was a problem hiding this comment.
The resolve_listen_uri function is vulnerable to a Denial of Service (DoS) vulnerability. It iterates through a user-provided port range and attempts to bind a socket to each port. If the port number is outside the valid range (0-65535), socket.bind() raises an OverflowError. This exception is not caught in the calling code in avocado/plugins/runner_nrunner.py (which only handles ValueError and OSError), causing the entire Avocado runner process to crash. An attacker or user providing a port range like 127.0.0.1:65530-70000 or 127.0.0.1:-1-10 can trigger this crash. Validating the port range before entering the loop is recommended.
start = int(start_s)
end = int(end_s)
if start > end:
raise ValueError(
f"Invalid port range (start > end) in status server URI: {uri}"
)
if not (0 <= start <= 65535 and 0 <= end <= 65535):
raise ValueError(
f"Port range out of bounds (0-65535) in status server URI: {uri}"
)
last_exc = None
avocado/core/status/server.py
Outdated
| if ":" in self._uri: | ||
| host, port = self._uri.split(":") | ||
| host, port = self._uri.rsplit(":", 1) | ||
| port = int(port) | ||
| self._server_task = await asyncio.start_server( | ||
| self.cb, host=host, port=port, limit=limit | ||
| ) | ||
| LOG_JOB.info("Status server listening on %s", self._uri) | ||
| else: | ||
| self._server_task = await asyncio.start_unix_server( | ||
| self.cb, path=self._uri, limit=limit | ||
| ) | ||
| LOG_JOB.info("Status server listening on %s", self._uri) |
There was a problem hiding this comment.
The logging statement LOG_JOB.info(...) is duplicated in both branches of the if/else statement. You can avoid repetition by moving it after the block.
if ":" in self._uri:
host, port = self._uri.rsplit(":", 1)
port = int(port)
self._server_task = await asyncio.start_server(
self.cb, host=host, port=port, limit=limit
)
else:
self._server_task = await asyncio.start_unix_server(
self.cb, path=self._uri, limit=limit
)
LOG_JOB.info("Status server listening on %s", self._uri)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6281 +/- ##
==========================================
+ Coverage 73.48% 73.65% +0.17%
==========================================
Files 206 206
Lines 22494 22655 +161
==========================================
+ Hits 16530 16687 +157
- Misses 5964 5968 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
xianglongfei-8888
force-pushed
the
dev
branch
3 times, most recently
from
b7f0504 to
c19157a
Compare
clebergnu
left a comment
clebergnu
left a comment
There was a problem hiding this comment.
Hi @xianglongfei-8888 , thanks for working on this.
Issue #5550 mentions that the outcome desired is to have this behavior by default, that is, it should allow users running avocado multiple times to not hit the status server port allocation. Have you looked into making the port range the default setting?
avocado/core/status/server.py
Outdated
|
|
||
| last_exc = None | ||
| for port in range(start, end + 1): | ||
| sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) |
There was a problem hiding this comment.
This should really reuse avocado.utils.network.ports.find_free_port()
There was a problem hiding this comment.
This should really reuse avocado.utils.network.ports.find_free_port()
@clebergnu Thank you, it has been modified
xianglongfei-8888
force-pushed
the
dev
branch
2 times, most recently
from
88b0438 to
bf1310f
Compare
@clebergnu Thank you, it has been changed to default |
Signed-off-by: xianglongfei_uniontech <xianglongfei@uniontech.com>
|
@richtja Could you help review the code？ Thanks |
2 participants
Fixed the issue:Supports specifying port ranges, with automatic matching between listen and URI.
Issue: #5550
Command line:
avocado run examples/tests/passtest.py --status-server-disable-auto
--status-server-listen 127.0.0.1:8888-9000 (defalut) --status-server-uri 127.0.0.1:8888-9000 (defalut)
Use the testing program to occupy ports 8888/8889 for verification
Logs:
...
2026-03-04 15:06:58,073 avocado.job job L0291 INFO | 'run.status_server_listen': '127.0.0.1:8888-9000',
2026-03-04 15:06:58,073 avocado.job job L0291 INFO | 'run.status_server_uri': '127.0.0.1:8888-9000',
...
2026-03-04 15:06:58,439 avocado.job server L0068 INFO | Status server listening on 127.0.0.1:8890
2026-03-04 15:06:58,740 avocado.job testlogs L0132 INFO | examples/tests/passtest.py:PassTest.test: STARTED
2026-03-04 15:06:58,948 avocado.job testlogs L0138 INFO | examples/tests/passtest.py:PassTest.test: PASS
...
Signed-off-by: xianglongfei xianglongfei@uniontech.com