Copilot commented on code in PR #4057:
URL: https://github.com/apache/hertzbeat/pull/4057#discussion_r2901935524
##########
material/licenses/LICENSE:
##########
@@ -1,3 +1,4 @@
+
Apache License
Version 2.0, January 2004
Review Comment:
The LICENSE file now starts with a blank line. Some license scanners /
tooling expect the Apache 2.0 header to start at the first line, so this can
cause detection issues. Consider removing the leading empty line.
##########
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/AppServiceImpl.java:
##########
@@ -606,51 +592,25 @@ public void save(String app, String ymlContent) {
@Override
public void delete(String app) {
- throw new UnsupportedOperationException("define yml inside jars
cannot be deleted");
+ throw new UnsupportedOperationException();
}
-
}
private class LocalFileAppDefineStoreImpl implements AppDefineStore {
-
@Override
public boolean loadAppDefines() {
var rootUrl = this.getClass().getClassLoader().getResource("");
- if (rootUrl == null) {
- return false;
- }
- var classpath = rootUrl.getPath();
- var defineAppPath = classpath + "define";
- var directory = new File(defineAppPath);
- if (!directory.exists() || directory.listFiles() == null) {
- rootUrl = this.getClass().getResource(File.separator);
- if (rootUrl == null) {
- return false;
- }
- classpath = rootUrl.getPath();
- defineAppPath = classpath + "define";
- directory = new File(defineAppPath);
- if (!directory.exists() || directory.listFiles() == null) {
- return false;
- }
- }
- log.info("load define path {}", defineAppPath);
+ if (rootUrl == null) return false;
+ var directory = new File(rootUrl.getPath() + "define");
+ if (!directory.exists()) return false;
Yaml yaml = new Yaml();
for (var appFile : Objects.requireNonNull(directory.listFiles())) {
- if (appFile.exists() && appFile.isFile()) {
- if (appFile.isHidden()
- || (!appFile.getName().endsWith("yml") &&
!appFile.getName().endsWith("yaml"))) {
- log.error("Ignore this template file: {}.",
appFile.getName());
- continue;
- }
- try (var fileInputStream = new FileInputStream(appFile)) {
- var app = yaml.loadAs(fileInputStream, Job.class);
- if (app != null) {
- appDefines.put(app.getApp().toLowerCase(), app);
- }
- } catch (IOException e) {
- log.error(e.getMessage(), e);
- log.error("Ignore this template file: {}.",
appFile.getName());
+ if (appFile.isFile() && (appFile.getName().endsWith("yml") ||
appFile.getName().endsWith("yaml"))) {
Review Comment:
`loadAppDefines()` calls `Objects.requireNonNull(directory.listFiles())`
after only checking `directory.exists()`. `listFiles()` can return null (I/O
error / permissions) and `exists()` doesn't guarantee it's a directory, so this
can still throw an NPE. Consider checking `directory.isDirectory()` and
handling a null `listFiles()` result gracefully (e.g., treat as no files / log
a warning).
##########
hertzbeat-common-core/src/main/java/org/apache/hertzbeat/common/util/XmlUtil.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hertzbeat.common.util;
+
+
+import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
+import tools.jackson.core.JacksonException;
+import tools.jackson.core.type.TypeReference;
+import tools.jackson.databind.DeserializationFeature;
+import tools.jackson.databind.SerializationFeature;
+import tools.jackson.dataformat.xml.XmlMapper;
+
+/**
+ * xml util
+ */
+@Slf4j
+public class XmlUtil {
+
+ private static final XmlMapper XML_MAPPER = XmlMapper.builder()
+ .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
+ .configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false)
+ .build();
+
+ private XmlUtil() {
+ }
+
+ /**
+ * Convert object to XML string
+ */
+ public static String toXml(Object source) {
+ if (source == null) {
+ return null;
+ }
+ return XML_MAPPER.writeValueAsString(source);
+ }
+
+ /**
+ * Convert XML string to object
+ */
+ public static <T> T fromXml(String xml, Class<T> clazz) {
+ if (StringUtils.isEmpty(xml)) {
+ return null;
+ }
+ return XML_MAPPER.readValue(xml, clazz);
+ }
+
+ /**
+ * Convert XML string to object with TypeReference
+ */
+ public static <T> T fromXml(String xml, TypeReference<T> type) {
+ if (!StringUtils.isEmpty(xml)) {
+ return null;
+ }
Review Comment:
`fromXml(String, TypeReference)` has an inverted empty-check: it returns
`null` when the XML string is *not* empty, so it will never attempt to parse
valid XML. Flip the condition to return null only when the input is empty/blank
(and consider trimming/blank checks).
##########
home/i18n/zh-cn/docusaurus-plugin-content-docs/current/start/package-deploy.md:
##########
@@ -40,7 +40,7 @@ Apache HertzBeat™ 支持在Linux Windows Mac系统安装运行，CPU支持X86/
3. 配置账户文件(可选)
- HertzBeat 默认内置三个用户账户,分别为 admin/hertzbeat tom/hertzbeat guest/hertzbeat
+ HertzBeat 默认内置三个用户账户,分别为 admin/hertzbeat tom/hertzbeat guest/hertzbeat
Review Comment:
This line documents that HertzBeat ships with three built-in accounts
(`admin/hertzbeat`, `tom/hertzbeat`, `guest/hertzbeat`) that are active by
default via `config/sureness.yml`. If an instance is exposed to an untrusted
network and the operator does not immediately change these credentials, an
attacker can trivially obtain full access by logging in with these well-known
passwords. Consider removing or disabling default accounts in the packaged
`sureness.yml`, forcing users to define strong unique credentials on first
startup, and updating the documentation to clearly require changing any sample
credentials before going to production.
##########
hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/AlertDefineJsonImExportServiceImpl.java:
##########
@@ -52,22 +47,12 @@ public String getFileName() {
@Override
public List<ExportAlertDefineDTO> parseImport(InputStream is) {
- try {
- return objectMapper.readValue(is, new TypeReference<>() {
- });
- } catch (IOException ex) {
- log.error("import alertDefine failed.", ex);
- throw new RuntimeException("import alertDefine failed");
- }
+ return JsonUtil.fromJson(is, new TypeReference<>() {
+ });
Review Comment:
`parseImport` can return `null` (since `JsonUtil.fromJson(InputStream, …)`
returns null on parse errors).
`AlertDefineAbstractImExportServiceImpl#importConfig` immediately calls
`.stream()` on the result, which will throw a NullPointerException. Ensure
`parseImport` never returns null (e.g., throw a descriptive exception on parse
failure, or return an empty list).
```suggestion
List<ExportAlertDefineDTO> result = JsonUtil.fromJson(is, new
TypeReference<>() {
});
if (result == null) {
log.error("Failed to parse alert define JSON import:
JsonUtil.fromJson returned null. The input may be invalid or malformed.");
throw new IllegalArgumentException("Failed to parse alert define
JSON import: invalid or malformed JSON content.");
}
return result;
```
--
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]
