Copilot commented on code in PR #4097:
URL: https://github.com/apache/hertzbeat/pull/4097#discussion_r3008953145
##########
hertzbeat-common-core/src/main/java/org/apache/hertzbeat/common/entity/job/protocol/FtpProtocol.java:
##########
@@ -68,8 +73,17 @@ public class FtpProtocol implements CommonRequestProtocol,
Protocol {
@Override
public boolean isInvalid() {
-
- // todo: add
- return true;
+ if (!validateIpDomain(host) || !validPort(port) ||
StringUtils.isBlank(direction) || StringUtils.isBlank(timeout)) {
+ return true;
+ }
+ if (!CommonUtil.isNumeric(timeout)) {
Review Comment:
`CommonUtil.isNumeric(timeout)` accepts decimal/signed values (e.g.,
"3000.5"), but the FTP collector later uses `Integer.parseInt(timeout)` (e.g.,
in `FtpCollectImpl`) which would throw at runtime. Consider validating
`timeout` as an integer (e.g., parse with `Integer.parseInt` in a try/catch, or
use a stricter integer-only check) to keep `isInvalid()` consistent with
downstream parsing.
```suggestion
try {
Integer.parseInt(timeout);
} catch (NumberFormatException ex) {
```
##########
hertzbeat-common-core/src/main/java/org/apache/hertzbeat/common/entity/job/protocol/ImapProtocol.java:
##########
@@ -66,8 +71,17 @@ public class ImapProtocol implements CommonRequestProtocol,
Protocol {
@Override
public boolean isInvalid() {
-
- // todo: add
- return true;
+ if (!validateIpDomain(host) || !validPort(port)) {
+ return true;
+ }
+ if (StringUtils.isAnyBlank(timeout, email, authorize, folderName)) {
+ return true;
+ }
+ if (!CommonUtil.isNumeric(timeout)) {
Review Comment:
`CommonUtil.isNumeric(timeout)` treats decimals as numeric, but
`ImapCollectImpl` later does `Integer.parseInt(imapProtocol.getTimeout())`,
which will fail for values like "6000.5". Tighten the timeout validation here
to ensure it matches the collector’s integer parsing expectations (integer-only
parse/check).
```suggestion
if (!StringUtils.isNumeric(timeout)) {
```
##########
hertzbeat-common-core/src/main/java/org/apache/hertzbeat/common/entity/job/protocol/NgqlProtocol.java:
##########
@@ -74,8 +81,18 @@ public class NgqlProtocol implements CommonRequestProtocol,
Protocol {
@Override
public boolean isInvalid() {
-
- // todo: add
- return true;
+ if (!validateIpDomain(host) || !validPort(port)) {
+ return true;
+ }
+ if (StringUtils.isAnyBlank(username, password, timeout, parseType)) {
+ return true;
+ }
+ if (!VALID_PARSE_TYPES.contains(parseType) ||
!CommonUtil.isNumeric(timeout)) {
+ return true;
+ }
Review Comment:
`CommonUtil.isNumeric(timeout)` allows decimal/signed values, but
`NebulaTemplate` later calls `Integer.parseInt(protocol.getTimeout())` when
configuring the client pool. Update `isInvalid()` to validate `timeout` as an
integer (or attempt an `Integer.parseInt` in a try/catch) so invalid decimal
values don’t slip through and fail later at runtime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
